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

Issue 737691 link

Starred by 30 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

dragEnd event not fired when iFrame is moved in the DOM

Reported by jadayt...@gmail.com, Jun 28 2017

Issue description

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

Steps to reproduce the problem:
I have a JSFiddle that demonstrates the issue here: https://jsfiddle.net/r8wxpujg/1/

1. In the drop function of an HTML5 drag and drop operation, move an iframe around in the dom.
2. Notice that the dragend event is not fired.

What is the expected behavior?
The dragend event should be fired on the element that was dragged. This is the actual behavior in Firefox and IE, but not in Chrome and Opera.

What went wrong?
The dragend event is never fired. 

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 59.0.3071.109  Channel: n/a
OS Version: 
Flash Version: 

I posted a stackoverflow question about this a couple of days ago in the hope that I could verify it was a Chromium bug before reporting it, but I got no response and have decided to go ahead and report it.

https://stackoverflow.com/questions/44764343/dragend-event-not-firing-in-chrome-when-an-iframe-is-moved-in-drop-function
 
Labels: Needs-Triage-M59
Labels: Needs-Feedback
Tested this issue using latest stable #59.0.3071.115, reported version #59.0.3071.109 and was unable to reprodcue the issue as per the steps mentioned in original comment.

@jadaytime: Please find the attched screen cast and let us know your observations.

Thanks!!
Jun 29 2017 3-07 PM.webm
3.6 MB View Download
Correction
=========
Tested this issue using latest stable #59.0.3071.115, reported version #59.0.3071.109 on Linux Ubuntu 14.04, and was unable to reprodcue the issue as per the steps mentioned in original comment.

Comment 4 by jadayt...@gmail.com, Jun 29 2017

Sorry, my explanation might not have been very clear. The demo defaults to using an <img> tag where the expected behavior is the actual behavior. You need to click the toggle img or iframe button once to switch to an iframe where the presumed bug is present. I have attached a screencast myself in order to demonstrate.
iframe.webm
98.4 KB View Download
Project Member

Comment 5 by sheriffbot@chromium.org, Jun 29 2017

Cc: sandeepkumars@chromium.org
Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "sandeepkumars@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by jsb...@chromium.org, Jun 30 2017

Status: Available (was: Unconfirmed)
Yep, confirmed. Thanks for the nice repro!

Tossing onto the DataTransfer backlog.

Comment 7 by bjrma...@gmail.com, Jul 28 2017

hi, just FYI, seems like the issue is present also in Chrome (>=59) for Windows (tested with Windows 10 PRO) and in Chrome for Mac OS (tested with El Capitan)


Comment 8 Deleted

I've been struggling with a very similar problem - the root cause is potentially the same. I have a node I'm dragging which I detach from the dom if it drags successfully. And in some cases, this causes dragend never to fire.

The workaround I've found is to use a setTimeout to wait to detach the dom node until after the dragend event has fired. Very annoying - causes artifacts to freeze on the page in my app. Please fix this!
I traced my issue to what sounds like probably the exact same issue. I was having issues where dragend would never fire if I created an iframe and attached it to the dom in a 'drop' event or anytime before 'dragend' was supposed to fire. Here's a JS fiddle for that:

https://jsfiddle.net/r8wxpujg/4/

Any updates on fixing this? There is no good way to work around this other than doing weird setTimeouts to delay the creation or manipulation of iframes.

Comment 11 by jer...@lytmus.com, Aug 17 2017

The `drop` event still seems to fire with an iframe inside of the dragged item.  

I was able to use that event to do what I needed to do, though this may or may not work for your situation. 
FWIW, this bug only affects Chrome. I tested this across all major browsers/platforms.
This also happens with 
<embed src="http://google.de" width=200 height=200 />
<object data="http://google.de" width="400" height="300" type="text/html"></object>

is there no workaround??


I think this is a good workaround.
https://github.com/ManifestWebDesign/angular-gridster/commit/fd0e107c03755a3474c23ff54bd8089a8d78a387

Its a example for this plugin but I think you can do this for all other plugins you need :-)
Cc: huangdarwin@chromium.org
Cc: -huangdarwin@chromium.org
Owner: huangdarwin@chromium.org
Status: Started (was: Available)
Starting
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df72f9b57cbf37f32f291add8a6f658068924a9a

commit df72f9b57cbf37f32f291add8a6f658068924a9a
Author: Darwin Huang <huangdarwin@chromium.org>
Date: Thu Nov 08 01:36:40 2018

Drag and Drop: Frames not containing drag source shouldn't be able to reset drag state

Only allow frames who are ancestors of the drag_src_ to reset a drag_src_

Created layout test to show previous lack of dragend when moving iframe in dom as a result of dragging
Created layout test to verify lack of regression when frames containing drag source are moved/detached

Bug:  737691 
Change-Id: Ic6cc8ac0528d35ac93a21612b453d6b3203a152d
Reviewed-on: https://chromium-review.googlesource.com/c/1265818
Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606276}
[add] https://crrev.com/df72f9b57cbf37f32f291add8a6f658068924a9a/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-iframe.html
[add] https://crrev.com/df72f9b57cbf37f32f291add8a6f658068924a9a/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-image.html
[add] https://crrev.com/df72f9b57cbf37f32f291add8a6f658068924a9a/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframe.html
[add] https://crrev.com/df72f9b57cbf37f32f291add8a6f658068924a9a/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframes.html
[add] https://crrev.com/df72f9b57cbf37f32f291add8a6f658068924a9a/third_party/WebKit/LayoutTests/fast/dnd/resources/drag-trigger-dom-move.js
[modify] https://crrev.com/df72f9b57cbf37f32f291add8a6f658068924a9a/third_party/blink/renderer/core/frame/local_frame.cc
[modify] https://crrev.com/df72f9b57cbf37f32f291add8a6f658068924a9a/third_party/blink/renderer/core/input/mouse_event_manager.cc

Status: Fixed (was: Started)
Fantastic. Thanks for the fix! When can we expect it to hit the Release channel? Thanks again.
Should be in M72 which should hit stable at the end of January 2019.
Project Member

Comment 21 by bugdroid1@chromium.org, Nov 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd3e5fb1f8d2b4106dc5e1805ad8c8108f08fc83

commit cd3e5fb1f8d2b4106dc5e1805ad8c8108f08fc83
Author: Robert Flack <flackr@chromium.org>
Date: Sat Nov 10 00:15:13 2018

Revert "Drag and Drop: Frames not containing drag source shouldn't be able to reset drag state"

This reverts commit df72f9b57cbf37f32f291add8a6f658068924a9a.

Reason for revert: This caused a memory leak detected by the WebKit Linux Trusty Leak bot:
https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/

Original change's description:
> Drag and Drop: Frames not containing drag source shouldn't be able to reset drag state
> 
> Only allow frames who are ancestors of the drag_src_ to reset a drag_src_
> 
> Created layout test to show previous lack of dragend when moving iframe in dom as a result of dragging
> Created layout test to verify lack of regression when frames containing drag source are moved/detached
> 
> Bug:  737691 
> Change-Id: Ic6cc8ac0528d35ac93a21612b453d6b3203a152d
> Reviewed-on: https://chromium-review.googlesource.com/c/1265818
> Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
> Reviewed-by: Victor Costan <pwnall@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606276}

TBR=dcheng@chromium.org,pwnall@chromium.org,huangdarwin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  737691 
Change-Id: I560230c44bbcc055632abcd81b3d6286f972f530
Reviewed-on: https://chromium-review.googlesource.com/c/1330329
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607044}
[delete] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-iframe.html
[delete] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-image.html
[delete] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframe.html
[delete] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframes.html
[delete] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/third_party/WebKit/LayoutTests/fast/dnd/resources/drag-trigger-dom-move.js
[modify] https://crrev.com/cd3e5fb1f8d2b4106dc5e1805ad8c8108f08fc83/third_party/blink/renderer/core/frame/local_frame.cc
[modify] https://crrev.com/cd3e5fb1f8d2b4106dc5e1805ad8c8108f08fc83/third_party/blink/renderer/core/input/mouse_event_manager.cc

Status: Started (was: Fixed)
Sorry, I'll fix the memory leak and re-land the fix asap.
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2a3ca40202b5875266f6eecbd7f5103363303302

commit 2a3ca40202b5875266f6eecbd7f5103363303302
Author: Darwin Huang <huangdarwin@chromium.org>
Date: Wed Nov 21 11:31:09 2018

Drag and Drop: Frames not containing drag source shouldn't be able to reset
drag state

Only allow frames who are ancestors of the drag_src_ to reset a drag_src_

Created layout test to show previous lack of dragend when moving iframe in dom
as a result of dragging
Created layout test to verify lack of regression when frames containing drag
source are moved/detached

Fix memory leak from reverted CL https://crrev.com/c/1265818

Bug:  737691 ,  903705 ,  903933 
Change-Id: I308680446662d6548587ae2d7dd2c139b09ee581
Reviewed-on: https://chromium-review.googlesource.com/c/1336440
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610001}
[add] https://crrev.com/2a3ca40202b5875266f6eecbd7f5103363303302/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-iframe.html
[add] https://crrev.com/2a3ca40202b5875266f6eecbd7f5103363303302/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-image.html
[add] https://crrev.com/2a3ca40202b5875266f6eecbd7f5103363303302/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframe.html
[add] https://crrev.com/2a3ca40202b5875266f6eecbd7f5103363303302/third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframes.html
[add] https://crrev.com/2a3ca40202b5875266f6eecbd7f5103363303302/third_party/WebKit/LayoutTests/fast/dnd/resources/drag-trigger-dom-move.js
[modify] https://crrev.com/2a3ca40202b5875266f6eecbd7f5103363303302/third_party/blink/renderer/core/frame/local_frame.cc
[modify] https://crrev.com/2a3ca40202b5875266f6eecbd7f5103363303302/third_party/blink/renderer/core/input/mouse_event_manager.cc

Status: Fixed (was: Started)
Sorry for the delay. The memory leak has been fixed, and the fix of the last fix has been landed. Let's hope it sticks this time.

Sign in to add a comment