Skip to content

GitLab

T
tangled
Commit cd3e5fb1 authored by Robert Flack's avatar Robert Flack Committed by Commit Bot
Browse files
Options

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

This reverts commit df72f9b5.

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/1330329Reviewed-by: default avatarRobert Flack <flackr@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607044}
parent 47b8dc1d
Hide whitespace changes
Inline Side-by-side
Showing with 0 additions and 435 deletions
third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-iframe.html deleted 100644 → 0
View file @ 47b8dc1d
<!doctype html>
<meta charset="utf-8">
<!-- This test will check whether an iframe that doesn't contain the drag
source, moving via DOM manipulation, will reset the drag source and
potentially cancel dragend event emission. -->
<title>Drag and Drop: Iframe DOM Move</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/interaction.html#drag-and-drop-processing-model">
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../resources/drag-trigger-dom-move.js"></script>
<style>
.box {
display: block;
border: 1px solid black;
width: 350px;
height: 200px;
text-align: center;
}
</style>
<p>
Please drag the "Drag Me" box into the "Drop Here" box.
</p>
<div id="drag-box" class="dragged box" draggable="true">
Drag me
</div>
<div id="drop-box" class="dropzone box">
Drop Here
</div>
<div id="moved-item-source" class="box">
<iframe id="outer-iframe" data-source="iframe-srcdoc"></iframe>
</div>
<div id="moved-item-destination" class="box"></div>
<script id="iframe-srcdoc" language="text/html">
<!doctype html>
<meta charset="utf-8">
<div/>
</script>
<p>
Current test: <code id="test-description"></code>
</p>
<script>
dragDomMoveTests([
{ load: 'iframe', expectDragEnd: true, action: 'appendChild' },
{ load: 'iframe', expectDragEnd: true, action: 'removeChild' },
]);
</script>
third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-image.html deleted 100644 → 0
View file @ 47b8dc1d
<!doctype html>
<meta charset="utf-8">
<!-- This test will check whether an image moving via DOM manipulation will
reset the drag source and potentially cancel dragend event emission. -->
<title>Drag and Drop: Image DOM Move</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/interaction.html#drag-and-drop-processing-model">
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../resources/drag-trigger-dom-move.js"></script>
<style>
.box {
display: block;
border: 1px solid black;
width: 350px;
height: 200px;
text-align: center;
}
</style>
<p>
Please drag the "Drag Me" box into the "Drop Here" box.
</p>
<div id="drag-box" class="dragged box" draggable="true">
Drag me
</div>
<div id="drop-box" class="dropzone box">
Drop Here
</div>
<div id="moved-item-source" class="box"></div>
<div id="moved-item-destination" class="box"></div>
<p>
Current test: <code id="test-description"></code>
</p>
<script>
dragDomMoveTests([
{ load: 'image', expectDragEnd: true, action: 'removeChild' },
{ load: 'image', expectDragEnd: true, action: 'appendChild' },
]);
</script>
third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframe.html deleted 100644 → 0
View file @ 47b8dc1d
<!doctype html>
<meta charset="utf-8">
<!-- This test will check whether an iframe that contains the drag source,
moving via DOM manipulation, will reset the drag source and potentially
cancel dragend event emission. -->
<title>Drag and Drop: Nested Iframe DOM Move</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/interaction.html#drag-and-drop-processing-model">
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../resources/drag-trigger-dom-move.js"></script>
<style>
.box {
display: block;
border: 1px solid black;
width: 350px;
height: 200px;
text-align: center;
}
</style>
<div id="moved-item-source" class="dragged box">
<iframe id="outer-iframe" data-source="iframe-srcdoc"></iframe>
</div>
<div id="drop-box" class="dropzone box">
Drop Here
</div>
<p>
Please drag the "Drag Me" into the "Drop Here" box.
</p>
<div id="moved-item-destination" class="box"></div>
<script id="iframe-srcdoc" language="text/html">
<!doctype html>
<meta charset="utf-8">
<style>
.box {
display: block;
border: 1px solid black;
width: 250px;
height: 100px;
text-align: center;
}
</style>
<div id="drag-box" class="box" draggable="true">
<p>Drag me!</p>
</div>
</script>
<p>
Current test: <code id="test-description"></code>
</p>
<script>
dragDomMoveTests([
{ load: 'nested iframe', expectDragEnd: false,
action: 'appendChild' },
{ load: 'nested iframe', expectDragEnd: false,
action: 'removeChild' },
]);
</script>
third_party/WebKit/LayoutTests/fast/dnd/dragtriggerdommove/drag-trigger-dom-move-nested-iframes.html deleted 100644 → 0
View file @ 47b8dc1d
<!doctype html>
<meta charset="utf-8">
<!-- This test will check whether an iframe that contains an iframe containing
the drag source, moving via DOM manipulation, will reset the drag source
and potentially cancel dragend event emission. -->
<title>Drag and Drop: Nested Iframe DOM Move</title>
<link rel="help" href="https://html.spec.whatwg.org/multipage/interaction.html#drag-and-drop-processing-model">
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../resources/drag-trigger-dom-move.js"></script>
<style>
.box {
display: block;
border: 1px solid black;
width: 350px;
height: 200px;
text-align: center;
}
</style>
<div id="moved-item-source" class="dragged box">
<iframe id="outer-iframe" data-source="iframe-srcdoc"></iframe>
</div>
<div id="drop-box" class="dropzone box">
Drop Here
</div>
<p>
Please drag the "Drag Me" into the "Drop Here" box.
</p>
<div id="moved-item-destination" class="box"></div>
<script id="iframe-srcdoc" language="text/html">
<!doctype html>
<meta charset="utf-8">
<iframe id="inner-iframe" data-source="inner-iframe-srcdoc"/>
</script>
<script id="inner-iframe-srcdoc" language="text/html">
<!doctype html>
<meta charset="utf-8">
<style>
.box {
display: block;
border: 1px solid black;
width: 250px;
height: 100px;
text-align: center;
}
</style>
<div id="drag-box" class="box" draggable="true">
<p>Drag me!</p>
</div>
</script>
<p>
Current test: <code id="test-description"></code>
</p>
<script>
dragDomMoveTests([
{ load: 'doubly nested iframe', expectDragEnd: false,
action: 'appendChild' },
{ load: 'doubly nested iframe', expectDragEnd: false,
action: 'removeChild' },
]);
</script>
third_party/WebKit/LayoutTests/fast/dnd/resources/drag-trigger-dom-move.js deleted 100644 → 0
View file @ 47b8dc1d
'use strict'
/** Moves the mouse to the center of |element|. */
const mouseMoveToCenter = element => {
const clientRect = element.getBoundingClientRect();
const centerX = (clientRect.left + clientRect.right) / 2;
const centerY = (clientRect.top + clientRect.bottom) / 2;
if (window.eventSender)
eventSender.mouseMoveTo(centerX, centerY);
};
/**
* Recursively loads content into a series of nested iframes.
* Returns a Promise that resolves with the HTMLDocument of the innermost frame.
*/
const loadNestedFrames = async domRoot => {
const frame = domRoot.querySelector('iframe');
if (!frame)
return domRoot;
const htmlSourceId = frame.getAttribute('data-source');
frame.srcdoc = document.getElementById(htmlSourceId).textContent;
const frameLoaded = new Promise(resolve => { frame.onload = resolve; });
await frameLoaded;
return await loadNestedFrames(frame.contentDocument);
};
/** Retrieves an element with id in an arbitrarily deep nesting of iframes. */
const getElementByIdAcrossIframes = (domRoot, id) => {
if (!domRoot)
return null;
const dragBox = domRoot.getElementById(id);
if (dragBox)
return dragBox;
return getElementByIdAcrossIframes(
domRoot.querySelector('iframe').contentDocument, id);
};
const loadIframe = async () => {
await loadNestedFrames(document);
return document.getElementById('outer-iframe');
};
const loadImage = async () => {
const image = document.createElement('img');
image.src = '../resources/greenbox.png';
const imageLoaded = new Promise(resolve => {
image.onload = resolve(image);
document.getElementById('moved-item-source').appendChild(image);
});
return await imageLoaded;
};
const loadMovedItem = async loadItem => {
if (loadItem.includes('iframe'))
return await loadIframe();
else if (loadItem.includes('image'))
return await loadImage();
};
/**
* Test if moving an element (iframe or image) will cancel dragging by
* resetting drag source.
* testCase: a testCase to test, containing a specified type to load and
* whether or not dragend is expected to fire, as well as the action
* to attempt.
*/
const dragDomMoveTest = testCase => {
promise_test(async t => {
document.querySelector('#test-description').textContent =
JSON.stringify(testCase);
const gotEvent = {
dragStart: false,
dragOver: false,
drop: false,
dragEnd: false,
};
let movedItem = await loadMovedItem(testCase.load);
const dragBox = getElementByIdAcrossIframes(document, 'drag-box');
const dropBox = document.getElementById('drop-box');
const doneButton = document.createElement('button');
dragBox.ondragstart = t.step_func(e => {
gotEvent.dragStart = true;
e.dataTransfer.setData('text/plain', 'Needed to work in Firefox');
});
dropBox.ondragover = t.step_func(e => {
gotEvent.dragOver = true;
e.preventDefault();
});
const dndTest = new Promise(resolve => {
dragBox.ondragend = t.step_func(e => {
gotEvent.dragEnd = true;
return resolve();
});
dropBox.ondrop = t.step_func(async e => {
gotEvent.drop = true;
e.preventDefault();
const movedItemDestination =
document.getElementById('moved-item-destination');
const movedItemSource =
document.getElementById('moved-item-source');
// Test whether dragging away or detaching movedItem
// will disable dragging.
if (testCase.action == 'removeChild')
movedItem = movedItem.parentNode.removeChild(movedItem);
else if (testCase.action == 'appendChild')
movedItemDestination.appendChild(movedItem);
else
return reject("Error: Invalid testCase.action. Please make sure the testCase is spelled correctly");
// Click to resolve test as backup in case dragend never triggers to
// end the test.
setTimeout(() => {
const clickEvent = new Event('click');
doneButton.dispatchEvent(clickEvent);
}, 100);
// Reset iframe location to teardown and prep for next test.
if (testCase.load.includes('iframe')) {
const movedItemLoaded = new Promise(resolve => {
movedItem.onload = resolve;
setTimeout(() => { movedItemSource.appendChild(movedItem); }, 100);
});
await movedItemLoaded;
}
});
doneButton.onclick = t.step_func(() => {
return resolve();
})
// Do drag and drop.
if (window.eventSender) {
mouseMoveToCenter(dragBox);
eventSender.mouseDown();
setTimeout(() => {
mouseMoveToCenter(dropBox);
eventSender.mouseUp();
}, 100);
}
});
await dndTest;
assert_true(gotEvent.dragStart,
'drag-box should have gotten a dragstart event');
assert_true(gotEvent.dragOver,
'drop-box should have gotten a dragover event');
assert_true(gotEvent.drop,
'drop-box should have gotten a drop event');
assert_equals(gotEvent.dragEnd, testCase.expectDragEnd,
'drag-box should have gotten a dragEnd event');
}, `tested with input: ${testCase.load}, ${testCase.action}`);
};
const dragDomMoveTests = testCases => {
for (let testCase of testCases)
dragDomMoveTest(testCase);
promise_test(() => {
return Promise.resolve().then(() => {
document.querySelector('#test-description').textContent = 'done';
});
}, 'all tests complete');
}
third_party/blink/renderer/core/frame/local_frame.cc
View file @ cd3e5fb1
...@@ -493,10 +493,6 @@ void LocalFrame::DidAttachDocument() { ...@@ -493,10 +493,6 @@ void LocalFrame::DidAttachDocument() {
Document* document = GetDocument(); Document* document = GetDocument();
DCHECK(document); DCHECK(document);
GetEditor().Clear(); GetEditor().Clear();
// Clearing the event handler clears many events, but notably can ensure that
// for a drag started on an element in a frame that was moved (likely via
// appendChild()), the drag source will detach and stop firing drag events
// even after the frame reattaches.
GetEventHandler().Clear(); GetEventHandler().Clear();
Selection().DidAttachDocument(document); Selection().DidAttachDocument(document);
GetInputMethodController().DidAttachDocument(document); GetInputMethodController().DidAttachDocument(document);
......
third_party/blink/renderer/core/input/mouse_event_manager.cc
View file @ cd3e5fb1
...@@ -1100,24 +1100,8 @@ DragState& MouseEventManager::GetDragState() { ...@@ -1100,24 +1100,8 @@ DragState& MouseEventManager::GetDragState() {
} }
void MouseEventManager::ResetDragSource() { void MouseEventManager::ResetDragSource() {
// Check validity of drag source.
if (!frame_->GetPage()) if (!frame_->GetPage())
return; return;
Node* drag_src = GetDragState().drag_src_;
if (!drag_src)
return;
Frame* drag_src_frame = drag_src->GetDocument().GetFrame();
if (!drag_src_frame)
return;
// Only allow resetting drag_src_ if the frame requesting reset is above the
// drag_src_ node's frame in the frame hierarchy. This way, unrelated frames
// can't reset a drag state.
if (!drag_src_frame->Tree().IsDescendantOf(frame_))
return;
GetDragState().drag_src_ = nullptr; GetDragState().drag_src_ = nullptr;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment