Overflow element scrollbar is light for dark mode content.
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2019 23:34:26 +0000 (23:34 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2019 23:34:26 +0000 (23:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194407
rdar://problem/45991585

Reviewed by Beth Dakin.

Source/WebCore:

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:

LayoutTests:

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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241150 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/css-dark-mode/supported-color-schemes-scrollbar-expected.txt
LayoutTests/css-dark-mode/supported-color-schemes-scrollbar.html
Source/WebCore/ChangeLog
Source/WebCore/page/ChromeClient.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 8d67bb2..e25ee51 100644 (file)
@@ -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 Beth Dakin.
+
+        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-02-07  Nikita Vasilyev  <nvasilyev@apple.com>
 
         Web Inspector: Fix modify-css-property-race.html flakiness
index 074a0db..3b726b4 100644 (file)
@@ -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 
 
index b7d037b..e24a8ce 100644 (file)
@@ -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>
index ffaa3e6..c8e4288 100644 (file)
@@ -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 Beth Dakin.
+
+        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-02-07  Eric Carlson  <eric.carlson@apple.com>
 
         [MSE] Convert debug-only logging to runtime logging
index ec11807..4cb98c0 100644 (file)
@@ -406,7 +406,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;
         
index 5a9ded2..0c13a7a 100644 (file)
@@ -364,7 +364,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());
index cc1ce5d..47a9e4d 100644 (file)
@@ -3242,6 +3242,11 @@ bool RenderLayer::hasScrollableOrRubberbandableAncestor()
     return false;
 }
 
+bool RenderLayer::useDarkAppearance() const
+{
+    return renderer().useDarkAppearance();
+}
+
 #if ENABLE(CSS_SCROLL_SNAP)
 void RenderLayer::updateSnapOffsets()
 {
index cbaf79d..88b3255 100644 (file)
@@ -445,6 +445,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
index 63d9af0..ff35c56 100644 (file)
@@ -2552,14 +2552,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:
@@ -2572,6 +2594,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();
index 37c5490..5a2ccdb 100644 (file)
@@ -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;
index 0e74659..df16b5f 100644 (file)
@@ -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().