[BLOCKER] editor shortcut keys get dispatched twice

VERIFIED FIXED in M10

Status

()

P3
blocker
VERIFIED FIXED
19 years ago
17 years ago

People

(Reporter: buster, Assigned: Brade)

Tracking

Trunk
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

(Reporter)

Description

19 years ago
in M9 candidate build, the editor has both a set of XUL keybindings and an
nsIDOMKeyListener.  Each dispatches editor commands, with the intent that the
XUL keybindings get first crack at a key event and if they trigger an action,
then the editor's own event listener never gets notification of the key event.
This is not happening.  XUL is firing the action, but not consuming the event.
Kathy and I tried a series of experiments that convinced us of this.  Kathy can
provide the details.  I did my experiments on Windows only.  Kathy can provide
details about other platforms.
(Reporter)

Updated

19 years ago
Target Milestone: M9
(Reporter)

Comment 1

19 years ago
something has to be done for this for M9.  Worse case is to yank XUL keybindings
from the editor completely.
(Assignee)

Comment 2

19 years ago
joki--I thought we had the events getting eaten but that doesn't seem to be the
case anymore.  Ideas?

Comment 3

19 years ago
I believe that it does still work as far as DOM is concerned.  I would suspect
the problem is in one handling keydown while the other handles keypress.

Comment 4

19 years ago
Wow, mid air collision, but I was about to say the same thing Tom. I thought akk
was going to remap the XUL to keydown?
(Assignee)

Comment 5

19 years ago
The correct behavior we agreed to was to use keypress.
Steve and I tried moving the internal (non-XUL) keybindings to keypress but it
still seemed to be getting called (with both XUL and internal keybindings are on
keypress).

Comment 6

19 years ago
Do you have an example binding that I can try?

Comment 7

19 years ago
Ok, I just walked through the XUL keybinding code, and it definitely does return
the event status of consumeNoDefault, via that hack that joki has... should I be
setting the status in the event directly as well? (for some reason I thought
returning that BASE_ERROR thing was currently correct thing to do)

Comment 8

19 years ago
I believe I am beginning to understand the problem. In nsRDFElement.cpp,
RDFElementImpl::HandleDOMEvent line 2347 there lies this comment :

    //Capturing stage
    // XXX Needs to be implemented.  Copy from nsGenericElement at some point.
    // Talk to joki@netscape.com for help.
    // For now at least handle document capture.

And after that, the event merrily goes on its way to be handled a second time.
There is the bug, CCing waterson. Joki, are we ready to do the right thing
instead of returning NS_BASE_ERROR?

Comment 9

19 years ago
I think their may still be a more basic misunderstanding here.  Internally
we're using the consumeNoDefault flag.  Now ignore that for a second.  From the
DOM perspective, there are three things that can be done to an event during
processing.  One is cancelling the default action.  This is the equivalent of
setting the consume flag.  This does *not* prevent further processing.  It
simply prevents the default action of the system.  Now in most cases the system
would be the browser itself but since much of the browser is implemented inside
the content area this is a little confusing as the system is really now Gecko,
not the browser, and since Gecko doesn't generally do too much with the event
themselves, cancelling the default action won't buy you much.

Then there are two other methods, preventCapture and preventBubble.
These methods will, respectively, prevent further handling by lower Nodes in
the tree after capturing and prevent further handling by higher Nodes in the
tree during bubbling.  If you wish a lower or higher level Node to get an event
and then stop the event from going further you need to use these flags.  The
consumeNoDefault flag will only tell the system to stop processing the event
but it will still go give the event to all the other DOM handlers.

Comment 10

19 years ago
By the way, if any of that was unclear please go read the DOM Level 2 Event
spec at:
http://www.w3.org/TR/WD-DOM-Level-2/events.html
Its real close to final so not much is gonna change.

Updated

19 years ago
Status: NEW → ASSIGNED

Comment 11

19 years ago
Thank you, much more clear now. So to properly "handle and consume" an event, I

need to preventCapture and preventBubble... I'll give that a shot.

Comment 12

19 years ago
Mmmm, tasty, with no harmful side effects (at least in my tests). I'll toss
patches at a few people before checking in.

Updated

19 years ago
Target Milestone: M9 → M10

Comment 13

19 years ago
Ok, I disabled keybinding on linux and win32 for M9, but I'm keeping the bug open

for the real fix in M10.



The real fix has a weird side effect on XPMenus that needs to be addressed before

I can check in the patch.
(Assignee)

Updated

19 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

19 years ago
saari said he fixed this for M10 now so marking this as fixed.

Comment 15

19 years ago
Just adding verification steps for when I regress this one:

 to verify this one: check Control-B (for Bold) that it does make the text bold
but doesn't unbold it
too (applying it a second time).

Updated

19 years ago
Status: RESOLVED → VERIFIED

Comment 16

19 years ago
seems to work fine now...verified in 9/2 build.
You need to log in before you can comment on or make changes to this bug.