Closed Bug 124990 Opened 23 years ago Closed 22 years ago

Event ordering rewrite tracking bug


(Core :: DOM: UI Events & Focus Handling, defect)

Not set





(Reporter: joki, Assigned: bryner)


(Blocks 1 open bug)


(Keywords: embed, topembed-, Whiteboard: [ADT2])


(5 files, 5 obsolete files)

Event handling at the XP level is being rewritten to use two separate passes
though the event handlers, one for external script handlers on web pages and a
second for internal event handlers used to trigger browser actions.

This bug fix will affect a number of current bugs, opening this tracking bug to
better organize them.
Target Milestone: --- → mozilla0.9.9
Blocks: 102005
Blocks: 112025
Blocks: 81739
Blocks: 65581
Blocks: 54035
124990 and its associated bugs is not quite ready for checkin for 0.9.9. 
Maintaining high priority and moving to 1.0 for completion and further testing.
Target Milestone: mozilla0.9.9 → mozilla1.0
Blocks: 127903
Keywords: topembed
Blocks: 128449
Attached patch Patch from event branch (obsolete) — Splinter Review
Attached patch Updated patch (obsolete) — Splinter Review
Attachment #74922 - Attachment is obsolete: true
Blocks: 53579
Blocks: 133478
Marking it topembed+. However any event rewrite sounds scary to me and prone to
regressions. What do we get out of rewriting it (i.e., benefits of it)?
Keywords: topembedtopembed+
What we gain is the ability to fix all the bugs blocked by this, a number of 
which are fairly important.  I agree, though, that it is risky.  With that in 
mind, saari and I have talked and decided to take a cautious route for getting 
this code in.  QA worked with it last week and it looks pretty good so far.  
We're planning to land it after moz1.0 and hopefully backport it to any release 
which needs the fixes it provides.  In addition, test builds of this branch 
will be available shortly for additional testing by anyone who wants to try it.
This patch appears to fix many event problems, bringing lots of goodness. Adding
nsbeta1, and ADT2 so that its on the ADT radar for MachV. 
Keywords: nsbeta1
Whiteboard: [ADT2]
Attached file zip with new files
New files from branch.	Can also be found on EVENTSREWRITE_20000211_BRANCH.
Attached patch One last updateSplinter Review
Attachment #74924 - Attachment is obsolete: true
Attachment #78283 - Attachment is obsolete: true
Attachment #79679 - Attachment is obsolete: true
Comment on attachment 79691 [details] [diff] [review]
One last update

heafty heafty heafy. r=saari
Attachment #79691 - Flags: review+
Comment on attachment 79691 [details] [diff] [review]
One last update

Attachment #79691 - Flags: superreview+
QA Contact: madhur → rakeshmishra
It looks like this was backed out because it caused regressions on Linux,
although there are no comments on the bug indicating that nor any indication in
the backout checkin comment of what the regressions were.  I heard some mention
on Friday of an inability to use Backspace in the address field of the compose
Window, although I can't find a corresponding bug.  However, there's another
problem I'm seeing (on Linux) which I suspect is related:  when I try to operate
the menus with the keyboard, I can open the menu with Alt-<shortcut>, but once I
do that no key events are accepted and I have to use the mouse.  (I can't use
Esc to close the menu or one of the shortcuts to select one of the items.)
dbaron: yes, the problem with backspace in mail compose is the only reason I'm
aware of that this was backed out (well, that and joki going on vacation for a
dbaron, thanks for the comment, I'm sure Tom will look at that too once he gets
back. Sounds like there is one issue that is unique to linux that is causing
this stuff, Tom just couldn't find it before vacation so we backed it out for now.
topembed-, embed per EDT triage.
Keywords: topembed+embed, topembed-
Blocks: 146399
nsbeta1- per the ADT. let's look at it for a future release.
Keywords: nsbeta1nsbeta1-
OK. So saari says the linux issues have been resolved.... could we possibly land
this at the start of the 1.2alpha cycle?  
Here's the small bit that had to be added to the patch.  It fixes a bug in the
gtk widget lib which resulted in reused nsEvent structs causing data from
keydown events to get into keypress events.
Comment on attachment 97710 [details] [diff] [review]
Small addition to patch to prevent reopening of 138548

Attachment #97710 - Flags: superreview+
Comment on attachment 97710 [details] [diff] [review]
Small addition to patch to prevent reopening of 138548

r=bzbarsky, but our default linux build is "gtk", not "gtk2", no?

Looking at nsGtkEventHandler.cpp, it looks like
handle_key_press_event_for_text() and handle_key_press_event() may need a
similar change (both reuse nsKeyEvent structs).
Attachment #97710 - Flags: review+
Yeah, what bz said.  The gtk2 build is experimental at this point, you need to
patch widget/src/gtk as well.
Comment on attachment 101479 [details] [diff] [review]
gtk1 patch to fix key event reusage

Attachment #101479 - Flags: superreview+
Comment on attachment 101479 [details] [diff] [review]
gtk1 patch to fix key event reusage

Attachment #101479 - Flags: review+
Both the gtk and gtk2 patches have been checked in on the trunk.
Assignee: joki → bryner
Blocks: 179330
This checkin caused regression bug 179172: Context menus no longer work in
Mailnews thread or folder panes.
This is checked in.  I'm tracking regressions on bug 180455.
Closed: 22 years ago
Resolution: --- → FIXED
QA Contact: rakeshmishra → trix
This patch implements an older version of the DOM Level 3 Events module; one
which had AddGroupedEventListener instead of AddEventListenerNS. I'll be
changing nsIDOM3EventTarget to remove those methods and add the up-to-date spec
methods instead, as part of my work on bug 256333. Since this iface is not
frozen, I guess it's okay.

Just FYI.
fwiw. this patch added the non-frozen interfaces:

to the sdk (the DOM3 Events spec has since changed at least some of these ifaces)
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.