Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(435)

Issue 1047733002: Fixed mouseenter/mouseleave event firing order. (Closed)

Created:
5 years, 8 months ago by mustaq
Modified:
5 years, 8 months ago
Reviewers:
Mike West
CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-events_chromium.org, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Fixed mouseenter/mouseleave event firing order. Fixed mouseenter/mouseleave event firing order to match the spec: http://www.w3.org/TR/DOM-Level-3-Events/#events-mouseevent-event-order . We have been dispatching mouseenter and mouseleave events before mouseover and mouseout events, and also sending mouseenter events to child nodes before their parents. This patch decouples mouseenter/mouseleave from active/hover states to fix the problem. Also cleans up Document.cpp a bit. BUG=470947 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=192954

Patch Set 1 #

Patch Set 2 : Fixed tests. #

Total comments: 12

Patch Set 3 : Better comments. Brought back the deleted test. #

Patch Set 4 : #

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+236 lines, -88 lines) Patch
M LayoutTests/fast/events/mouse-events-within-no-element-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/fast/events/mouseenter-mouseleave-capture-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
D LayoutTests/fast/events/mouseenter-mouseleave-chained-listeners-expected.txt View 1 2 1 chunk +5 lines, -5 lines 0 comments Download
M LayoutTests/fast/events/mouseenter-mouseleave-expected.txt View 1 3 chunks +14 lines, -14 lines 0 comments Download
M LayoutTests/fast/events/mouseenter-mouseleave-inline-attributes-expected.txt View 1 3 chunks +14 lines, -14 lines 0 comments Download
A LayoutTests/fast/events/mouseenter-mouseleave-on-drag.html View 1 chunk +79 lines, -0 lines 0 comments Download
A LayoutTests/fast/events/mouseenter-mouseleave-on-drag-expected.txt View 1 chunk +19 lines, -0 lines 0 comments Download
M LayoutTests/fast/events/mousemove-from-iframe-to-top-element-expected.txt View 1 1 chunk +2 lines, -2 lines 0 comments Download
M LayoutTests/fast/events/resources/mouse-events-within-no-element-iframe.html View 1 1 chunk +2 lines, -2 lines 0 comments Download
M Source/core/dom/Document.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/dom/Document.cpp View 1 2 3 4 6 chunks +5 lines, -33 lines 0 comments Download
M Source/core/page/EventHandler.h View 1 2 1 chunk +4 lines, -1 line 0 comments Download
M Source/core/page/EventHandler.cpp View 1 2 3 4 5 chunks +88 lines, -13 lines 0 comments Download

Messages

Total messages: 17 (7 generated)
mustaq
ptal.
5 years, 8 months ago (2015-03-30 23:56:41 UTC) #2
Mike West
Looks pretty good % nits and a question. https://codereview.chromium.org/1047733002/diff/20001/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/1047733002/diff/20001/Source/core/dom/Document.cpp#newcode5326 Source/core/dom/Document.cpp:5326: // ...
5 years, 8 months ago (2015-03-31 11:45:34 UTC) #3
mustaq
mkwst@: ptal. + Addressed all nits. + Brought back listening to mouseenter that was set ...
5 years, 8 months ago (2015-03-31 14:08:32 UTC) #4
mustaq
https://codereview.chromium.org/1047733002/diff/20001/Source/core/page/EventHandler.cpp File Source/core/page/EventHandler.cpp (right): https://codereview.chromium.org/1047733002/diff/20001/Source/core/page/EventHandler.cpp#newcode1908 Source/core/page/EventHandler.cpp:1908: // for 'mouseleave' might set a capturing 'mouseenter' handler, ...
5 years, 8 months ago (2015-03-31 18:06:27 UTC) #5
Mike West
Would you mind adding a comment about our behavior with regard to DOM mutations in ...
5 years, 8 months ago (2015-04-01 10:00:58 UTC) #6
mustaq
On 2015/04/01 10:00:58, Mike West wrote: > Would you mind adding a comment about our ...
5 years, 8 months ago (2015-04-01 14:28:30 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1047733002/100001
5 years, 8 months ago (2015-04-01 14:56:37 UTC) #11
Rick Byers
On 2015/04/01 14:28:30, mustaq wrote: > On 2015/04/01 10:00:58, Mike West wrote: > > Would ...
5 years, 8 months ago (2015-04-01 15:12:15 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1047733002/120001
5 years, 8 months ago (2015-04-01 15:50:52 UTC) #16
commit-bot: I haz the power
5 years, 8 months ago (2015-04-01 18:20:22 UTC) #17
Message was sent while issue was closed.
Committed patchset #5 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=192954

Powered by Google App Engine
This is Rietveld 408576698