Update the mouse events on tapping of gesture across frames

Update the mouse events on tapping of gesture across frames

The problem is that mouseout/leave events are not fired on
tapping of gesture across frames after mouseover/enter is
triggered. It is caused by passing the targeted gesture
event directly to the hit frame.

This patch makes mouseout and mouseleave events be fired
first before passing to the hit frame, and mouseover and
mouseenter events be fired in the ancestor frame chain of
target frame.

BUG=475055
R=rbyers@chromium.org, mustaq@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198729

[Miyoung Shin(c), 2015-06-04 15:41:04]

PTAL.

[mustaq, 2015-06-08 17:24:25]

Thanks Miyoung for taking care of the bug. I will need more time to review, but
here are my current questions. There is only one major question, on use of
enteredFrameChain. Others are mostly on making the test simpler.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
File
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html
(right):

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:28:
<iframe src='resources/gesture-tap-mouse-events-between-frames-iframe.html'
id='frame'></iframe>
Please add the *-iframe.html file to the CL.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:32:
description("Verifies that tapping on an element sends mouse events to
appropriate ancestors of the tapped element in correct order.");
Please update the description.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:38:
["gray", "mouseover"],
My repro in the bug was a bit too complicated because I reused it for other
tests. I would suggest nuking the mid-level divs ("lightgreen" and "hotpink") in
this test. Since gesture-tap-mouse-events.html already tests mouse event order
for "deep" divs, it makes sense to focus only on inter-frame ordering in this
test.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:113:
eventSender.gestureTap(x, y);
I guess just sending a gestureTap here should be enough to test your logic
because we do not need to check mouseup, mousedown and click events here.
Correct me if I am missing something.

My motivation here is the same as in my last comment above: keep this test
focused on inter-frame mouse entry tracking.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:127:

Please cover the case of two consecutive taps in a common frame (as in the
repro), we need to make sure that in future no redundant events are fired in
such a case.

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
Source/core/page/EventHandler.cpp:2910: LocalFrame* enteredFrameInDocument =
indexEnteredFrameChain ? enteredFrameChain[indexEnteredFrameChain-1] : nullptr;
Could you please add a comment explaining the logic here? I don't see why other
entries in enteredFrameChain are not needed here. I was expecting a search for
the common ancestor in enteredFrameChain & exitedFrameChain, as is done here:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
File Source/core/page/EventHandler.h (right):

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
Source/core/page/EventHandler.h:202: Node* lastNodeUnderMouse() { return
m_lastNodeUnderMouse.get(); }
I think it's better not to make it public until needed. Can't you use
m_lastNodeUnderMouse directly?

[Miyoung Shin(c), 2015-06-10 12:10:05]

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
File
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html
(right):

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:28:
<iframe src='resources/gesture-tap-mouse-events-between-frames-iframe.html'
id='frame'></iframe>
On 2015/06/08 17:24:24, mustaq wrote:
> Please add the *-iframe.html file to the CL.

Sorry to miss the file. I've added it.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:32:
description("Verifies that tapping on an element sends mouse events to
appropriate ancestors of the tapped element in correct order.");
On 2015/06/08 17:24:24, mustaq wrote:
> Please update the description.

Done.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:38:
["gray", "mouseover"],
On 2015/06/08 17:24:24, mustaq wrote:
> My repro in the bug was a bit too complicated because I reused it for other
> tests. I would suggest nuking the mid-level divs ("lightgreen" and "hotpink")
in
> this test. Since gesture-tap-mouse-events.html already tests mouse event order
> for "deep" divs, it makes sense to focus only on inter-frame ordering in this
> test. 

I've removed the gray and hotpink's divs. (now using lightgreen/green and
lightpink,deeppink)
If you didn't mean it, please correct me.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:113:
eventSender.gestureTap(x, y);
On 2015/06/08 17:24:24, mustaq wrote:
> I guess just sending a gestureTap here should be enough to test your logic
> because we do not need to check mouseup, mousedown and click events here.
> Correct me if I am missing something.
> 
> My motivation here is the same as in my last comment above: keep this test
> focused on inter-frame mouse entry tracking.

I agree with you. I've removed to check the mousedown/up/click here and only
used gestureTap.

https://codereview.chromium.org/1170513002/diff/1/LayoutTests/fast/events/tou...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:127:

On 2015/06/08 17:24:24, mustaq wrote:
> Please cover the case of two consecutive taps in a common frame (as in the
> repro), we need to make sure that in future no redundant events are fired in
> such a case.

Done.

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
File Source/core/page/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
Source/core/page/EventHandler.cpp:2910: LocalFrame* enteredFrameInDocument =
indexEnteredFrameChain ? enteredFrameChain[indexEnteredFrameChain-1] : nullptr;
On 2015/06/08 17:24:24, mustaq wrote:
> Could you please add a comment explaining the logic here? I don't see why
other
> entries in enteredFrameChain are not needed here. I was expecting a search for
> the common ancestor in enteredFrameChain & exitedFrameChain, as is done here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

I tried to remain the comment in more detail at the next patch.

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
File Source/core/page/EventHandler.h (right):

https://codereview.chromium.org/1170513002/diff/1/Source/core/page/EventHandl...
Source/core/page/EventHandler.h:202: Node* lastNodeUnderMouse() { return
m_lastNodeUnderMouse.get(); }
On 2015/06/08 17:24:24, mustaq wrote:
> I think it's better not to make it public until needed. Can't you use
> m_lastNodeUnderMouse directly?

Done.

[mustaq, 2015-06-16 14:50:44]

https://codereview.chromium.org/1170513002/diff/20001/LayoutTests/fast/events...
File
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html
(right):

https://codereview.chromium.org/1170513002/diff/20001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:30:
description("Verifies that mouse events  are  fired on tapping of gesture across
frames in the correct order.");
Nit: double space before "are"

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2887: // Avoid that unnecessary mouse events
are fired on tapping across the frames
Thanks Miyoung, I was thinking about a scenario exactly like this. But my
concern was different: why is only the last entry in enteredFrameChain checked
here? Let's step though your example here to emphasize the issue here:

1. At the end of enteredFrameChain loop above, we have
enteredFrameChain=[E,D,A,Main].

2. During the first iteration of this loop for exitedFrameChain, we have
exitedFrameInDoc=C, and exitedFrameChain becomes [C].

3. For all subsequent iterations of this loop, Line 2883 is true, so Line 2884
pushes frames into the list w/o looking for duplicates in enteredFrameChain.

4. Therefore, at Line 2912, we have exitedFrameChain=[C,B,A,Main]. We shouldn't
expect A & Main in the list, right?

It would be great if you could add another test with multi-level frames, exactly
like your example here.

[Miyoung Shin(c), 2015-06-20 21:04:35]

Patchset #3 (id:40001) has been deleted

[Miyoung Shin(c), 2015-06-20 21:08:29]

mustaq@. I've uploaded the new patch. PTAL.

https://codereview.chromium.org/1170513002/diff/20001/LayoutTests/fast/events...
File
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html
(right):

https://codereview.chromium.org/1170513002/diff/20001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames.html:30:
description("Verifies that mouse events  are  fired on tapping of gesture across
frames in the correct order.");
On 2015/06/16 14:50:44, mustaq wrote:
> Nit: double space before "are"

Done.

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2887: // Avoid that unnecessary mouse events
are fired on tapping across the frames
On 2015/06/16 14:50:44, mustaq wrote:
> Thanks Miyoung, I was thinking about a scenario exactly like this. But my
> concern was different: why is only the last entry in enteredFrameChain checked
> here? Let's step though your example here to emphasize the issue here:
> 
> 1. At the end of enteredFrameChain loop above, we have
> enteredFrameChain=[E,D,A,Main].
> 
> 2. During the first iteration of this loop for exitedFrameChain, we have
> exitedFrameInDoc=C, and exitedFrameChain becomes [C].
> 
> 3. For all subsequent iterations of this loop, Line 2883 is true, so Line 2884
> pushes frames into the list w/o looking for duplicates in enteredFrameChain.
> 
> 4. Therefore, at Line 2912, we have exitedFrameChain=[C,B,A,Main]. We
shouldn't
> expect A & Main in the list, right?

The iteration of this loop is to check frames from Main to C. loop=[Main -> A ->
B -> C]
So, we have exitedFrameChain=[B, C] and enteredFrameChain= [E, D] (because of
moving the index to D) 

> 
> It would be great if you could add another test with multi-level frames,
exactly
> like your example here.

I've updated the test for multi-level frames.

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2927: while (indexEnteredFrameChain > 1) {
 After updating the test, I found A sub frame didn't update the mouseout and
mouseover events. so I changed the code here to work it.

[mustaq, 2015-07-02 20:37:30]

Sorry for the long delay!

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2887: // Avoid that unnecessary mouse events
are fired on tapping across the frames
On 2015/06/20 21:08:29, Miyoung Shin(c) wrote:
> On 2015/06/16 14:50:44, mustaq wrote:
> > Thanks Miyoung, I was thinking about a scenario exactly like this. But my
> > concern was different: why is only the last entry in enteredFrameChain
checked
> > here? Let's step though your example here to emphasize the issue here:
> > 
> > 1. At the end of enteredFrameChain loop above, we have
> > enteredFrameChain=[E,D,A,Main].
> > 
> > 2. During the first iteration of this loop for exitedFrameChain, we have
> > exitedFrameInDoc=C, and exitedFrameChain becomes [C].
> > 
> > 3. For all subsequent iterations of this loop, Line 2883 is true, so Line
2884
> > pushes frames into the list w/o looking for duplicates in enteredFrameChain.
> > 
> > 4. Therefore, at Line 2912, we have exitedFrameChain=[C,B,A,Main]. We
> shouldn't
> > expect A & Main in the list, right?
> 
> The iteration of this loop is to check frames from Main to C. loop=[Main -> A
->
> B -> C]
> So, we have exitedFrameChain=[B, C] and enteredFrameChain= [E, D] (because of
> moving the index to D) 
> 
> > 
> > It would be great if you could add another test with multi-level frames,
> exactly
> > like your example here.
> 
> I've updated the test for multi-level frames.

Thanks for clarifying, and also for adding the elaborate test.

I think your previous comment was correct: Frame A shouldn't receive
mouseout/over.

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2927: while (indexEnteredFrameChain > 1) {
On 2015/06/20 21:08:29, Miyoung Shin(c) wrote:
>  After updating the test, I found A sub frame didn't update the mouseout and
> mouseover events. so I changed the code here to work it.  

The change makes sense. But this shouldn't influence A because (as you mentioned
above) you already got rid of A from the list, right?

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
File
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames-expected.txt
(right):

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames-expected.txt:28:
PASS fired event is (greenyellow, mouseout).
There shouldn't be any mouseout/mouseover on greenyellow. Am I missing something
here?

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
File
LayoutTests/fast/events/touch/gesture/resources/gesture-tap-mouse-events-between-frames-iframe3.html
(right):

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/resources/gesture-tap-mouse-events-between-frames-iframe3.html:9:
<div class="box" id="darkgreen">
Nested divs is not our focus here, so please get rid of the inner div
("darkgreen").

Same for Frame5 ("deeppink").

https://codereview.chromium.org/1170513002/diff/60001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/60001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2524: LocalFrame* enteredFrameInDocument =
indexEnteredFrameChain ? enteredFrameChain[indexEnteredFrameChain-1] : nullptr;
Please avoid duplicate variable names with overlapping scopes. May be rename
this to "lastEntered..."? More below.

[Miyoung Shin(c), 2015-07-05 12:39:33]

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2927: while (indexEnteredFrameChain > 1) {
On 2015/07/02 20:37:29, mustaq wrote:
> On 2015/06/20 21:08:29, Miyoung Shin(c) wrote:
> >  After updating the test, I found A sub frame didn't update the mouseout and
> > mouseover events. so I changed the code here to work it.  
> 
> The change makes sense. But this shouldn't influence A because (as you
mentioned
> above) you already got rid of A from the list, right?

Yes I got rid of A from list, but A is the parent of D, so we should dispatch
mouseout/mouseover events on A when tapping between B and D.

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
File
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames-expected.txt
(right):

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames-expected.txt:28:
PASS fired event is (greenyellow, mouseout).
On 2015/07/02 20:37:29, mustaq wrote:
> There shouldn't be any mouseout/mouseover on greenyellow. Am I missing
something
> here?

As I tested the mouse case at http://output.jsbin.com/cenoje/quiet,
it shows the mouseout/mouseover events on parent element(lightyellow) when
moving the pointer between the child elements.

So, I checked the spec and it describe like below.
http://www.w3.org/TR/DOM-Level-3-Events/#event-type-mouseout
"it must be dispatched when the pointer device moves from an element onto the
boundaries of one of its descendent elements."

I think that the gesture event also keeps compatibility with mouse case.

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
File
LayoutTests/fast/events/touch/gesture/resources/gesture-tap-mouse-events-between-frames-iframe3.html
(right):

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/resources/gesture-tap-mouse-events-between-frames-iframe3.html:9:
<div class="box" id="darkgreen">
On 2015/07/02 20:37:29, mustaq wrote:
> Nested divs is not our focus here, so please get rid of the inner div
> ("darkgreen").
> 
> Same for Frame5 ("deeppink").

Done

https://codereview.chromium.org/1170513002/diff/60001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/60001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2524: LocalFrame* enteredFrameInDocument =
indexEnteredFrameChain ? enteredFrameChain[indexEnteredFrameChain-1] : nullptr;
On 2015/07/02 20:37:29, mustaq wrote:
> Please avoid duplicate variable names with overlapping scopes. May be rename
> this to "lastEntered..."? More below.

I changed the name to lastEnteredFrameInDocument.

[mustaq, 2015-07-06 17:14:50]

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/20001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2927: while (indexEnteredFrameChain > 1) {
On 2015/07/05 12:39:33, Miyoung Shin(c) wrote:
> On 2015/07/02 20:37:29, mustaq wrote:
> > On 2015/06/20 21:08:29, Miyoung Shin(c) wrote:
> > >  After updating the test, I found A sub frame didn't update the mouseout
and
> > > mouseover events. so I changed the code here to work it.  
> > 
> > The change makes sense. But this shouldn't influence A because (as you
> mentioned
> > above) you already got rid of A from the list, right?
> 
> Yes I got rid of A from list, but A is the parent of D, so we should dispatch
> mouseout/mouseover events on A when tapping between B and D.

I think A shouldn't get any bubbling events from child frames. See my question
in the expected.txt file.

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
File
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames-expected.txt
(right):

https://codereview.chromium.org/1170513002/diff/60001/LayoutTests/fast/events...
LayoutTests/fast/events/touch/gesture/gesture-tap-mouse-events-between-frames-expected.txt:28:
PASS fired event is (greenyellow, mouseout).
On 2015/07/05 12:39:33, Miyoung Shin(c) wrote:
> On 2015/07/02 20:37:29, mustaq wrote:
> > There shouldn't be any mouseout/mouseover on greenyellow. Am I missing
> something
> > here?
> 
> As I tested the mouse case at http://output.jsbin.com/cenoje/quiet,
> it shows the mouseout/mouseover events on parent element(lightyellow) when
> moving the pointer between the child elements.
> 
> So, I checked the spec and it describe like below.
> http://www.w3.org/TR/DOM-Level-3-Events/#event-type-mouseout
> "it must be dispatched when the pointer device moves from an element onto the
> boundaries of one of its descendent elements."
> 
> I think that the gesture event also keeps compatibility with mouse case. 

There are two tiny but important distinctions here: the events received by
lightyellow in this case are bubbled events only, and there is no frame
boundary. Bubbled events shouldn't cross frame boundaries.

Thanks for bringing this up, just realized that the spec seems unclear even
about non-bubbling mouseenter and mouseleve events across frame boundaries.
Compatibility with other browsers is our only guide here I think. Could you
please check what Firefox and IE do in the deep frame nesting example you made
(i.e. for A{B{C} D{E}} where X{Y} means frame X contains frame Y)? I see the
following possibilities:

#1. FF & IE are like your current solution: follow the frame-tree structure for
mouseenter/mouseleave as well as mouseover/mouseout.

#2. They follow the frame-tree structure for mouseenter/mouseleave only, and
mouseover/mouseout never come out of the deepest affected frame.

#3. Perhaps the simplest solution: update the mouse state (for all of
mouseenter, mouseleave, mouseover and mouseout) in the deepest affected frame
only without worrying about the frame tree structure. In your case, if the mouse
moves from C to E, none of A, B, and D should be notified.

If you have a jsbin demo for your deep frame nesting example, please update the
bug so that we can manually check corner cases.

[Miyoung Shin(c), 2015-07-08 14:51:07]

> Thanks for bringing this up, just realized that the spec seems unclear even
> about non-bubbling mouseenter and mouseleve events across frame boundaries.
> Compatibility with other browsers is our only guide here I think. Could you
> please check what Firefox and IE do in the deep frame nesting example you made
> (i.e. for A{B{C} D{E}} where X{Y} means frame X contains frame Y)? I see the
> following possibilities:
> 
> #1. FF & IE are like your current solution: follow the frame-tree structure
for
> mouseenter/mouseleave as well as mouseover/mouseout.
> 
> #2. They follow the frame-tree structure for mouseenter/mouseleave only, and
> mouseover/mouseout never come out of the deepest affected frame.
> 
> #3. Perhaps the simplest solution: update the mouse state (for all of
> mouseenter, mouseleave, mouseover and mouseout) in the deepest affected frame
> only without worrying about the frame tree structure. In your case, if the
mouse
> moves from C to E, none of A, B, and D should be notified.
> 
Sorry to delay the response. I've used JSbin for the first time so I needed time
getting used to.
Dome URL : https://output.jsbin.com/yofero/8
I checked FF / Chrome only (IE showed "Access is denied.") and I saw #1 result
among your possibilities.
I think it is reasonable that Chrome also follows #1 behavior for gesture event.
WDYT?

> If you have a jsbin demo for your deep frame nesting example, please update
the
> bug so that we can manually check corner cases.
Do you mean issue 475055 here ?

[mustaq, 2015-07-08 20:13:37]

On 2015/07/08 14:51:07, Miyoung Shin(c) wrote:
> > Thanks for bringing this up, just realized that the spec seems unclear even
> > about non-bubbling mouseenter and mouseleve events across frame boundaries.
> > Compatibility with other browsers is our only guide here I think. Could you
> > please check what Firefox and IE do in the deep frame nesting example you
made
> > (i.e. for A{B{C} D{E}} where X{Y} means frame X contains frame Y)? I see the
> > following possibilities:
> > 
> > #1. FF & IE are like your current solution: follow the frame-tree structure
> for
> > mouseenter/mouseleave as well as mouseover/mouseout.
> > 
> > #2. They follow the frame-tree structure for mouseenter/mouseleave only, and
> > mouseover/mouseout never come out of the deepest affected frame.
> > 
> > #3. Perhaps the simplest solution: update the mouse state (for all of
> > mouseenter, mouseleave, mouseover and mouseout) in the deepest affected
frame
> > only without worrying about the frame tree structure. In your case, if the
> mouse
> > moves from C to E, none of A, B, and D should be notified.
> > 
> Sorry to delay the response. I've used JSbin for the first time so I needed
time
> getting used to.
> Dome URL : https://output.jsbin.com/yofero/8
> I checked FF / Chrome only (IE showed "Access is denied.") and I saw #1 result
> among your possibilities.
> I think it is reasonable that Chrome also follows #1 behavior for gesture
event.
> WDYT?
> 
> > If you have a jsbin demo for your deep frame nesting example, please update
> the
> > bug so that we can manually check corner cases.
> Do you mean issue 475055 here ?

Yes, please post your demo in crbug.com/475055 for future reference. Also
mention in the bug that we are following FF behavior for mouse-entry states
across frames.

LGTM.

[mustaq, 2015-07-08 20:21:54]

A few nits I missed. Please fix them before CQing.

https://codereview.chromium.org/1170513002/diff/80001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/80001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2527: // Avoid that unnecessary mouse events
are fired on tapping across the frames
Forgot ask you before: please remove this comment block. I originally meant a
comment in your response email.

https://codereview.chromium.org/1170513002/diff/80001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2562: LocalFrame* exitedFrameInDocument =
exitedFrameChain[--indexExitedFrameChain];
Please rename "exitedFrameInDocument": a duplicate name in this scope.

[Miyoung Shin(c), 2015-07-09 11:59:44]

Rick@, I got LGTM from mustaq. PTAL.

https://codereview.chromium.org/1170513002/diff/80001/Source/core/input/Event...
File Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/1170513002/diff/80001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2527: // Avoid that unnecessary mouse events
are fired on tapping across the frames
On 2015/07/08 20:21:54, mustaq wrote:
> Forgot ask you before: please remove this comment block. I originally meant a
> comment in your response email.

Done.

https://codereview.chromium.org/1170513002/diff/80001/Source/core/input/Event...
Source/core/input/EventHandler.cpp:2562: LocalFrame* exitedFrameInDocument =
exitedFrameChain[--indexExitedFrameChain];
On 2015/07/08 20:21:54, mustaq wrote:
> Please rename "exitedFrameInDocument": a duplicate name in this scope.

Done.

[Rick Byers, 2015-07-10 19:52:10]

Wow, that took a fair amount of work, but (in addition to fixing the immediate
bug) we'll definitely need logic like this for pointer events.  Thanks both of
you!

LGTM

[Miyoung Shin(c), 2015-07-11 05:59:37]

The CQ bit was checked by myid.shin@chromium.org

[Miyoung Shin(c), 2015-07-11 05:59:38]

The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org
Link to the patchset: https://codereview.chromium.org/1170513002/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-07-11 05:59:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1170513002/100001

[commit-bot: I haz the power, 2015-07-11 07:02:46]

Committed patchset #5 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198729