Changeset 177912 in webkit


Ignore:
Timestamp:
Jan 5, 2015 10:11:56 AM (9 years ago)
Author:
Brent Fulgham
Message:

[Mac] Cannot scroll when a non-scrollable iframe is contained inside a scrollable iframe
​https://bugs.webkit.org/show_bug.cgi?id=139914
<rdar://problem/18750910>

Reviewed by Darin Adler.

Source/WebCore:

Tests: platform/mac/fast/scrolling/scroll-nested-iframe.html

The latching logic was breaking down when a non-scrollable iframe was the closest target of
a wheel event. EventHandler would latch to the enclosing scrollable region (in this case, the
non-scrollable iframe) and would eat scroll events, preventing anything from working.

The fix is as follows:

  1. Modify the logic to understand a stack of latched states, so that we can discared 'invalid' latched states as we discover them.
  2. Revise the latching logic so that it understands the case where the 'latched' node for wheel events is in a parent frame of the current wheel event target. For example, when the mouse is over an element in an unscrollable iframe that is contained within a scrollable iframe. We should be latched to the scrollable iframe so events go to the right place.
  • page/EventHandler.cpp:

(WebCore::EventHandler::handleWheelEvent): Update to call new 'stack' versions of latch
state methods.
(WebCore::EventHandler::clearLatchedState): Ditto.
(WebCore::EventHandler::defaultWheelEventHandler): Ditto.

  • page/MainFrame.cpp: Update to store a stack of latched states. Provide methods to control

the lifetime of the stack and its elements.
(WebCore::MainFrame::MainFrame):
(WebCore::MainFrame::latchingState):
(WebCore::MainFrame::pushNewLatchingState):
(WebCore::MainFrame::resetLatchingState):
(WebCore::MainFrame::popLatchingState):

  • page/MainFrame.h:
  • page/mac/EventHandlerMac.mm:

(WebCore::latchingIsLockedToParentOfThisFrame): Added helper function.
(WebCore::EventHandler::platformPrepareForWheelEvents): Update to use new 'stack' style latch
methods. Also, if we are latched to a frame that contains the frame we are currently evaluating,
don't replace the current event target with the latched targets because (1) they will be processed
in the enclosing scope when we leave this routine, and (2) if we do change targets to the latched
elements we create an infinite loop.
(WebCore::EventHandler::platformCompleteWheelEvent): We want to mark the element as having started
at the scroll limit regardless of what the wheel event handler returns as its success state.
(WebCore::EventHandler::platformCompletePlatformWidgetWheelEvent): Revise to handle the new
stack-based latching methods.

LayoutTests:

  • platform/mac/fast/scrolling/scroll-nested-iframe-expected.txt: Added.
  • platform/mac/fast/scrolling/scroll-nested-iframe.html: Added.
  • platform/mac/fast/scrolling/resources/scroll_nested_iframe_test_inner.html: Added.
  • platform/mac/fast/scrolling/resources/scroll_nested_iframe_test_outer.html: Added.
Location:
trunk
Files:
5 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r177909 r177912  
     12014-12-23  Brent Fulgham  <bfulgham@apple.com>
     2
     3        [Mac] Cannot scroll when a non-scrollable iframe is contained inside a scrollable iframe
     4        https://bugs.webkit.org/show_bug.cgi?id=139914
     5        <rdar://problem/18750910>
     6
     7        Reviewed by Darin Adler.
     8
     9        * platform/mac/fast/scrolling/scroll-nested-iframe-expected.txt: Added.
     10        * platform/mac/fast/scrolling/scroll-nested-iframe.html: Added.
     11        * platform/mac/fast/scrolling/resources/scroll_nested_iframe_test_inner.html: Added.
     12        * platform/mac/fast/scrolling/resources/scroll_nested_iframe_test_outer.html: Added.
     13
    1142015-01-05  peavo@outlook.com  <peavo@outlook.com>
    215

  • trunk/Source/WebCore/ChangeLog

    r177911 r177912  
     12014-12-23  Brent Fulgham  <bfulgham@apple.com>
     2
     3        [Mac] Cannot scroll when a non-scrollable iframe is contained inside a scrollable iframe
     4        https://bugs.webkit.org/show_bug.cgi?id=139914
     5        <rdar://problem/18750910>
     6
     7        Reviewed by Darin Adler.
     8
     9        Tests: platform/mac/fast/scrolling/scroll-nested-iframe.html
     10
     11        The latching logic was breaking down when a non-scrollable iframe was the closest target of
     12        a wheel event. EventHandler would latch to the enclosing scrollable region (in this case, the
     13        non-scrollable iframe) and would eat scroll events, preventing anything from working.
     14       
     15        The fix is as follows:
     16        1. Modify the logic to understand a stack of latched states, so that we can discared 'invalid'
     17           latched states as we discover them.
     18        2. Revise the latching logic so that it understands the case where the 'latched' node for wheel
     19           events is in a parent frame of the current wheel event target. For example, when the mouse is over
     20           an element in an unscrollable iframe that is contained within a scrollable iframe. We should
     21           be latched to the scrollable iframe so events go to the right place.
     22
     23        * page/EventHandler.cpp:
     24        (WebCore::EventHandler::handleWheelEvent): Update to call new 'stack' versions of latch
     25        state methods.
     26        (WebCore::EventHandler::clearLatchedState): Ditto.
     27        (WebCore::EventHandler::defaultWheelEventHandler): Ditto.
     28        * page/MainFrame.cpp: Update to store a stack of latched states. Provide methods to control
     29        the lifetime of the stack and its elements.
     30        (WebCore::MainFrame::MainFrame):
     31        (WebCore::MainFrame::latchingState):
     32        (WebCore::MainFrame::pushNewLatchingState):
     33        (WebCore::MainFrame::resetLatchingState):
     34        (WebCore::MainFrame::popLatchingState):
     35        * page/MainFrame.h:
     36        * page/mac/EventHandlerMac.mm:
     37        (WebCore::latchingIsLockedToParentOfThisFrame): Added helper function.
     38        (WebCore::EventHandler::platformPrepareForWheelEvents): Update to use new 'stack' style latch
     39        methods. Also, if we are latched to a frame that contains the frame we are currently evaluating,
     40        don't replace the current event target with the latched targets because (1) they will be processed
     41        in the enclosing scope when we leave this routine, and (2) if we do change targets to the latched
     42        elements we create an infinite loop.
     43        (WebCore::EventHandler::platformCompleteWheelEvent): We want to mark the element as having started
     44        at the scroll limit regardless of what the wheel event handler returns as its success state.
     45        (WebCore::EventHandler::platformCompletePlatformWidgetWheelEvent): Revise to handle the new
     46        stack-based latching methods.
     47
    1482015-01-05  Darin Adler  <darin@apple.com>
    249

  • trunk/Source/WebCore/page/EventHandler.cpp

    r176753 r177912  
    26882688#if PLATFORM(MAC)
    26892689    if (event.phase() == PlatformWheelEventPhaseNone && event.momentumPhase() == PlatformWheelEventPhaseNone)
    2690         m_frame.mainFrame().latchingState()->clear();
     2690        m_frame.mainFrame().resetLatchingState();
    26912691#endif
    26922692
    … …  
    27352735{
    27362736#if PLATFORM(MAC)
    2737     m_frame.mainFrame().latchingState()->clear();
     2737    m_frame.mainFrame().resetLatchingState();
    27382738#endif
    27392739    m_frame.mainFrame().wheelEventDeltaTracker()->endTrackingDeltas();
    … …  
    27492749#if PLATFORM(MAC)
    27502750    ScrollLatchingState* latchedState = m_frame.mainFrame().latchingState();
    2751     ASSERT(latchedState);
    2752     Element* stopElement = latchedState->previousWheelScrolledElement();
     2751    Element* stopElement = latchedState ? latchedState->previousWheelScrolledElement() : nullptr;
    27532752
    27542753    // Workaround for scrolling issues <rdar://problem/14758615>.
    … …  
    27682767   
    27692768#if PLATFORM(MAC)
    2770     if (!latchedState->wheelEventElement())
     2769    if (latchedState && !latchedState->wheelEventElement())
    27712770        latchedState->setPreviousWheelScrolledElement(stopElement);
    27722771#endif

  • trunk/Source/WebCore/page/MainFrame.cpp

    r176499 r177912  
    2727#include "MainFrame.h"
    2828
     29#include "Element.h"
    2930#include "PageConfiguration.h"
    3031#include "PageOverlayController.h"
    … …  
    4243    , m_selfOnlyRefCount(0)
    4344#if PLATFORM(MAC)
    44     , m_latchingState(std::make_unique<ScrollLatchingState>())
    4545#if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
    4646    , m_servicesOverlayController(std::make_unique<ServicesOverlayController>(*this))
    … …  
    8989
    9090#if PLATFORM(MAC)
     91ScrollLatchingState* MainFrame::latchingState()
     92{
     93    if (m_latchingState.isEmpty())
     94        return nullptr;
     95
     96    return &m_latchingState.last();
     97}
     98
     99void MainFrame::pushNewLatchingState()
     100{
     101    m_latchingState.append(ScrollLatchingState());
     102}
     103
    91104void MainFrame::resetLatchingState()
    92105{
    93     if (!m_latchingState)
    94         return;
     106    m_latchingState.clear();
     107}
     108   
     109void MainFrame::popLatchingState()
     110{
     111    m_latchingState.removeLast();
     112}
    95113
    96     m_latchingState->clear();
    97 }
    98114#endif
    99115

  • trunk/Source/WebCore/page/MainFrame.h

    r176499 r177912  
    2828
    2929#include "Frame.h"
     30#include <wtf/Vector.h>
    3031
    3132namespace WebCore {
    … …  
    5556#endif // ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
    5657
    57     ScrollLatchingState* latchingState() { return m_latchingState.get(); }
     58    ScrollLatchingState* latchingState();
     59    void pushNewLatchingState();
     60    void popLatchingState();
    5861    void resetLatchingState();
    5962#endif // PLATFORM(MAC)
    … …  
    6972
    7073#if PLATFORM(MAC)
    71     std::unique_ptr<ScrollLatchingState> m_latchingState;
     74    Vector<ScrollLatchingState> m_latchingState;
    7275#if ENABLE(SERVICE_CONTROLS) || ENABLE(TELEPHONE_NUMBER_DETECTION)
    7376    std::unique_ptr<ServicesOverlayController> m_servicesOverlayController;

  • trunk/Source/WebCore/page/mac/EventHandlerMac.mm

    r177711 r177912  
    825825    return false;
    826826}
    827    
     827
     828static bool latchingIsLockedToAncestorOfThisFrame(const Frame& frame)
     829{
     830    ScrollLatchingState* latchedState = frame.mainFrame().latchingState();
     831    if (!latchedState || !latchedState->frame())
     832        return false;
     833
     834    if (&frame == latchedState->frame())
     835        return false;
     836
     837    for (Frame* ancestor = frame.tree().parent(); ancestor; ancestor->tree().parent()) {
     838        if (ancestor == latchedState->frame())
     839            return true;
     840    }
     841   
     842    return false;
     843}
     844
    828845void EventHandler::platformPrepareForWheelEvents(const PlatformWheelEvent& wheelEvent, const HitTestResult& result, RefPtr<Element>& wheelEventTarget, RefPtr<ContainerNode>& scrollableContainer, ScrollableArea*& scrollableArea, bool& isOverWidget)
    829846{
    … …  
    853870   
    854871    ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();
    855     ASSERT(latchingState);
    856872    if (wheelEvent.shouldConsiderLatching()) {
     873        m_frame.mainFrame().pushNewLatchingState();
     874        latchingState = m_frame.mainFrame().latchingState();
    857875        if (scrollableArea && scrollableContainer)
    858876            latchingState->setStartedGestureAtScrollLimit(scrolledToEdgeInDominantDirection(*scrollableContainer, *scrollableArea, wheelEvent.deltaX(), wheelEvent.deltaY()));
    … …  
    869887        clearLatchedState();
    870888
    871     if (!wheelEvent.shouldResetLatching() && latchingState->wheelEventElement()) {
     889    if (!wheelEvent.shouldResetLatching() && latchingState && latchingState->wheelEventElement()) {
    872890        if (latchingIsLockedToPlatformFrame(m_frame))
     891            return;
     892
     893        if (latchingIsLockedToAncestorOfThisFrame(m_frame))
    873894            return;
    874895
    … …  
    908929
    909930    ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();
    910     ASSERT(latchingState);
    911     if (wheelEvent.useLatchedEventElement() && latchingState->scrollableContainer()) {
     931    if (wheelEvent.useLatchedEventElement() && latchingState && latchingState->scrollableContainer()) {
    912932        view = frameViewForLatchingState(m_frame, latchingState);
    913933        if (!view || !view->frame().isMainFrame()) {
    914934            bool didHandleWheelEvent = view && view->wheelEvent(wheelEvent);
    915             if (!didHandleWheelEvent && scrollableContainer == latchingState->scrollableContainer()) {
     935            if (scrollableContainer == latchingState->scrollableContainer()) {
    916936                // If we are just starting a scroll event, and have nowhere left to scroll, allow
    917937                // the enclosing frame to handle the scroll.
    918938                didHandleWheelEvent = !latchingState->startedGestureAtScrollLimit();
    919939                if (!didHandleWheelEvent)
    920                     latchingState->setFrame(nullptr);
     940                    m_frame.mainFrame().popLatchingState();
    921941            }
    922942
    … …  
    951971
    952972    ScrollLatchingState* latchingState = m_frame.mainFrame().latchingState();
    953     ASSERT(latchingState);
     973    if (!latchingState)
     974        return false;
     975
    954976    if (wheelEvent.useLatchedEventElement() && latchingState->scrollableContainer() && scrollableContainer == latchingState->scrollableContainer())
    955977        return !latchingState->startedGestureAtScrollLimit();
Note: See TracChangeset for help on using the changeset viewer.