Event ordering rewrite tracking bug

RESOLVED FIXED in mozilla1.0

Status

()

defect
RESOLVED FIXED
18 years ago
4 months ago

People

(Reporter: joki, Assigned: bryner)

Tracking

(Blocks 2 bugs, {embed, topembed-})

Trunk
mozilla1.0
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ADT2])

Attachments

(5 attachments, 5 obsolete attachments)

Reporter

Description

18 years ago
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.
Reporter

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.9
Reporter

Updated

18 years ago
Blocks: 102005
Reporter

Updated

18 years ago
Blocks: 112025
Reporter

Updated

18 years ago
Blocks: 81739
Reporter

Updated

18 years ago
Blocks: 65581
Reporter

Updated

18 years ago
Blocks: 54035
Reporter

Comment 1

18 years ago
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

Updated

18 years ago
Keywords: topembed

Updated

18 years ago
Blocks: 128449
Reporter

Comment 2

18 years ago
Posted patch Patch from event branch (obsolete) — Splinter Review
Reporter

Comment 3

18 years ago
Posted patch Updated patch (obsolete) — Splinter Review
Reporter

Comment 4

18 years ago
Attachment #74922 - Attachment is obsolete: true

Updated

18 years ago
Blocks: 133478

Comment 5

18 years ago
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+
Reporter

Comment 6

18 years ago
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]
Reporter

Comment 9

17 years ago
Reporter

Comment 10

17 years ago
Posted file zip with new files
New files from branch.	Can also be found on EVENTSREWRITE_20000211_BRANCH.
Reporter

Comment 11

17 years ago
Attachment #74924 - Attachment is obsolete: true
Reporter

Updated

17 years ago
Attachment #78283 - Attachment is obsolete: true
Attachment #79679 - Attachment is obsolete: true

Comment 12

17 years ago
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

sr=jst
Attachment #79691 - Flags: superreview+

Updated

17 years ago
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.)
Assignee

Comment 15

17 years ago
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
week).

Comment 16

17 years ago
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.

Comment 17

17 years ago
topembed-, embed per EDT triage.
Keywords: topembed+embed, topembed-

Updated

17 years ago
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?  
Reporter

Comment 20

17 years ago
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

sr=jst
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+
Assignee

Comment 23

17 years ago
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

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

r=dbaron
Attachment #101479 - Flags: review+
Assignee

Comment 27

17 years ago
Both the gtk and gtk2 patches have been checked in on the trunk.
Assignee

Comment 29

17 years ago
taking.
Assignee: joki → bryner
Status: ASSIGNED → NEW

Updated

17 years ago
Blocks: 179330
This checkin caused regression bug 179172: Context menus no longer work in
Mailnews thread or folder panes.
Assignee

Comment 31

17 years ago
This is checked in.  I'm tracking regressions on bug 180455.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED

Updated

17 years ago
QA Contact: rakeshmishra → trix

Comment 32

15 years ago
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:
nsIDOM3DocumentEvent.idl
nsIDOM3EventTarget.idl
nsIDOMCustomEvent.idl
nsIDOMEventGroup.idl

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.