Closed
Bug 428680
Opened 16 years ago
Closed 16 years ago
Sometimes, the document node is the event target for mousemove events now
Categories
(Core :: DOM: UI Events & Focus Handling, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9
People
(Reporter: martijn.martijn, Assigned: kinetik)
References
()
Details
(Keywords: regression, testcase)
Attachments
(2 files, 1 obsolete file)
513 bytes,
text/html
|
Details | |
2.74 KB,
patch
|
smaug
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
See testcase, which shows the nodeName of the target node of the mousemove event at that moment. It will only display new nodeNames when the nodeName has changed. In current trunk builds this will also show "#document" as result when you move from the document into the chrome area. This regressed between 2008-03-11 and 2008-03-13: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-03-11+04&maxdate=2008-03-13+09&cvsroot=%2Fcvsroot I think a regression from bug 297080. I think this is the cause of the js errors that can be seen on hi5.com. Example of js error: Error: target.getAttribute is not a function Source File: http://static.hi5.com/friend/js/photos/photo_tagging_1204594622.js Line: 1
Assignee | ||
Comment 1•16 years ago
|
||
I think this is a dupe of bug 426630, but the patch I posted in that bug doesn't fix the problem shown in the testcase attached to this bug. Taking.
Assignee: nobody → kinetik
Assignee | ||
Comment 2•16 years ago
|
||
The #document event target is showing up for mouse exit events that were previously treated as top-level exits by nsEventStateManager (before my patch from but #297080 was checked in) that we now treat as child-level exits. After the exit is munged into a move in nsEventStateManager::PreHandleEvent, we dispatch it into the DOM at PresShell::HandleEventInternal step #2, but via the fall-through path using mDocument as the event target.
Assignee | ||
Comment 3•16 years ago
|
||
The cause of this bug is that we convert a mouse exit (indicating we have left the content area and moved into chrome) into a mouse move. The event widget is the content area widget we have left, so the event is handled by the content view manager. PresShell::HandlePositionedEvent is called with the target frame being the viewport. Once we convert the event to a move inside nsEventStateManager::PreHandleEvent, the DOM event generation step (#2) in PresShell::HandleEventInternal results in the events targeting the top level document content (mDocument) because there is no content under the event location (since we have exited the area where anything could be targeted for this view manager). What should a mouse exit that has been converted into a mouse move target? We have already moved outside the area where it could target anything sensible, I think. So I propose that when we convert the exit into a move, we convert it into a synthesized move. This prevents regular mouse move handling on the event, but still allows mouse out and mouse over handling to take place. This patch fixes the testcase in this bug, and also fixes bug 426630 and bug 420804. The change to the windows widget code is not necessary to fix any of these, but I noticed the top-level exit detection was incorrect when I initially debugged bug 426630 (and attached this chunk of the patch there)--the problem is we were treating exits into the window frame as a child exit. This could mean we would mis a top-level exit if the pointer moved slowly into the window frame (generating a child exit) then out of the window (generating no further exit events). There was also a subtle bug visible where the pointer would change to the resizer and back again within a single pixel move as we hit the inner edge of the window frame--this was caused by treating the exit as a child rather than a top-level exit.
Attachment #316905 -
Flags: superreview?(roc)
Attachment #316905 -
Flags: review?(roc)
+ HWND mouseWnd = ::WindowFromPoint(mp); + HWND mouseTopLevel = nsWindow::GetTopLevelHWND(mouseWnd); + if (mouseWnd == mouseTopLevel) + return PR_TRUE; Is this correct? We return true if the window containing the point is a toplevel window. But what if the mouse moves from a child window into the toplevel window, won't this return true then? I think the synthesized event approach makes sense here, but CCing Olli and Ere in case they have an opinion.
Assignee | ||
Comment 5•16 years ago
|
||
(In reply to comment #4) > + HWND mouseWnd = ::WindowFromPoint(mp); > + HWND mouseTopLevel = nsWindow::GetTopLevelHWND(mouseWnd); > + if (mouseWnd == mouseTopLevel) > + return PR_TRUE; > > Is this correct? We return true if the window containing the point is a > toplevel window. But what if the mouse moves from a child window into the > toplevel window, won't this return true then? It is, because GetTopLevelHWND (with a default aStopOnFirstTopLevel parameter of false) ends up returning the HWND for the window frame (which includes the non-client area). So any exit into this window frame should be treated as a top level exit. If you pass aStopOnFirstTopLevel = true, GetTopLevelHWND would return the HWND for the top level of the client area, if I understand correctly.
Comment on attachment 316905 [details] [diff] [review] patch v1 OK, please add comments to that effect.
Attachment #316905 -
Flags: superreview?(roc)
Attachment #316905 -
Flags: superreview+
Attachment #316905 -
Flags: review?(roc)
Attachment #316905 -
Flags: review?(Olli.Pettay)
Assignee | ||
Comment 7•16 years ago
|
||
With comment.
Attachment #316905 -
Attachment is obsolete: true
Attachment #316914 -
Flags: review?(Olli.Pettay)
Attachment #316905 -
Flags: review?(Olli.Pettay)
Updated•16 years ago
|
Attachment #316914 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 8•16 years ago
|
||
Comment on attachment 316914 [details] [diff] [review] patch v2 Requesting approval. Fixes a nasty web content visible regression introduced by my patch for bug 297080, and also fixes bug 426630 and bug 420804.
Attachment #316914 -
Flags: approval1.9?
Comment 9•16 years ago
|
||
Comment on attachment 316914 [details] [diff] [review] patch v2 a1.9+=damons
Attachment #316914 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Comment 10•16 years ago
|
||
mozilla/widget/src/windows/nsWindow.cpp 3.741 mozilla/content/events/src/nsEventStateManager.cpp 1.741
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Updated•5 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
•