⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 287777 in webkit


Ignore:
Timestamp:
Jan 7, 2022, 12:28:04 PM (5 years ago)
Author:
Wenson Hsieh
Message:

Teach modal container observer to make the body element scrollable if necessary
​https://bugs.webkit.org/show_bug.cgi?id=234708
rdar://86960677

Reviewed by Tim Horton.

Source/WebCore:

Add a mechanism to allow ModalContainerObserver to force the body and/or document elements in the main document
to become vertically scrollable, in the case where a modal container has been detected.

In particular, if a modal container has already been detected and hidden away, the frame is non-scrollable, and
the body and/or document element satisfies both conditions:

  1. Has a height that is taller than the visible height of the top FrameView
  2. Has overflow-y: hidden;

...then we'll flag one or both of those elements and force them to be vertically scrollable during style
adjustment (i.e. return true from shouldMakeVerticallyScrollable()).

Covered by augmenting an existing API test:
ModalContainerObservation.HideUserInteractionBlockingElementAndMakeDocumentScrollable

  • page/ModalContainerObserver.cpp:

(WebCore::ModalContainerObserver::setContainer):
(WebCore::ModalContainerObserver::collectClickableElementsTimerFired):
(WebCore::ModalContainerObserver::makeBodyAndDocumentElementScrollableIfNeeded):

Helper method that contains logic for overriding scrollability on the body or html element, if needed.

(WebCore::ModalContainerObserver::clearScrollabilityOverrides):
(WebCore::ModalContainerObserver::hideUserInteractionBlockingElementIfNeeded):

Drive-by fix: target is just a raw pointer here, so just assign it directly to foundElement instead of
trying to use move semantics.

(WebCore::ModalContainerObserver::revealModalContainer):
(WebCore::ModalContainerObserver::shouldMakeVerticallyScrollable const):

Add a helper method (similar to shouldHide()) that can be used to make elements vertically scrollable during
style adjustment time. See above for more details.

(WebCore::ModalContainerObserver::tryToMakeBodyAndDocumentElementScrollableThroughQuirks):

  • page/ModalContainerObserver.h:
  • style/StyleAdjuster.cpp:

(WebCore::Style::Adjuster::adjust const):

Consult shouldMakeVerticallyScrollable in addition to shouldHide if ModalContainerObserver is present.

Tools:

Adjust an existing API test to exercise the change.

  • TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html:
  • TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:

(TestWebKitAPI::TEST):

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r287772 r287777  
     12022-01-07  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Teach modal container observer to make the body element scrollable if necessary
     4        https://bugs.webkit.org/show_bug.cgi?id=234708
     5        rdar://86960677
     6
     7        Reviewed by Tim Horton.
     8
     9        Add a mechanism to allow ModalContainerObserver to force the body and/or document elements in the main document
     10        to become vertically scrollable, in the case where a modal container has been detected.
     11
     12        In particular, if a modal container has already been detected and hidden away, the frame is non-scrollable, and
     13        the body and/or document element satisfies both conditions:
     14
     15        1. Has a height that is taller than the visible height of the top FrameView
     16        2. Has `overflow-y: hidden;`
     17
     18        ...then we'll flag one or both of those elements and force them to be vertically scrollable during style
     19        adjustment (i.e. return `true` from `shouldMakeVerticallyScrollable()`).
     20
     21        Covered by augmenting an existing API test:
     22        ModalContainerObservation.HideUserInteractionBlockingElementAndMakeDocumentScrollable
     23
     24        * page/ModalContainerObserver.cpp:
     25        (WebCore::ModalContainerObserver::setContainer):
     26        (WebCore::ModalContainerObserver::collectClickableElementsTimerFired):
     27        (WebCore::ModalContainerObserver::makeBodyAndDocumentElementScrollableIfNeeded):
     28
     29        Helper method that contains logic for overriding scrollability on the body or html element, if needed.
     30
     31        (WebCore::ModalContainerObserver::clearScrollabilityOverrides):
     32        (WebCore::ModalContainerObserver::hideUserInteractionBlockingElementIfNeeded):
     33
     34        Drive-by fix: `target` is just a raw pointer here, so just assign it directly to `foundElement` instead of
     35        trying to use move semantics.
     36
     37        (WebCore::ModalContainerObserver::revealModalContainer):
     38        (WebCore::ModalContainerObserver::shouldMakeVerticallyScrollable const):
     39
     40        Add a helper method (similar to `shouldHide()`) that can be used to make elements vertically scrollable during
     41        style adjustment time. See above for more details.
     42
     43        (WebCore::ModalContainerObserver::tryToMakeBodyAndDocumentElementScrollableThroughQuirks):
     44        * page/ModalContainerObserver.h:
     45        * style/StyleAdjuster.cpp:
     46        (WebCore::Style::Adjuster::adjust const):
     47
     48        Consult `shouldMakeVerticallyScrollable` in addition to `shouldHide` if `ModalContainerObserver` is present.
     49
    1502022-01-07  Antti Koivisto  <antti@apple.com>
    251

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

    r287671 r287777  
    263263            return;
    264264
    265         if (auto observer = container->document().modalContainerObserverIfExists(); observer && container == observer->container())
    266             observer->hideUserInteractionBlockingElementIfNeeded();
     265        auto observer = container->document().modalContainerObserverIfExists();
     266        if (!observer || container != observer->container())
     267            return;
     268
     269        observer->hideUserInteractionBlockingElementIfNeeded();
     270        observer->makeBodyAndDocumentElementScrollableIfNeeded();
    267271    });
    268272}
    … …  
    539543                    return;
    540544
    541                 if (RefPtr controlToClick = classifiedControls.controlToClick(decision))
     545                if (RefPtr controlToClick = classifiedControls.controlToClick(decision)) {
     546                    observer->clearScrollabilityOverrides(*document);
    542547                    controlToClick->dispatchSimulatedClick(nullptr, SendMouseUpDownEvents, DoNotShowPressedLook);
     548                }
    543549
    544550                decisionScope.continueHidingModalContainerAfterScope();
    … …  
    548554}
    549555
     556void ModalContainerObserver::makeBodyAndDocumentElementScrollableIfNeeded()
     557{
     558    if (!container())
     559        return;
     560
     561    Ref document = container()->document();
     562    RefPtr view = document->view();
     563    if (!view || view->isScrollable())
     564        return;
     565
     566    document->updateLayoutIgnorePendingStylesheets();
     567
     568    auto visibleHeight = view->visibleSize().height();
     569    auto shouldMakeElementScrollable = [visibleHeight] (Element* element) {
     570        if (!element)
     571            return false;
     572
     573        auto renderer = element->renderer();
     574        if (!renderer || renderer->style().overflowY() != Overflow::Hidden)
     575            return false;
     576
     577        return element->boundingClientRect().height() > visibleHeight;
     578    };
     579
     580    if (!m_makeBodyElementScrollable) {
     581        if (RefPtr body = document->body(); shouldMakeElementScrollable(body.get())) {
     582            m_makeBodyElementScrollable = true;
     583            body->invalidateStyle();
     584        }
     585    }
     586
     587    if (!m_makeDocumentElementScrollable) {
     588        if (RefPtr documentElement = document->documentElement(); shouldMakeElementScrollable(documentElement.get())) {
     589            m_makeDocumentElementScrollable = true;
     590            documentElement->invalidateStyle();
     591        }
     592    }
     593}
     594
     595void ModalContainerObserver::clearScrollabilityOverrides(Document& document)
     596{
     597    if (std::exchange(m_makeBodyElementScrollable, false)) {
     598        if (auto element = document.body())
     599            element->invalidateStyle();
     600    }
     601
     602    if (std::exchange(m_makeDocumentElementScrollable, false)) {
     603        if (auto element = document.documentElement())
     604            element->invalidateStyle();
     605    }
     606}
     607
    550608void ModalContainerObserver::hideUserInteractionBlockingElementIfNeeded()
    551609{
    552     if (m_userInteractionBlockingElement) {
    553         ASSERT_NOT_REACHED();
    554         return;
    555     }
     610    if (m_userInteractionBlockingElement)
     611        return;
    556612
    557613    RefPtr container = this->container();
    … …  
    594650
    595651        if (!foundElement)
    596             foundElement = WTFMove(target);
     652            foundElement = target;
    597653    }
    598654
    … …  
    604660{
    605661    auto [container, frameOwner] = std::exchange(m_containerAndFrameOwnerForControls, { });
    606     if (container)
     662    if (container) {
    607663        container->invalidateStyle();
     664        clearScrollabilityOverrides(container->document());
     665    }
    608666
    609667    if (auto element = std::exchange(m_userInteractionBlockingElement, { }))
    … …  
    661719}
    662720
     721bool ModalContainerObserver::shouldMakeVerticallyScrollable(const Element& element) const
     722{
     723    if (m_makeBodyElementScrollable && element.document().body() == &element)
     724        return true;
     725
     726    if (m_makeDocumentElementScrollable && element.document().documentElement() == &element)
     727        return true;
     728
     729    return false;
     730}
     731
    663732} // namespace WebCore

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

    r287671 r287777  
    5151    ~ModalContainerObserver();
    5252
     53    bool shouldMakeVerticallyScrollable(const Element&) const;
    5354    inline bool shouldHide(const Element&) const;
    5455    void updateModalContainerIfNeeded(const FrameView&);
    … …  
    6465    void setContainer(Element&, HTMLFrameOwnerElement* = nullptr);
    6566    void searchForModalContainerOnBehalfOfFrameOwnerIfNeeded(HTMLFrameOwnerElement&);
     67
     68    void makeBodyAndDocumentElementScrollableIfNeeded();
     69    void clearScrollabilityOverrides(Document&);
    6670
    6771    Element* container() const;
    … …  
    7983    bool m_collectingClickableElements { false };
    8084    bool m_hasAttemptedToFulfillPolicy { false };
     85    bool m_makeBodyElementScrollable { false };
     86    bool m_makeDocumentElementScrollable { false };
    8187};
    8288

  • trunk/Source/WebCore/style/StyleAdjuster.cpp

    r287742 r287777  
    554554
    555555    if (m_element) {
    556         if (auto observer = m_element->document().modalContainerObserver(); observer && observer->shouldHide(*m_element))
    557             style.setDisplay(DisplayType::None);
     556        if (auto observer = m_element->document().modalContainerObserverIfExists()) {
     557            if (observer->shouldHide(*m_element))
     558                style.setDisplay(DisplayType::None);
     559            if (observer->shouldMakeVerticallyScrollable(*m_element))
     560                style.setOverflowY(Overflow::Auto);
     561        }
    558562    }
    559563

  • trunk/Tools/ChangeLog

    r287773 r287777  
     12022-01-07  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Teach modal container observer to make the body element scrollable if necessary
     4        https://bugs.webkit.org/show_bug.cgi?id=234708
     5        rdar://86960677
     6
     7        Reviewed by Tim Horton.
     8
     9        Adjust an existing API test to exercise the change.
     10
     11        * TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html:
     12        * TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm:
     13        (TestWebKitAPI::TEST):
     14
    1152022-01-07  Ryan Haddad  <ryanhaddad@apple.com>
    216

  • trunk/Tools/TestWebKitAPI/Tests/WebKit/modal-container-with-overlay.html

    r287671 r287777  
    11<!DOCTYPE html>
    2 <html>
     2<html style="overflow: hidden;">
    33<head>
    44<meta name="viewport" content="width=device-width, initial-scale=1">
    … …  
    88    background-color: #EFEFEF;
    99    font-family: system-ui;
     10}
     11
     12body {
     13    height: 3000px;
    1014}
    1115
    … …  
    5963</script>
    6064</head>
    61 <body>
     65<body style="overflow: hidden;">
    6266<p id="content">Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.</p>
    6367<div id="overlay"></div>
    … …  
    6872<script>
    6973button.addEventListener("click", () => {
     74    document.body.style.removeProperty("overflow");
     75    document.documentElement.style.removeProperty("overflow");
    7076    fixed.remove();
    7177    overlay.remove();

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalContainerObservation.mm

    r287671 r287777  
    257257}
    258258
    259 TEST(ModalContainerObservation, HideUserInteractionBlockingElement)
     259TEST(ModalContainerObservation, HideUserInteractionBlockingElementAndMakeDocumentScrollable)
    260260{
    261261    auto webView = createModalContainerWebView();
    … …  
    267267    NSString *hitTestedText = [webView stringByEvaluatingJavaScript:@"document.elementFromPoint(50, 50).textContent"];
    268268    EXPECT_TRUE([hitTestedText containsString:@"Lorem"]);
     269    EXPECT_WK_STREQ("auto", [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.documentElement).overflowY"]);
     270    EXPECT_WK_STREQ("auto", [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.body).overflowY"]);
    269271}
    270272
Note: See TracChangeset for help on using the changeset viewer.