Bug 202194 - REGRESSION(r244635): [GTK] White text on white background in 2.26 when using dark GTK theme breaks Evolution and Geary
Summary: REGRESSION(r244635): [GTK] White text on white background in 2.26 when using ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords: Gtk
: 201817 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-09-25 03:29 PDT by Michael Gratton
Modified: 2020-01-29 02:39 PST (History)
8 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Gratton 2019-09-25 03:29:20 PDT
There's a few bug reports about people getting white text on white backgrounds for people running 2.25/2.26: https://gitlab.gnome.org/GNOME/geary/issues/573 and https://gitlab.gnome.org/GNOME/evolution/issues/617

While the reports above are for contenteditable elements, it also seems to happen for arbitrary, non-editable content when the app and/or page doesn't explicitly specify both foreground and background colours.

For example, install Geary from flathub, enable a dark theme for it using the GTK Inspector, and view an email from gitlab.gnome.org, or compose a new email.

Evo has already deployed a workaround, and I probably will for Geary as well, shouldn't either both or nether the text and background colour get set, not one and not the other?
Comment 1 Michael Catanzaro 2019-12-23 10:10:35 PST
*** Bug 201817 has been marked as a duplicate of this bug. ***
Comment 2 Michael Catanzaro 2020-01-20 14:04:44 PST
AFAIK this bug is breaking distros that updated from 2.24 -> 2.26 without also updating Geary or Evolution with a workaround for this issue.
Comment 3 Michael Catanzaro 2020-01-27 16:04:36 PST
OK, this regressed in r244635. The intent of that change was to fix bug #126907 and make text boxes readable in dark mode. Previously we used hardcoded black text on top of the theme background color, which was bogus because that is unreadable if the theme background color is dark. Well that's now fixed, but at the cost that we now have a disaster when the default (theme) text color is used on top of a custom background color.

So I'm not sure what we should do here. This a 2.24 -> 2.26 regression and it makes it difficult for distros to upgrade to 2.26, since both Evolution and Geary need to be separately patched to do this upgrade safely. Generally we treat changes that break applications as regressions, but it's unclear whether that's OK to do in this case. On the one hand, Evolution and Geary workarounds already exist and are very easy to deploy. Also, Evolution and Geary changes are absolutely required regardless of this issue, due to 2.26 dropping single web process mode. But on the other hand, any application breakage makes it extremely difficult to justify WebKit upgrades, and according to the security advisories, there are currently 23 CVEs currently fixed in 2.26 but not in 2.24. Also, we can (and do) patch WebKit to force individual apps to use single-process mode again by checking prgname, which allows updating WebKit to 2.26 while leaving Evolution and Geary unchanged at older versions.

So I don't know whether we should roll out r244635 or not.

Relevant consideration: dark mode on the web has *never* worked acceptably well, and it's very much lost cause for us. We've discussed this extensively in bug #197947, and I believe we have consensus that we need to remove dark mode support and force light mode until such time that we're able to develop an HTML theme that doesn't depend on GTK. (See that bug for details.) If we agree on that, then I think that tilts the balance in favor of rollout. Once we fix bug #197947, all other dark mode bugs will immediately go away, so the rollout won't really hurt anymore.

But perhaps another reasonable solution would be: if the text color is not specified, then use the theme text color only if the text is being displayed on the default background, otherwise use black if the background has been set (to anything other than the default theme background). Problem is, I don't know how to implement that, and I don't know whether that would be easy or really hard.

Thoughts?
Comment 4 Michael Catanzaro 2020-01-27 16:15:24 PST
(In reply to Michael Catanzaro from comment #3)
> But perhaps another reasonable solution would be: if the text color is not
> specified, then use the theme text color only if the text is being displayed
> on the default background, otherwise use black if the background has been
> set (to anything other than the default theme background). Problem is, I
> don't know how to implement that, and I don't know whether that would be
> easy or really hard.

It's been five minutes and I'm already regretting having suggested this. It's a can of worms. We should just revert r244635 and press ahead on forcing light mode in bug #198947. There is just no chance that dark mode is *ever* going to work with web content that's not expecting it. We're causing all these theme problems by forcing it regardless of whether color-schemes is set, and trying to fix individual problems here and there without regressing other web content is not going to be successful. If we looked for the color-schemes property before using the dark theme, then we'd know the web content is prepared to handle dark mode with light default text color and dark background color. Otherwise, it's just not.
Comment 5 Michael Catanzaro 2020-01-27 16:17:39 PST
(In reply to Michael Catanzaro from comment #4)
> We should just revert r244635 and press ahead on
> forcing light mode in bug #198947.

Sigh, of course I meant: bug #197947.
Comment 6 Milan Crha 2020-01-28 01:38:24 PST
For what it worth, evo has a code to pick a suitable text color for certain background color [1]. Maybe you can reuse it for cases where the input item's background or text color is not specified by the web page.

[1] https://gitlab.gnome.org/GNOME/evolution/blob/master/src/e-util/e-misc-utils.c#L1659
Comment 7 Carlos Garcia Campos 2020-01-28 08:42:10 PST
I think this should be fixed by patch attached to bug #197947.
Comment 8 Carlos Garcia Campos 2020-01-29 01:50:28 PST
I'll backport r255342 to 2.26 branch
Comment 9 Carlos Garcia Campos 2020-01-29 02:39:38 PST
Backported r255350: <https://trac.webkit.org/changeset/255350>