New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Regression

Blocking:
issue 276329



Sign in to add a comment
link

Issue 515921: Mouseleave fires on elements no longer in the DOM

Reported by cuiffo@google.com, Jul 31 2015 Project Member

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.89 Safari/537.36

Steps to reproduce the problem:
1. Open example: http://jsfiddle.net/cuiffo/czmohkvo/2/
2. Move mouse over the black box
3. Click and notice the element is removed, do not move mouse!
4. Move the mouse after waiting and then notice that the event listener is fired on the already removed element

What is the expected behavior?
Listener should not fire. If anything, it should fire during the removal of the element. This is the behavior in Firefox.

What went wrong?
Listener fired on the removed element. 

Did this work before? N/A 

Chrome version: 44.0.2403.89  Channel: stable
OS Version: 
Flash Version: Shockwave Flash 18.0 r0
 

Comment 1 by gundrum@google.com, Jul 31 2015

Labels: Hotlist-GoogleApps
Internal bug that affected Google Sheets: b/22847019

Comment 2 by tkent@chromium.org, Aug 3 2015

Labels: -Cr-Blink-JavaScript Cr-Blink-Input

Comment 3 by amin...@google.com, Aug 3 2015

Labels: -Pri-2 Pri-1
This is regressing a first-party application so bumping priority.  Let's make sure this gets reviewed sooner rather than later.

Comment 4 by rsch...@chromium.org, Aug 3 2015

Labels: Needs-Bisect
Requesting bisect from QA team. Just to make it explicit, the GOOD behavior is that the listener count should not increase on step 4. The BAD behavior is that it does.

Comment 5 by smokana@chromium.org, Aug 4 2015

Cc: rsch...@chromium.org
Labels: -Type-Bug -OS-Linux -Needs-Bisect Type-Bug-Regression OS-All M-46
Owner: mustaq@chromium.org
Status: Assigned
Able to reproduce the issue on Windows, MAC and Linux.

This is a regression issue broken in M43 and below is the Bisect Info:

CHANGELOG URL:
  https://chromium.googlesource.com/chromium/src/+log/6932a19f6429d867eab2114a6a3019484c0f20b3..2f958f4157655afa8e86afb0549fafe51b79f219

BLINK CHANGELOG URL:
  http://build.chromium.org/f/chromium/perf/dashboard/ui/changelog_blink.html?url=/trunk&range=192954%3A192953

From Blink roll, suspecting - 192954

Comment 6 Deleted

Comment 7 by mustaq@chromium.org, Aug 6 2015

It seems IE behaves perfectly here, and both Chrome & FF break the mouseenter/mouseleave pairing for existing (non-deleted) ancestor nodes.

Here is a slightly different repro to highlight both the issues (pairing problem + events on deleted node):

1. Open http://output.jsbin.com/dowabo/1
2. Move the mouse over the part of 'green' NOT covering 'yellow'.
3. Click on 'green' to make it disappear.
4. Move the mouse over to 'yellow'.

Expected and actual sequences of mouseenter & mouseleave on 'green' (G) & 'yellow' (Y):
- Expected  sequence:
    mouseenter on Y, mouseeneter on G, mouseleave on Y, mouseenter on Y.

- Actual sequence in Chrome:
    mouseenter on Y, mouseeneter on G, mouseleave on G, mouseenter on Y.
    Problem: mouseleave on a deleted node.

- Actual sequence in FF:
    mouseenter on Y, mouseeneter on G, mouseenter on Y.
    Problem: double mouseeneter on non-deleted node.

- Actual sequence in IE: Perfect.

A slightly modified test: if in Step 2 the mouse is moved to the part of 'green' COVERING 'yellow', and other steps remain the same, Chrome & FF produce the same output as before, but IE again matches the expected sequence:
  mouseenter on Y, mouseeneter on G.

Chrome should match the IE behavior for sure.

Comment 8 by rbyers@chromium.org, Aug 12 2015

Blocking: chromium:276329

Comment 9 by mustaq@chromium.org, Aug 13 2015

Cc: dtapu...@chromium.org
This bug seems somewhat conflicting with  crbug.com/479207 . My current patch (crrev.com/1288483003) causes fast/events/mouseout-dead-node.html to fail.

I checked FF and got confused: FF fires mouseouts on dead nodes for mouseout-dead-node.html, but doesn't fire mouseouts in my repro in #7 above.

Dave, any comments?

Comment 10 by mustaq@chromium.org, Aug 13 2015

FF behavior is now clear: if the deleted nodes had got the event listeners through addEventListener, they don't receive the events after deletion. Otherwise, they do.

This doesn't make sense, so I am inclined to make chromium consistent: don't fire the events for any deleted nodes.

Comment 11 by dtapu...@chromium.org, Aug 14 2015

I agree with comment #10..  Doing less work is better; and if other vendors aren't firing the events then we shouldn't either.

Comment 12 by mustaq@chromium.org, Aug 14 2015

Thanks Dave. Just confirmed that my fix (crrev.com/1288483003) makes chromium behave exactly the same as IE11, which I believe conforms to the spec that "the propagation path must reflect the hierarchical tree structure of the document" determined "at the beginning of the dispatch".
  http://www.w3.org/TR/uievents/#event-flow

Here is my repro to highlight IE behavior vs the inconsistency in FF:
  http://output.jsbin.com/yalaxi

Comment 13 by bugdroid1@chromium.org, Aug 17 2015

Project Member
The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=200656

------------------------------------------------------------------
r200656 | mustaq@chromium.org | 2015-08-17T18:25:19.341568Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/mouse-events-on-node-deletion.html?r1=200656&r2=200655&pathrev=200656
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/mouseout-dead-node-expected.txt?r1=200656&r2=200655&pathrev=200656
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/mouseover-mouseout-expected.txt?r1=200656&r2=200655&pathrev=200656
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/mouseover-mouseout2-expected.txt?r1=200656&r2=200655&pathrev=200656
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/mouse-events-on-node-deletion-expected.txt?r1=200656&r2=200655&pathrev=200656
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/mouseover-mouseout.html?r1=200656&r2=200655&pathrev=200656
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/input/EventHandler.cpp?r1=200656&r2=200655&pathrev=200656
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/mouseover-mouseout2.html?r1=200656&r2=200655&pathrev=200656

Skipping mouseenter/over/out/leave on deleted nodes.

Both IE and FF skip firing of mouse transition events at deleted nodes,
with only IE doing it in a perfect manner by maintining the pairing of
entry/exit events. A crack in our code had been causing firing of these
events at a node after the node got deleted. This CL fixes the bug by
making chromium behave similar to FF. The perfect solution seems
non-trivial.

BUG= 515921 

Review URL: https://codereview.chromium.org/1288483003
-----------------------------------------------------------------

Comment 14 by mustaq@chromium.org, Aug 17 2015

Status: Fixed

Comment 15 by mustaq@chromium.org, Sep 15 2015

Cc: yu...@chromium.org ssamanoori@chromium.org tdres...@chromium.org esprehn@chromium.org lanwei@chromium.org
 Issue 529032  has been merged into this issue.

Comment 16 by bugdroid1@chromium.org, Sep 23 2015

Project Member
Labels: merge-merged-2490
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd

commit a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd
Author: mustaq@chromium.org <mustaq@chromium.org>
Date: Mon Aug 17 18:25:19 2015

Skipping mouseenter/over/out/leave on deleted nodes.

Both IE and FF skip firing of mouse transition events at deleted nodes,
with only IE doing it in a perfect manner by maintining the pairing of
entry/exit events. A crack in our code had been causing firing of these
events at a node after the node got deleted. This CL fixes the bug by
making chromium behave similar to FF. The perfect solution seems
non-trivial.

BUG= 515921 

Review URL: https://codereview.chromium.org/1288483003

git-svn-id: svn://svn.chromium.org/blink/trunk@200656 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[add] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/LayoutTests/fast/events/mouse-events-on-node-deletion-expected.txt
[add] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/LayoutTests/fast/events/mouse-events-on-node-deletion.html
[modify] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/LayoutTests/fast/events/mouseout-dead-node-expected.txt
[modify] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/LayoutTests/fast/events/mouseover-mouseout-expected.txt
[modify] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/LayoutTests/fast/events/mouseover-mouseout.html
[modify] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/LayoutTests/fast/events/mouseover-mouseout2-expected.txt
[modify] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/LayoutTests/fast/events/mouseover-mouseout2.html
[modify] http://crrev.com/a1ca18019e2bb69732f97fe23d18b95c0fd7e3cd/third_party/WebKit/Source/core/input/EventHandler.cpp

Comment 17 by rbyers@google.com, Oct 5 2015

Labels: Hotlist-Input-Dev

Comment 18 by rbyers@chromium.org, Jun 4 2018

Labels: -Hotlist-GoogleApps Hotlist-Partner-GSuite

Sign in to add a comment