Attachment 361428 Details for Bug 194407 – Patch

[patch] Patch

bug-194407-20190207123459.patch (text/plain), 13.24 KB, created by Timothy Hatcher on 2019-02-07 12:34:59 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Timothy Hatcher

Created: 2019-02-07 12:34:59 PST

Size: 13.24 KB

patch

obsolete

>Subversion Revision: 240122
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 088803b78eb67f7472e7b9e25209a47d6902c9ef..ceef91908a1bfb12da161852812726dc878d8ab9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-02-07  Timothy Hatcher  <timothy@apple.com>
>+
>+        Overflow element scrollbar is light for dark mode content.
>+        https://bugs.webkit.org/show_bug.cgi?id=194407
>+        rdar://problem/45991585
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Tested by css-dark-mode/supported-color-schemes-scrollbar.html.
>+
>+        * page/ChromeClient.h:
>+        (WebCore::FrameView::preferredScrollbarOverlayStyle): Return WTF::nullopt by default to avoid
>+        short-circuiting auto detection in recalculateScrollbarOverlayStyle() for clients, like WK1,
>+        that do not implement preferredScrollbarOverlayStyle().
>+        * page/FrameView.cpp:
>+        (WebCore::FrameView::recalculateScrollbarOverlayStyle): Use WTF::nullopt in the false case
>+        to auto detect overlay style when page() is null.
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::useDarkAppearance const): Added.
>+        * rendering/RenderLayer.h:
>+        * testing/Internals.cpp:
>+        (WebCore::Internals::scrollbarOverlayStyle const): Added Node argument.
>+        (WebCore::Internals::scrollbarUsingDarkAppearance const): Added.
>+        * testing/Internals.h:
>+        * testing/Internals.idl:
>+
> 2019-01-17  Jiewen Tan  <jiewen_tan@apple.com>
> 
>         [Mac] Add a new quirk to HTMLFormControlElement::isMouseFocusable
>diff --git a/Source/WebCore/page/ChromeClient.h b/Source/WebCore/page/ChromeClient.h
>index 32040e15bf8505e70096cb653ddeea77f634caaf..072a84ea93ef1e0844c86bccb738129ec9ccf656 100644
>--- a/Source/WebCore/page/ChromeClient.h
>+++ b/Source/WebCore/page/ChromeClient.h
>@@ -403,7 +403,7 @@ public:
>     virtual void notifyScrollerThumbIsVisibleInRect(const IntRect&) { }
>     virtual void recommendedScrollbarStyleDidChange(ScrollbarStyle) { }
> 
>-    virtual Optional<ScrollbarOverlayStyle> preferredScrollbarOverlayStyle() { return ScrollbarOverlayStyleDefault; }
>+    virtual Optional<ScrollbarOverlayStyle> preferredScrollbarOverlayStyle() { return WTF::nullopt; }
> 
>     virtual void wheelEventHandlersChanged(bool hasHandlers) = 0;
>         
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index 8dd924d797c1673bd07cc84268bb48be155d9487..7051ba25fc5630eb15d7b3088ffbcc11f4af2fe4 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -390,7 +390,7 @@ void FrameView::detachCustomScrollbars()
> void FrameView::recalculateScrollbarOverlayStyle()
> {
>     ScrollbarOverlayStyle oldOverlayStyle = scrollbarOverlayStyle();
>-    Optional<ScrollbarOverlayStyle> clientOverlayStyle = frame().page() ? frame().page()->chrome().client().preferredScrollbarOverlayStyle() : ScrollbarOverlayStyleDefault;
>+    Optional<ScrollbarOverlayStyle> clientOverlayStyle = frame().page() ? frame().page()->chrome().client().preferredScrollbarOverlayStyle() : WTF::nullopt;
>     if (clientOverlayStyle) {
>         if (clientOverlayStyle.value() != oldOverlayStyle)
>             setScrollbarOverlayStyle(clientOverlayStyle.value());
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 072e937bf789e210906d20d4c9d3c3ff85887018..78730d40abe840f011056ba48ed6c6855fb82827 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -3244,6 +3244,11 @@ bool RenderLayer::hasScrollableOrRubberbandableAncestor()
>     return false;
> }
> 
>+bool RenderLayer::useDarkAppearance() const
>+{
>+    return renderer().useDarkAppearance();
>+}
>+
> #if ENABLE(CSS_SCROLL_SNAP)
> void RenderLayer::updateSnapOffsets()
> {
>diff --git a/Source/WebCore/rendering/RenderLayer.h b/Source/WebCore/rendering/RenderLayer.h
>index 78ff638c1984aec767d01f751f5d15be07960bf4..1be73e1a73b3022411c1982dfed2f630081af782 100644
>--- a/Source/WebCore/rendering/RenderLayer.h
>+++ b/Source/WebCore/rendering/RenderLayer.h
>@@ -438,6 +438,7 @@ public:
>     ScrollableArea* enclosingScrollableArea() const override;
>     bool isScrollableOrRubberbandable() override;
>     bool hasScrollableOrRubberbandableAncestor() override;
>+    bool useDarkAppearance() const final;
> #if ENABLE(CSS_SCROLL_SNAP)
>     void updateSnapOffsets() override;
> #endif
>diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp
>index 3099971f337f6090f842a4ff921f5b5c923bc668..e6ef748524a5455c6256c93df1edcdbce5a306d4 100644
>--- a/Source/WebCore/testing/Internals.cpp
>+++ b/Source/WebCore/testing/Internals.cpp
>@@ -2542,14 +2542,36 @@ ExceptionOr<String> Internals::repaintRectsAsText() const
>     return document->frame()->trackedRepaintRectsAsText();
> }
> 
>-ExceptionOr<String> Internals::scrollbarOverlayStyle() const
>+ExceptionOr<String> Internals::scrollbarOverlayStyle(Node* node) const
> {
>-    Document* document = contextDocument();
>-    if (!document || !document->view())
>+    if (!node)
>+        node = contextDocument();
>+
>+    if (!node)
>         return Exception { InvalidAccessError };
> 
>-    auto& frameView = *document->view();
>-    switch (frameView.scrollbarOverlayStyle()) {
>+    node->document().updateLayoutIgnorePendingStylesheets();
>+
>+    ScrollableArea* scrollableArea = nullptr;
>+    if (is<Document>(*node)) {
>+        auto* frameView = downcast<Document>(node)->view();
>+        if (!frameView)
>+            return Exception { InvalidAccessError };
>+
>+        scrollableArea = frameView;
>+    } else if (is<Element>(*node)) {
>+        auto& element = *downcast<Element>(node);
>+        if (!element.renderBox())
>+            return Exception { InvalidAccessError };
>+
>+        scrollableArea = element.renderBox()->layer();
>+    } else
>+        return Exception { InvalidNodeTypeError };
>+
>+    if (!scrollableArea)
>+        return Exception { InvalidNodeTypeError };
>+
>+    switch (scrollableArea->scrollbarOverlayStyle()) {
>     case ScrollbarOverlayStyleDefault:
>         return "default"_str;
>     case ScrollbarOverlayStyleDark:
>@@ -2562,6 +2584,38 @@ ExceptionOr<String> Internals::scrollbarOverlayStyle() const
>     return "unknown"_str;
> }
> 
>+ExceptionOr<bool> Internals::scrollbarUsingDarkAppearance(Node* node) const
>+{
>+    if (!node)
>+        node = contextDocument();
>+
>+    if (!node)
>+        return Exception { InvalidAccessError };
>+
>+    node->document().updateLayoutIgnorePendingStylesheets();
>+
>+    ScrollableArea* scrollableArea = nullptr;
>+    if (is<Document>(*node)) {
>+        auto* frameView = downcast<Document>(node)->view();
>+        if (!frameView)
>+            return Exception { InvalidAccessError };
>+
>+        scrollableArea = frameView;
>+    } else if (is<Element>(*node)) {
>+        auto& element = *downcast<Element>(node);
>+        if (!element.renderBox())
>+            return Exception { InvalidAccessError };
>+
>+        scrollableArea = element.renderBox()->layer();
>+    } else
>+        return Exception { InvalidNodeTypeError };
>+
>+    if (!scrollableArea)
>+        return Exception { InvalidNodeTypeError };
>+
>+    return scrollableArea->useDarkAppearance();
>+}
>+
> ExceptionOr<String> Internals::scrollingStateTreeAsText() const
> {
>     Document* document = contextDocument();
>diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h
>index f70b7b6b93e7bf017364036002da4686a6eece94..f777bb8298c95abf6f4efe73966ceb3d5bffadb2 100644
>--- a/Source/WebCore/testing/Internals.h
>+++ b/Source/WebCore/testing/Internals.h
>@@ -352,7 +352,8 @@ public:
>     ExceptionOr<uint64_t> layerIDForElement(Element&);
>     ExceptionOr<String> repaintRectsAsText() const;
> 
>-    ExceptionOr<String> scrollbarOverlayStyle() const;
>+    ExceptionOr<String> scrollbarOverlayStyle(Node*) const;
>+    ExceptionOr<bool> scrollbarUsingDarkAppearance(Node*) const;
> 
>     ExceptionOr<String> scrollingStateTreeAsText() const;
>     ExceptionOr<String> mainThreadScrollingReasons() const;
>diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl
>index 0e746595da3af637cb7611324f69b8f20f33d04d..df16b5f2382603f3c4f838d7cb6b9eeb01ff9e43 100644
>--- a/Source/WebCore/testing/Internals.idl
>+++ b/Source/WebCore/testing/Internals.idl
>@@ -376,7 +376,8 @@ enum CompositingPolicy {
> 
>     [MayThrowException] unsigned long long layerIDForElement(Element element);
> 
>-    [MayThrowException] DOMString scrollbarOverlayStyle();
>+    [MayThrowException] DOMString scrollbarOverlayStyle(optional Node? node = null);
>+    [MayThrowException] boolean scrollbarUsingDarkAppearance(optional Node? node = null);
> 
>     [MayThrowException] DOMString scrollingStateTreeAsText();
>     [MayThrowException] DOMString mainThreadScrollingReasons(); // FIXME: rename to synchronousScrollingReasons().
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f3795236f90def9634bdae62857680d715f1e860..a631ee12dbb39dca4d7b0b2e0f2434c65c4ba221 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-02-07  Timothy Hatcher  <timothy@apple.com>
>+
>+        Overflow element scrollbar is light for dark mode content.
>+        https://bugs.webkit.org/show_bug.cgi?id=194407
>+        rdar://problem/45991585
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Updated tests to look at overflow elements and if dark apearance
>+        is used by the scrollbar directly.
>+
>+        * css-dark-mode/supported-color-schemes-scrollbar-expected.txt:
>+        * css-dark-mode/supported-color-schemes-scrollbar.html:
>+
> 2019-01-16  Dean Jackson  <dino@apple.com>
> 
>         (iPad) Link tapping is sluggish on many sites
>diff --git a/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar-expected.txt b/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar-expected.txt
>index 074a0dbfa99085c2fbcab3db1bca1892ad7557c0..3b726b4983c7bf42dfa528bd09b0210ea04fe802 100644
>--- a/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar-expected.txt
>+++ b/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar-expected.txt
>@@ -3,9 +3,15 @@ PASS Set dark appearance
> PASS Set view to transparent 
> PASS Body Element supported color scheme is light and dark 
> PASS Document Element supported color scheme is auto 
>-PASS Scrollbar overlay style is light 
>+PASS Document scrollbar overlay style is light 
>+PASS Document scrollbar is using dark appearance 
>+PASS Element scrollbar overlay style is default 
>+PASS Element scrollbar is using dark appearance 
> PASS Set prefers-color-schemes: light on the document element 
> PASS Body Element supported color scheme is light and dark 
> PASS Document Element supported color scheme is light 
>-PASS Scrollbar overlay style is default 
>+PASS Document scrollbar overlay style is default 
>+PASS Document scrollbar is using light appearance 
>+PASS Element scrollbar overlay style is default 
>+PASS Element scrollbar is using dark appearance 
> 
>diff --git a/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar.html b/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar.html
>index b7d037b44c7b289c1d26b6d71d7eb84480a95d1d..e24a8ce9743b0584fac76f475bf8d33e2e6f1eed 100644
>--- a/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar.html
>+++ b/LayoutTests/css-dark-mode/supported-color-schemes-scrollbar.html
>@@ -9,9 +9,21 @@
> body {
>     supported-color-schemes: light dark;
> }
>+
>+#test {
>+    overflow-x: hidden;
>+    overflow-y: scroll;
>+    width: 100px;
>+    height: 50px;
>+}
>+
>+#test-content {
>+    width: 100px;
>+    height: 100px;
>+}
> </style>
> 
>-<body></body>
>+<body><div id="test"><div id="test-content"></div></div></body>
> 
> <script>
> function test_prop(element, prop, expected) {
>@@ -42,7 +54,25 @@ test(function() {
>     if (!window.internals)
>         return;
>     assert_equals(internals.scrollbarOverlayStyle(), "light");
>-}, "Scrollbar overlay style is light");
>+}, "Document scrollbar overlay style is light");
>+
>+test(function() {
>+    if (!window.internals)
>+        return;
>+    assert_equals(internals.scrollbarUsingDarkAppearance(), true);
>+}, "Document scrollbar is using dark appearance");
>+
>+test(function() {
>+    if (!window.internals)
>+        return;
>+    assert_equals(internals.scrollbarOverlayStyle(document.getElementById("test")), "default");
>+}, "Element scrollbar overlay style is default");
>+
>+test(function() {
>+    if (!window.internals)
>+        return;
>+    assert_equals(internals.scrollbarUsingDarkAppearance(document.getElementById("test")), true);
>+}, "Element scrollbar is using dark appearance");
> 
> test(function() {
>     let styleElement = document.createElement("style");
>@@ -62,5 +92,23 @@ test(function() {
>     if (!window.internals)
>         return;
>     assert_equals(internals.scrollbarOverlayStyle(), "default");
>-}, "Scrollbar overlay style is default");
>+}, "Document scrollbar overlay style is default");
>+
>+test(function() {
>+    if (!window.internals)
>+        return;
>+    assert_equals(internals.scrollbarUsingDarkAppearance(), false);
>+}, "Document scrollbar is using light appearance");
>+
>+test(function() {
>+    if (!window.internals)
>+        return;
>+    assert_equals(internals.scrollbarOverlayStyle(document.getElementById("test")), "default");
>+}, "Element scrollbar overlay style is default");
>+
>+test(function() {
>+    if (!window.internals)
>+        return;
>+    assert_equals(internals.scrollbarUsingDarkAppearance(document.getElementById("test")), true);
>+}, "Element scrollbar is using dark appearance");
> </script>

Actions: View | Formatted Diff | Diff

Attachments on bug 194407: 361428