Yeah, I think the incognito mode theme is based on Adwaita. I'm not sure how to handle this better. Perhaps we need to check that the current theme is Adwaita, and if not, avoid loading the style provider at all.

For reference, the theme is here: https://git.gnome.org/browse/epiphany/tree/src/resources/epiphany.scss

Uhm, not loading the style provider at all would not load all the shipped style, right? In that case that's going to break other things, something you could do is not setting the incognito-mode styleclass if the current theme is not adwaita and switch the window to the dark theme variant (or light in case of global dark theme is set), IIRC apple does something like that on safari.

Thanks for the quick reply!

Is there a way to check the theme name programatically?

How do we handle the many other applications with Adwaita-specific styles, do we still keep all of that CSS in Adwaita itself?

Might be easiest to split the incognito mode theme to a separate SCSS file so that we can use two separate style providers. That's possible, right?

Those are all question for a proper gtk+ hacker, me write css :-)
From a theme pov I think it's totally possible to split the incognito-mode css, if you don't set the the styleclass though that's not needed.
There aren't many applications with Adwaita specific style, when I write style for apps I avoid at all costs to write Adwaita specific code (in epiphany's case it wasn't possible unfortunatelly, I blame jimmac for that :-D), that's usually a matter of killing borders and stuff like that, builder which needs adwaita specific code includes different theme versions, you should check with Christian about that though, I just take a look at builder on Christian requests and that doesn't happen frequently, since he's pretty good himself :-)

Copying a mail from Christian:

"""
In libdazzle we have DzlCssProvider that can update the CSS theme based
on GtkSettings:gtk-theme-name and gtk-application-prefer-dark-theme.

When the theme changes, the proper CSS will be (re)loaded. So all you
need to do is have a /org/gnome/epiphany/themes/ resource directory with
something like:

  Adwaita.css
  Adwaita-dark-css (if not there, it will load Adwaita.css)
  shared.css (fallback if no per-theme setting was found)
  Arc.css
  Arc-Dark.css
  .. etc

It should be fairly straight-forward to copy it out of tree if
necessary. But if you use DzlApplication, it will automatically load all
this and handle it for you.

https://git.gnome.org/browse/libdazzle/tree/src/theming/dzl-css-provider.c
"""

I have made a proof-of-concept patch which makes EphyEmbedShell
a subclass of DzlApplication, and splits the CSS into “shared.css”
and “Adwaita.css” (which @imports also the former), and moves the
resource paths around so the theme manager in libdazzle autoloads
CSS files automagically.

So far looks like this: https://imgur.com/a/KU0aE7L

I have to do some cleanups before submitting the patch.

Created attachment 371553 [details] [review]
[PATCH] Simpler custom CSS theming for non-Adwaita themes

Tested with a few themes, both with light and dark header bars:

 - Adwaita
 - Arc, Arc-Dark, Arc-Darker
 - Arc-OSX-Darker
 - MosArc
 - Zukitre

For these above, the theming for the incognito windows works
pretty well, and bug #792447 is fixed as well: the “shared.css”
used for themes other than Adwaita does not try to change the
colors of the header bar or the URL entry at all, so the colors
defined by the GTK+ theme are used.

(In reply to Adrian Perez from comment #8)
> Created attachment 371553 [details] [review] [review]
> [PATCH] Simpler custom CSS theming for non-Adwaita themes
> 
> Tested with a few themes, both with light and dark header bars:
>
> [...]

FWIW, I have been running Epiphany since last Friday with this
patch applied and it has been working perfectly 

Review of attachment 371553 [details] [review]:

::: embed/meson.build
@@ +42,3 @@
   m_dep,
+  webkit2gtk_dep,
+  dazzle_dep,

Alphabetize it, please

::: lib/widgets/meson.build
@@ +33,3 @@
   libsoup_dep,
+  webkit2gtk_dep,
+  dazzle_dep,

Ditto

::: meson.build
@@ +66,3 @@
 nettle_requirement = '>= 3.2'
 webkitgtk_requirement = '>= 2.21.1'
+dazzle_requirement = '>= 3.28.0'

Ditto

Actually, since this is only used in one place, I prefer to inline it to save vertical space.

@@ +70,2 @@
 cairo_dep = dependency('cairo', version: '>= 1.2')
+dazzle_dep = dependency('libdazzle-1.0', version: dazzle_requirement)

Good alphabetization :P

::: src/resources/epiphany.gresource.xml
@@ +36,3 @@
     <file compressed="true" alias="scalable/actions/ephy-bookmark-tag-symbolic.svg">ephy-bookmark-tag-symbolic.svg</file>
   </gresource>
+  <gresource prefix="/org/gnome/Epiphany">

A follow-up patch to get rid of the /org/gnome/epiphany prefix would be lovely.

::: src/resources/parse-sass.sh
@@ +1,3 @@
 #!/bin/sh
 
+: ${GTK_SOURCE_PATH:="../../../gtk+-3"}

What does the colon do?

I don't like this script, because of the way it depends on the relative path to the GTK+ checkout. Ideally we would require scss as part of the build, like gnome-shell has done, and remove the CSS files from version control. But it seems like that's just not possible, because of this very weird way of depending on the internals of GTK+.

Created attachment 371556 [details] [review]
[PATCH v2] Simpler custom CSS theming for non-Adwaita themes

Addresses review comments.

(In reply to Michael Catanzaro from comment #10)

> ::: src/resources/parse-sass.sh
> @@ +1,3 @@
>  #!/bin/sh
>  
> +: ${GTK_SOURCE_PATH:="../../../gtk+-3"}
> 
> What does the colon do?

The colon makes the shell evaluate the rest of the command line,
without trying to execute any command. This is a typical idiom
with the “:=” variable expansion assignment, which allows
overriding the value of the variable, e.g. by running:

  % GTK_SOURCE_PATH=foo/bar ./src/resources/parse-sass.sh

while the default value (after the “:=” symbol) will be assigned
and used if the variable is undefined at the point of executing
this line of the script. 
 
> I don't like this script, because of the way it depends on the relative path
> to the GTK+ checkout. Ideally we would require scss as part of the build,
> like gnome-shell has done, and remove the CSS files from version control.
> But it seems like that's just not possible, because of this very weird way
> of depending on the internals of GTK+.

Yep, that would be nice, but I don't see an easy way around it, other
than copying GTK+'s SCSS files in the Epiphany repository, and that's
equally ugly... so I would rather just leave it as-is.

The following fixes have been pushed:
bcc0b21 Bundle libdazzle in flatpak manifest
6406d18 Simpler custom CSS theming for non-Adwaita themes

Created attachment 371557 [details] [review]
Bundle libdazzle in flatpak manifest

Created attachment 371558 [details] [review]
Simpler custom CSS theming for non-Adwaita themes

In particular, the theming for the incognito windows tends to look odd
with themes other than Adwaita. It is possible to load different CSS
resources depending on the selected theme by handling changes to the
GtkSettings::gtk-theme-name property: this splits the CSS into a
"shared.css" part which contains the rules which play well with most
themes, and an "Adwaita.css" which builds upon the shared CSS rules
and is loaded only for the Adwaita theme. The CSS code is still
generated from SCSS, with definitions used by SCSS snippets moved
into a new _definitions.scss file to avoid repeating them.

Note that instead of manually handling theme changes, EphyEmbedShell
is changed to inherit from DzlApplication (instead of GtkApplication),
which already implements the desired CSS resource loading behaviour.
This makes the existing CSS loading code unneeded, and therefore it
is removed. Also, the resources are moved into the resource path
/org/gnome/Epiphany/themes as expected by DzlApplication.