[Webkit-unassigned] [Bug 197947] [GTK] Should use light theme unless website declares support for dark themes in color-schemes property

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=197947

--- Comment #23 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Michael Catanzaro from comment #21)
> (In reply to Michael Catanzaro from comment #20)
> > Thanks Carlos, this was a huge help. I'll test it soon to verify it fixes
> > older unpatched Evolution, and to check out its behavior in Cassidy's test
> > cases.
> 
> OK, it fixes the Evolution/Geary regression as expected. Great!

\o/

> It also fixes Cassidy's third test in comment #3, dark style, which was
> previously broken.
> 
> It badly fails Cassidy's first two tests though (neither of which passed
> before).

They work fine here, I'm getting the same results than the screenshots (expect the WebKitGTK and Firefox on Linux, of course).

> Test #1 (no style) is now worse than before because it now uses
> black text (as desired) instead of light text (not desired) on a dark
> background (not desired).

Background shouldn't be dark, it's white here.

> Test #2 (some style) is a little better than
> before, but still pretty bad. We need to pass these tests to be
> web-compatible. The problem is that you've now fixed the text color and all
> the form controls, but the background color is still wrong because the web
> process background is transparent, so the web content is displayed over the
> UI process's dark background instead of the light web process background.
> That is, everything is now perfect except for the background, and the
> background is only wrong because it is still drawn using the UI process
> theme!

 #if HAVE(OS_DARK_MODE_SUPPORT)
-#if PLATFORM(COCOA)
-    static const auto cssValueControlBackground = CSSValueAppleSystemControlBackground;
-#else
-    static const auto cssValueControlBackground = CSSValueWindow;
-#endif
-    Color baseBackgroundColor = backgroundColor.valueOr(RenderTheme::singleton().systemColor(cssValueControlBackground, styleColorOptions()));
+    Color baseBackgroundColor = backgroundColor.valueOr(RenderTheme::singleton().systemColor(CSSValueAppleSystemControlBackground, styleColorOptions()));
 #else
     Color baseBackgroundColor = backgroundColor.valueOr(Color::white);
 #endif

This reverts changes made in r244635, so now we always use white. Please, check OS_DARK_MODE_SUPPORT is not defined in your build.

> I'm going to give r+ because your patch moves us far in the desired
> direction. Everything except the background now works as desired, which is
> much better than before. We'll probably get new bug reports from users once
> this lands though, because dark mode users are going to see more dark text
> on dark background than they did before this patch. So after this patch
> lands, then a full, complete fix for all our dark mode problems would simply
> require rendering a default web view background in the web process, instead
> of allowing the UI process background color to peek through. So I plan to
> close all existing dark mode bugs and open one new bug for the background.
> Do you agree? And: is the background something you'd have time to work on?

Yes, I can work on it if I can reproduce it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200129/7a9374f7/attachment-0001.htm>