Closed
Bug 124990
Opened 23 years ago
Closed 22 years ago
Event ordering rewrite tracking bug
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Core
DOM: UI Events & Focus Handling
Tracking
()
RESOLVED
FIXED
mozilla1.0
People
(Reporter: joki, Assigned: bryner)
References
(Blocks 1 open bug)
Details
(Keywords: embed, topembed-, Whiteboard: [ADT2])
Attachments
(5 files, 5 obsolete files)
7.60 KB,
application/octet-stream
|
Details | |
110.04 KB,
patch
|
saari
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
1.52 KB,
patch
|
bzbarsky
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
2.82 KB,
patch
|
dbaron
:
review+
blizzard
:
superreview+
|
Details | Diff | Splinter Review |
106.33 KB,
patch
|
Details | Diff | Splinter Review |
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•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.9
Reporter | ||
Comment 1•23 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
Reporter | ||
Comment 2•23 years ago
|
||
Reporter | ||
Comment 3•23 years ago
|
||
Reporter | ||
Comment 4•23 years ago
|
||
Attachment #74922 -
Attachment is obsolete: true
Comment 5•23 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)?
Reporter | ||
Comment 6•23 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.
Comment 7•23 years ago
|
||
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 8•23 years ago
|
||
Reporter | ||
Comment 9•23 years ago
|
||
Reporter | ||
Comment 10•23 years ago
|
||
New files from branch. Can also be found on EVENTSREWRITE_20000211_BRANCH.
Reporter | ||
Comment 11•23 years ago
|
||
Attachment #74924 -
Attachment is obsolete: true
Reporter | ||
Updated•23 years ago
|
Attachment #78283 -
Attachment is obsolete: true
Attachment #79679 -
Attachment is obsolete: true
Comment 12•23 years ago
|
||
Comment on attachment 79691 [details] [diff] [review]
One last update
heafty heafty heafy. r=saari
Attachment #79691 -
Flags: review+
Comment 13•23 years ago
|
||
Comment on attachment 79691 [details] [diff] [review]
One last update
sr=jst
Attachment #79691 -
Flags: superreview+
Updated•23 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•23 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•23 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•23 years ago
|
||
topembed-, embed per EDT triage.
Comment 18•23 years ago
|
||
nsbeta1- per the ADT. let's look at it for a future release.
Comment 19•23 years ago
|
||
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•22 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 21•22 years ago
|
||
Comment on attachment 97710 [details] [diff] [review]
Small addition to patch to prevent reopening of 138548
sr=jst
Attachment #97710 -
Flags: superreview+
Comment 22•22 years ago
|
||
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•22 years ago
|
||
Yeah, what bz said. The gtk2 build is experimental at this point, you need to
patch widget/src/gtk as well.
Assignee | ||
Comment 24•22 years ago
|
||
Comment 25•22 years ago
|
||
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•22 years ago
|
||
Both the gtk and gtk2 patches have been checked in on the trunk.
Assignee | ||
Comment 28•22 years ago
|
||
Comment 30•22 years ago
|
||
This checkin caused regression bug 179172: Context menus no longer work in
Mailnews thread or folder panes.
Assignee | ||
Comment 31•22 years ago
|
||
This is checked in. I'm tracking regressions on bug 180455.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•22 years ago
|
QA Contact: rakeshmishra → trix
Comment 32•20 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.
Comment 33•20 years ago
|
||
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)
Comment 34•20 years ago
|
||
filed Bug 275220 about that last comment
Updated•6 years ago
|
Component: Event Handling → User events and focus handling
You need to log in
before you can comment on or make changes to this bug.
Description
•