[PATCH] 'sending unencrypted data' warning dialog regressions

[Lubos Lunak]

On Tuesday 25 of November 2003 15:51, David Faure wrote:
> On Tuesday 25 November 2003 15:31, Lubos Lunak wrote:
[snip]
> >
> >  The enter_keyrelease.patch seems to fix it. At least if I understand
> > correctly that 'm_active' means that the activating event (keypress or
> > mousepress) was done, and the matching release didn't happen yet
> > (HTMLGenericFormElementImpl::defaultEventHandler()).
>
> Didn't Dirk commit this already?

 Aha, I didn't notice (I complained about this to him yesterday). But I think 
he missed that one case that's in my patch.

>
> > [...]
> > 3. There's actually one more regression - holding Space on the button
> > activates it before Space is released. The fix should be indeed not
> > reacting on keyrelease events comming from autorepeat, but inspecting
> > TextEventImpl() ctor, KHTMLView::dispatchKeyEvent() and
> > KHTMLView::keyPressEvent() makes me little clueless person feel like
> > whoever wrote that was nuts.
>
> No, I think it's just that the Qt event model and the DOM event model
> differ. Qt has 2 events (press,release), the DOM has 3 events
> (down,press,up)
>
> Well, and the fact that we use a QKeyEvent as a data holder for the
> TextEventImpl, so the autorepeat bool is abused for another meaning,
> apparently (hence the comment I added to KHTMLView::dispatchKeyEvent).
>
> > Could somebody
> > please explain to me what KHTML_KEYDOWN_EVENT, KHTML_KEYUP_EVENT and
> > KHTML_KEYPRESS_EVENT are supposed to mean?
>
> The DOM events spec would do it better than anyone else :)

Each keyboard input results in 3 separate key events: KeyDown, followed by 
KeyPress, followed KeyUp. All platforms must generate these events in this 
sequence for every keyboard input (except IME events).

Key repeat (automatic generation of keyevents when a key is held down) results 
in a sequence of a KeyDown event, followed by multiple keypress events, 
terminated by a KeyUp event. A keypress event should be generated for each 
platform key repeat event recieved.

 I hope it's correct, I found this on some page.

>
> >  My guess based on reading the code is that
> > - KEYDOWN comes after pressing a key, and also while autorepeating
> > - KEYPRESS is the first press, i.e. not while autorepeating (kinda
> > strange that the if() in TextEventImpl ctor has key->isAutoRepeat() ).
>
> I think you mixed up those two.
> TextEventImpl says that KEYPRESS is when pressing and while autorepeating.
> KEYDOWN is when you press initially ("going down").
>
> The if() isn't strange - man qwidget says that autorepeating leads to
> repeated keyReleaseEvents in Qt. But "UP" is only when releasing for good -
> not during autorepeat.

 Hmm. It eventually even makes sense. It was just that some events weren't 
properly filtered out.

>
> > - KEYUP - is releasing the key, also while autorepeating
>
> No, only when releasing (the key goes up).
>
> > (which among other things means that I cannot fix the autorepeating
> > problem with Space above)
>
> If you want to activate when releasing then it should be listening for the
> dom KEYUP event... (I haven't looked into your actual problem though)

 It does indeed.

 I'll take your mail as a proof that it's really broken. And I see that this 
is a more important problem than just holding Space on a button being a bit 
weird (hey, I'm gonna to be 31337 KHTML hacker now ;) ).

 I suggest, besides fixing 2), and possibly also handling the one more case in 
1) which Dirk missed, to apply also the attached patch. I tested with 
khtmltests/ecma/events_document.html , and modified it a bit also to see the 
full sequence, and it generates "keydown,keypress,keyup" for single 
press+release, and "keydown, keypress repeatedly,keyup" for holding down a 
key.

-- 
Lubos Lunak
KDE developer
---------------------------------------------------------------------
SuSE CR, s.r.o.  e-mail: l.lunak at suse.cz , l.lunak at kde.org
Drahobejlova 27  tel: +420 2 9654 2373
190 00 Praha 9   fax: +420 2 9654 2374
Czech Republic   http://www.suse.cz/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: khtml.patch
Type: text/x-diff
Size: 1837 bytes
Desc: not available
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20031125/333a76d0/attachment.patch>