- Revision
- 211775
- Author
- carlo...@webkit.org
- Date
- 2017-02-06 22:45:43 -0800 (Mon, 06 Feb 2017)
Log Message
Overlay scrolling with iframe-s broken
https://bugs.webkit.org/show_bug.cgi?id=165056
Reviewed by Antonio Gomes.
Source/WebCore:
Mouse press events for overlay scrollbars are ignored if there's a subframe under the scrollbar. This doesn't
happen with normal scrollbars, because the subframe is not really under the scrollbar, so events are always
correctly passed to the scrollbar. With overlay scrollbars, the hit test detects the scrollbar, but events are
always passed first to the subframe. Scrollbars are correctly updated on hover though, because
handleMouseMoveEvent checks the presence of scrollbars before checking for subframes and move events are
actually passed to both, the scrollbar and the subframe. Overlay scrollbars should always take precedence over
subframes to handle mouse press events, so we should check first if mouse is over a scrollbar and never pass the
event to a subframe in that case. Another problem is that the cursor is not updated either when the overlay
scrollbar is hovered and there's a subframe. This is because in handleMouseMoveEvent we pass the event to both
the scrollbar and subframe but we never update the cursor when a suframe was found. So, here again we need to make
an exception for scrollbars and upate the cursor when mouse is over the scrollbar even if a subframe was found.
Test: fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html
* page/EventHandler.cpp:
(WebCore::EventHandler::handleMousePressEvent): Move the scrollbar check before the subframe check.
(WebCore::EventHandler::handleMouseMoveEvent): Update the cursor when hovering a scrollbar even if a subframe
was found.
(WebCore::EventHandler::updateLastScrollbarUnderMouse): Use an enum instead of a boolean for setLast parameter.
* page/EventHandler.h:
LayoutTests:
Add a new test to check that clicking on an overlay scrollbar works even it's over a subframe.
* fast/scrolling/scroll-animator-overlay-scrollbars-clicked-expected.txt: Added.
* fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html: Added.
* platform/ios-simulator/TestExpectations:
* platform/mac-wk1/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (211774 => 211775)
--- trunk/LayoutTests/ChangeLog 2017-02-07 06:34:26 UTC (rev 211774)
+++ trunk/LayoutTests/ChangeLog 2017-02-07 06:45:43 UTC (rev 211775)
@@ -1,3 +1,17 @@
+2017-02-06 Carlos Garcia Campos <cgar...@igalia.com>
+
+ Overlay scrolling with iframe-s broken
+ https://bugs.webkit.org/show_bug.cgi?id=165056
+
+ Reviewed by Antonio Gomes.
+
+ Add a new test to check that clicking on an overlay scrollbar works even it's over a subframe.
+
+ * fast/scrolling/scroll-animator-overlay-scrollbars-clicked-expected.txt: Added.
+ * fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html: Added.
+ * platform/ios-simulator/TestExpectations:
+ * platform/mac-wk1/TestExpectations:
+
2017-02-06 Myles C. Maxfield <mmaxfi...@apple.com>
Migrate ComplexTextController to use platform-independent types
Added: trunk/LayoutTests/fast/scrolling/scroll-animator-overlay-scrollbars-clicked-expected.txt (0 => 211775)
--- trunk/LayoutTests/fast/scrolling/scroll-animator-overlay-scrollbars-clicked-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/scroll-animator-overlay-scrollbars-clicked-expected.txt 2017-02-07 06:45:43 UTC (rev 211775)
@@ -0,0 +1,21 @@
+CONSOLE MESSAGE: line 14: MainFrameView: didAddVerticalScrollbar
+CONSOLE MESSAGE: line 14: MainFrameView: didAddHorizontalScrollbar
+CONSOLE MESSAGE: line 14: FrameView: didAddVerticalScrollbar
+CONSOLE MESSAGE: line 14: FrameView: willRemoveVerticalScrollbar
+CONSOLE MESSAGE: line 14: MainFrameView: mouseEnteredContentArea
+CONSOLE MESSAGE: line 14: MainFrameView: mouseMovedInContentArea
+CONSOLE MESSAGE: line 16: MainFrameView: mouseEnteredVerticalScrollbar
+CONSOLE MESSAGE: line 16: FrameView: mouseEnteredContentArea
+CONSOLE MESSAGE: line 16: MainFrameView: mouseMovedInContentArea
+CONSOLE MESSAGE: line 17: MainFrameView: mouseIsDownInVerticalScrollbar
+CONSOLE MESSAGE: line 18: MainFrameView: mouseIsUpInVerticalScrollbar
+CONSOLE MESSAGE: line 19: MainFrameView: mouseExitedVerticalScrollbar
+CONSOLE MESSAGE: line 19: FrameView: mouseExitedContentArea
+CONSOLE MESSAGE: line 19: MainFrameView: mouseMovedInContentArea
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Test for https://bugs.webkit.org/show_bug.cgi?id=165056.
+
+
+
Added: trunk/LayoutTests/fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html (0 => 211775)
--- trunk/LayoutTests/fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html 2017-02-07 06:45:43 UTC (rev 211775)
@@ -0,0 +1,31 @@
+<html>
+<script src=""
+<script>
+ if (window.internals) {
+ window.internals.setUsesMockScrollAnimator(true);
+ window.internals.setUsesOverlayScrollbars(true);
+ }
+ if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ }
+ window._onload_ = function () {
+ if (window.eventSender) {
+ eventSender.mouseMoveTo(0, 0);
+ // Move to a position that is over the vertical scrollbar and the iframe.
+ eventSender.mouseMoveTo(window.innerWidth - 4, 100);
+ eventSender.mouseDown();
+ eventSender.mouseUp();
+ eventSender.mouseMoveTo(0, 0);
+ }
+ if (window.testRunner)
+ testRunner.notifyDone();
+ };
+</script>
+<script src=""
+<body>
+ <p>Test for <a href=""
+ <iframe width="1024" height="768" id="frame" src=""
+ <pre id="console"></pre>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/ios-simulator/TestExpectations (211774 => 211775)
--- trunk/LayoutTests/platform/ios-simulator/TestExpectations 2017-02-07 06:34:26 UTC (rev 211774)
+++ trunk/LayoutTests/platform/ios-simulator/TestExpectations 2017-02-07 06:45:43 UTC (rev 211775)
@@ -313,6 +313,7 @@
loader/stateobjects/pushstate-size.html [ Skip ]
loader/stateobjects/pushstate-size-iframe.html [ Skip ]
fast/scrolling/scroll-animator-basic-events.html [ Skip ]
+fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html [ Skip ]
fast/scrolling/scroll-animator-overlay-scrollbars-hovered.html [ Skip ]
fast/scrolling/scroll-animator-select-list-events.html [ Skip ]
fast/events/prevent-default-prevents-interaction-with-scrollbars.html [ Skip ]
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (211774 => 211775)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2017-02-07 06:34:26 UTC (rev 211774)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2017-02-07 06:45:43 UTC (rev 211775)
@@ -197,7 +197,8 @@
inspector/unit-tests/heap-snapshot.html [ Skip ]
inspector/unit-tests/heap-snapshot-collection-event.html [ Skip ]
-# This test checks ScrollAnimator events only for main frame scrollbars that use native widgets in WK1.
+# These tests check ScrollAnimator events for main frame scrollbars that use native widgets in WK1.
+fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html [ Skip ]
fast/scrolling/scroll-animator-overlay-scrollbars-hovered.html [ Skip ]
# This hit-test test doesn't work on DRT
Modified: trunk/Source/WebCore/ChangeLog (211774 => 211775)
--- trunk/Source/WebCore/ChangeLog 2017-02-07 06:34:26 UTC (rev 211774)
+++ trunk/Source/WebCore/ChangeLog 2017-02-07 06:45:43 UTC (rev 211775)
@@ -1,3 +1,31 @@
+2017-02-06 Carlos Garcia Campos <cgar...@igalia.com>
+
+ Overlay scrolling with iframe-s broken
+ https://bugs.webkit.org/show_bug.cgi?id=165056
+
+ Reviewed by Antonio Gomes.
+
+ Mouse press events for overlay scrollbars are ignored if there's a subframe under the scrollbar. This doesn't
+ happen with normal scrollbars, because the subframe is not really under the scrollbar, so events are always
+ correctly passed to the scrollbar. With overlay scrollbars, the hit test detects the scrollbar, but events are
+ always passed first to the subframe. Scrollbars are correctly updated on hover though, because
+ handleMouseMoveEvent checks the presence of scrollbars before checking for subframes and move events are
+ actually passed to both, the scrollbar and the subframe. Overlay scrollbars should always take precedence over
+ subframes to handle mouse press events, so we should check first if mouse is over a scrollbar and never pass the
+ event to a subframe in that case. Another problem is that the cursor is not updated either when the overlay
+ scrollbar is hovered and there's a subframe. This is because in handleMouseMoveEvent we pass the event to both
+ the scrollbar and subframe but we never update the cursor when a suframe was found. So, here again we need to make
+ an exception for scrollbars and upate the cursor when mouse is over the scrollbar even if a subframe was found.
+
+ Test: fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html
+
+ * page/EventHandler.cpp:
+ (WebCore::EventHandler::handleMousePressEvent): Move the scrollbar check before the subframe check.
+ (WebCore::EventHandler::handleMouseMoveEvent): Update the cursor when hovering a scrollbar even if a subframe
+ was found.
+ (WebCore::EventHandler::updateLastScrollbarUnderMouse): Use an enum instead of a boolean for setLast parameter.
+ * page/EventHandler.h:
+
2017-02-06 Chris Dumez <cdu...@apple.com>
Symbols exposed on cross-origin Window / Location objects should be configurable
Modified: trunk/Source/WebCore/page/EventHandler.cpp (211774 => 211775)
--- trunk/Source/WebCore/page/EventHandler.cpp 2017-02-07 06:34:26 UTC (rev 211774)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2017-02-07 06:45:43 UTC (rev 211775)
@@ -1677,17 +1677,23 @@
m_mousePressNode = mouseEvent.targetNode();
m_frame.document()->setFocusNavigationStartingNode(mouseEvent.targetNode());
- RefPtr<Frame> subframe = subframeForHitTestResult(mouseEvent);
- if (subframe && passMousePressEventToSubframe(mouseEvent, subframe.get())) {
- // 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.
- m_capturesDragging = subframe->eventHandler().capturesDragging();
- if (m_mousePressed && m_capturesDragging) {
- m_capturingMouseEventsElement = subframe->ownerElement();
- m_eventHandlerWillResetCapturingMouseEventsElement = true;
+ Scrollbar* scrollbar = scrollbarForMouseEvent(mouseEvent, m_frame.view());
+ updateLastScrollbarUnderMouse(scrollbar, SetOrClearLastScrollbar::Set);
+ bool passedToScrollbar = scrollbar && passMousePressEventToScrollbar(mouseEvent, scrollbar);
+
+ if (!passedToScrollbar) {
+ RefPtr<Frame> subframe = subframeForHitTestResult(mouseEvent);
+ if (subframe && passMousePressEventToSubframe(mouseEvent, subframe.get())) {
+ // 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.
+ m_capturesDragging = subframe->eventHandler().capturesDragging();
+ if (m_mousePressed && m_capturesDragging) {
+ m_capturingMouseEventsElement = subframe->ownerElement();
+ m_eventHandlerWillResetCapturingMouseEventsElement = true;
+ }
+ invalidateClick();
+ return true;
}
- invalidateClick();
- return true;
}
#if ENABLE(PAN_SCROLLING)
@@ -1746,10 +1752,6 @@
mouseEvent = m_frame.document()->prepareMouseEvent(HitTestRequest(), documentPoint, platformMouseEvent);
}
- Scrollbar* scrollbar = scrollbarForMouseEvent(mouseEvent, m_frame.view());
- updateLastScrollbarUnderMouse(scrollbar, true);
-
- bool passedToScrollbar = scrollbar && passMousePressEventToScrollbar(mouseEvent, scrollbar);
if (!swallowEvent) {
if (passedToScrollbar)
swallowEvent = true;
@@ -1931,7 +1933,7 @@
m_resizeLayer->resize(platformMouseEvent, m_offsetFromResizeCorner);
else {
Scrollbar* scrollbar = mouseEvent.scrollbar();
- updateLastScrollbarUnderMouse(scrollbar, !m_mousePressed);
+ updateLastScrollbarUnderMouse(scrollbar, m_mousePressed ? SetOrClearLastScrollbar::Clear : SetOrClearLastScrollbar::Set);
// On iOS, our scrollbars are managed by UIKit.
#if !PLATFORM(IOS)
@@ -1959,7 +1961,9 @@
// node to be detached from its FrameView, in which case the event should not be passed.
if (newSubframe->view())
swallowEvent |= passMouseMoveEventToSubframe(mouseEvent, newSubframe.get(), hoveredNode);
- } else {
+ }
+
+ if (!newSubframe || mouseEvent.scrollbar()) {
#if ENABLE(CURSOR_SUPPORT)
if (auto* view = m_frame.view())
updateCursor(*view, mouseEvent.hitTestResult(), platformMouseEvent.shiftKey());
@@ -3825,9 +3829,8 @@
return scrollbar->mouseDown(mouseEvent.event());
}
-// If scrollbar (under mouse) is different from last, send a mouse exited. Set
-// last to scrollbar if setLast is true; else set last to nullptr.
-void EventHandler::updateLastScrollbarUnderMouse(Scrollbar* scrollbar, bool setLast)
+// If scrollbar (under mouse) is different from last, send a mouse exited.
+void EventHandler::updateLastScrollbarUnderMouse(Scrollbar* scrollbar, SetOrClearLastScrollbar setOrClear)
{
if (m_lastScrollbarUnderMouse != scrollbar) {
// Send mouse exited to the old scrollbar.
@@ -3835,12 +3838,10 @@
m_lastScrollbarUnderMouse->mouseExited();
// Send mouse entered if we're setting a new scrollbar.
- if (scrollbar && setLast)
+ if (scrollbar && setOrClear == SetOrClearLastScrollbar::Set) {
scrollbar->mouseEntered();
-
- if (setLast && scrollbar)
m_lastScrollbarUnderMouse = scrollbar->createWeakPtr();
- else
+ } else
m_lastScrollbarUnderMouse = nullptr;
}
}
Modified: trunk/Source/WebCore/page/EventHandler.h (211774 => 211775)
--- trunk/Source/WebCore/page/EventHandler.h 2017-02-07 06:34:26 UTC (rev 211774)
+++ trunk/Source/WebCore/page/EventHandler.h 2017-02-07 06:45:43 UTC (rev 211775)
@@ -443,7 +443,8 @@
void updateSelectionForMouseDrag(const HitTestResult&);
#endif
- void updateLastScrollbarUnderMouse(Scrollbar*, bool);
+ enum class SetOrClearLastScrollbar { Clear, Set };
+ void updateLastScrollbarUnderMouse(Scrollbar*, SetOrClearLastScrollbar);
void setFrameWasScrolledByUser();
