Bug 191607 – Default the view background color and text color to different values when in dark mode

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Timothy Hatcher]

The view background color should not be white. And the text color should not be black. They should be [NSColor controlBackgroundColor] and [NSColor textColor].

[Radar WebKit Bug Importer]

<rdar://problem/46045854>

[Timothy Hatcher]

Created attachment 354724 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 354724 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354724&action=review

> Source/WebCore/css/html.css:41
> +    color: text;

Is this still black in light mode?

Also weird that we do something in the UA stylesheet which there is no standard way for authors to do in their pages.

> Source/WebCore/page/FrameView.cpp:3005
> +    Color backgroundColor = transparent ? Color::transparent : RenderTheme::singleton().systemColor(CSSValueAppleSystemControlBackground, styleColorOptions());

What's the value of CSSValueAppleSystemControlBackground in light mode? White, or are you affecting the default background color?

[Timothy Hatcher]

Comment on attachment 354724 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354724&action=review

>> Source/WebCore/css/html.css:41
>> +    color: text;
> 
> Is this still black in light mode?
> 
> Also weird that we do something in the UA stylesheet which there is no standard way for authors to do in their pages.

Yes, it is black in light mode. Test checks that.

>> Source/WebCore/page/FrameView.cpp:3005
>> +    Color backgroundColor = transparent ? Color::transparent : RenderTheme::singleton().systemColor(CSSValueAppleSystemControlBackground, styleColorOptions());
> 
> What's the value of CSSValueAppleSystemControlBackground in light mode? White, or are you affecting the default background color?

It is pure white in light mode. Test checks that.

[Timothy Hatcher]

Created attachment 354743 [details]
Patch

[Timothy Hatcher]

Created attachment 354745 [details]
Patch

[EWS Watchlist]

Comment on attachment 354745 [details]
Patch

Attachment 354745 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9981131

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 354751 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 354745 [details]
Patch

Attachment 354745 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9982311

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 354762 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 354745 [details]
Patch

Attachment 354745 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9983195

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 354766 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Timothy Hatcher]

Created attachment 354774 [details]
Patch

[Timothy Hatcher]

Created attachment 354775 [details]
Patch

[EWS Watchlist]

Comment on attachment 354775 [details]
Patch

Attachment 354775 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9985211

New failing tests:
inspector/css/shadow-scoped-style.html
printing/width-overflow.html

[EWS Watchlist]

Created attachment 354781 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 354775 [details]
Patch

Attachment 354775 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/9985376

New failing tests:
inspector/css/shadow-scoped-style.html
printing/width-overflow.html

[EWS Watchlist]

Created attachment 354785 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 354775 [details]
Patch

Attachment 354775 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9988557

New failing tests:
inspector/css/shadow-scoped-style.html
printing/width-overflow.html

[EWS Watchlist]

Created attachment 354810 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Timothy Hatcher]

Created attachment 354825 [details]
Patch

[Timothy Hatcher]

Created attachment 354833 [details]
Patch

[Timothy Hatcher]

Created attachment 354834 [details]
Patch

[zalan]

Created attachment 354835 [details]
Failing test (also on shipping Safari)

The test actually fails already on shipping Safari.

[Simon Fraser (smfr)]

Comment on attachment 354834 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354834&action=review

> Source/WebCore/css/html.css:29
> +    color: text;

Does this mean that our default text color is now semantic, and will not get color-filter applied?

[Timothy Hatcher]

Comment on attachment 354834 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354834&action=review

>> Source/WebCore/css/html.css:29
>> +    color: text;
> 
> Does this mean that our default text color is now semantic, and will not get color-filter applied?

Yes. The color-filter skips it, but it still auto switches with light and dark mode — yielding the same white text as the Mail color-filter.

[WebKit Commit Bot]

Comment on attachment 354834 [details]
Patch

Clearing flags on attachment: 354834

Committed r238212: <https://trac.webkit.org/changeset/238212>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

Looks like https://trac.webkit.org/changeset/238212/webkit

has caused two API failures on MacOS

build: https://build.webkit.org/builders/Apple%20Sierra%20Release%20WK2%20%28Tests%29/builds/12799

Failed

    TestWebKitAPI.WebKit.BackgroundColorNil
        
        /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/mac/BackgroundColor.mm:77
        Expected equality of these values:
          CGColorGetConstantColor(kCGColorWhite)
            Which is: 0x7f922871dee0
          [webView layer].backgroundColor
            Which is: 0x7f9228530400
        

    TestWebKitAPI.WebKit.BackgroundColorDefault
        
        /Volumes/Data/slave/sierra-release/build/Tools/TestWebKitAPI/Tests/mac/BackgroundColor.mm:47
        Expected equality of these values:
          CGColorGetConstantColor(kCGColorWhite)
            Which is: 0x7fb24d4f5ff0
          [webView layer].backgroundColor
            Which is: 0x7fb2520065d0

[Truitt Savell]

Additionally Mojave has this api failure in addition:

Failed

    TestWebKitAPI.WebKit.BackgroundColorNoDrawsBackground
        _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
        
        /Volumes/Data/slave/mojave-release/build/Tools/TestWebKitAPI/Tests/mac/BackgroundColor.mm:87
        Expected equality of these values:
          [NSColor whiteColor]
            Which is: 0x7f9562574c20
          backgroundColor
            Which is: 0x7f9562574440

[Timothy Hatcher]

Oops. I’ll get these fixed in the next hour.

[Timothy Hatcher]

Fix for API tests is bug 191697.