Skip to content

Commit

Permalink
REGRESSION (Safari 17.2): Does not respect mousemove events in an ifr…
Browse files Browse the repository at this point in the history
…ame when the mouse is clicked from outside of an iframe

https://bugs.webkit.org/show_bug.cgi?id=269886
rdar://120540148

Reviewed by Wenson Hsieh.

In 269246@main, we introduced a behavior change where initiating a drag
gesture from a cancelled mousedown event causes all mousemove events to
be routed to the originating frame till the drag gesture terminates with
a corresponding mouseup event. This was a web compatibility regression
since users expect to receive mouse events in the inner frame when we
are dragging the pointer over it.

Instead, this patch limits this mouse event target capturing behavior to
subframes only, and not the outer frame. This patch makes mouse event
targets independent of mousedown event cancellation when the pointer is
dragged out of a subframe. This change in behavior aligns us with other
major browser vendors' behavior.

Finally, we update cancel-mousedown-and-drag-to-frame.html since the new
behavior is essentially the opposite of what the test had been
expecting.

* LayoutTests/fast/events/cancel-mousedown-and-drag-from-frame-to-other-frame.html:
* LayoutTests/fast/events/cancel-mousedown-and-drag-to-frame-expected.txt:
* LayoutTests/fast/events/cancel-mousedown-and-drag-to-frame.html:
* LayoutTests/fast/events/resources/mouse-drag-from-frame-target-subframe.html:
* Source/WebCore/page/EventHandler.cpp:
(WebCore::EventHandler::handleMousePressEvent):

Canonical link: https://commits.webkit.org/275157@main
  • Loading branch information
aprotyas committed Feb 22, 2024
1 parent 0ee25f1 commit 50f43b7
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
testRunner.dumpAsText();
}

var unexpectedMouseMove = false;
var unexpectedMouseUp = false;
var mouseMoveInOtherFrame = false;
var mouseUpInOtherFrame = false;

function log(msg) {
var msgNode = document.createTextNode(msg);
Expand Down Expand Up @@ -40,7 +40,7 @@
eventSender.mouseMoveTo(endX, endY);
eventSender.mouseUp();
} finally {
if (unexpectedMouseUp || unexpectedMouseMove)
if (mouseUpInOtherFrame || mouseMoveInOtherFrame)
log("FAIL! Received unexpected mouse event on other frame.");
else
log("PASS!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ This tests that dragging from an element that returns false from its mousedown h
Drag Me!

Drag started
Root frame received mouse move
Root frame received mouseup event
Subframe received mousemove event
Subframe received mouseup event
PASS!
30 changes: 10 additions & 20 deletions LayoutTests/fast/events/cancel-mousedown-and-drag-to-frame.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,37 +7,22 @@
testRunner.dumpAsText();
}

var mouseMoveInOtherFrame = false;
var mouseUpInOtherFrame = false;

function log(msg) {
var msgNode = document.createTextNode(msg);
var li = document.createElement("li");
li.appendChild(msgNode);
document.getElementById("log").appendChild(li);
}
var dragging = false;
var waitingForUp = false;

function dragStarted() {
if (dragging || waitingForUp) {
log("Unexpected drag start");
return;
}
log("Drag started");
dragging = true;
}

window.onmousemove = function() {
if (!dragging || waitingForUp)
return;
log("Root frame received mouse move");
waitingForUp = true;
}

window.onmouseup = function() {
if (!waitingForUp) {
log("Unexpected mouseup");
return;
}
log("Root frame received mouseup event");
log("PASS!");
log("FAIL! Received unexpected mouseup on root frame");
}

window.onload = function() {
Expand All @@ -61,6 +46,11 @@
eventSender.mouseMoveTo(endX, endY);
eventSender.mouseUp();
} finally {
if (mouseUpInOtherFrame && mouseMoveInOtherFrame)
log("PASS!");
else
log("FAIL! Drag was not reflected in subframe");

if (window.testRunner)
testRunner.notifyDone();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<body onmousemove="parent.log('Received unexpected mousemove'); parent.unexpectedMouseMove = true;" onmouseup="parent.log('Received unexpected mouseup'); parent.unexpectedMouseUp = true;">
<body onmousemove="parent.log('Subframe received mousemove event'); parent.mouseMoveInOtherFrame = true;" onmouseup="parent.log('Subframe received mouseup event'); parent.mouseUpInOtherFrame = true;">

</body>
24 changes: 8 additions & 16 deletions Source/WebCore/page/EventHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1860,28 +1860,20 @@ HandleUserInputEventResult EventHandler::handleMousePressEvent(const PlatformMou
if (auto remoteMouseEventData = userInputEventDataForRemoteFrame(dynamicDowncast<RemoteFrame>(subframe).get(), mouseEvent.hitTestResult().roundedPointInInnerNodeFrame()))
return *remoteMouseEventData;

auto captureMouseEventsElementIfNeeded = [&](RefPtr<Element>&& element = nullptr) {
if (m_mousePressed && (m_capturesDragging || m_capturesDragging.inabilityReason() == CapturesDragging::InabilityReason::MousePressIsCancelled)) {
if (element)
m_capturingMouseEventsElement = WTFMove(element);
else
m_isCapturingRootElementForMouseEvents = true;
m_eventHandlerWillResetCapturingMouseEventsElement = true;
}
};
RefPtr localSubframe = dynamicDowncast<LocalFrame>(subframe.get());
if (localSubframe && passMousePressEventToSubframe(mouseEvent, *localSubframe)) {
// Start capturing future events for this frame. We only do this if we didn't clear
// the m_mousePressed flag, which may happen if an AppKit widget entered a modal event loop.
if (auto subframeCapturesDragging = localSubframe->eventHandler().capturesDragging())
m_capturesDragging = true;
else
m_capturesDragging = subframeCapturesDragging.inabilityReason();
captureMouseEventsElementIfNeeded(localSubframe->protectedOwnerElement().get());
m_capturesDragging = localSubframe->eventHandler().capturesDragging();
if (m_mousePressed) {
m_capturingMouseEventsElement = localSubframe->ownerElement();
m_eventHandlerWillResetCapturingMouseEventsElement = true;
if (!m_capturingMouseEventsElement)
m_isCapturingRootElementForMouseEvents = true;
}
invalidateClick();
return true;
} else
captureMouseEventsElementIfNeeded();
}
}

#if ENABLE(PAN_SCROLLING)
Expand Down

0 comments on commit 50f43b7

Please sign in to comment.