Bug 208129 - [GTK] Stop using gtk foreign drawing API to style form controls
Summary: [GTK] Stop using gtk foreign drawing API to style form controls
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 208132
  Show dependency treegraph
 
Reported: 2020-02-24 04:29 PST by Carlos Garcia Campos
Modified: 2020-03-31 06:38 PDT (History)
26 users (show)

See Also:


Attachments
Patch (216.31 KB, patch)
2020-02-24 04:38 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (216.23 KB, patch)
2020-02-24 04:45 PST, Carlos Garcia Campos
aperez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2020-02-24 04:29:23 PST
It causes layout issues in some websites, it doesn't really work with all GTK themes and it won't be possible with GTK4 because foreign drawing APIs have been removed. Instead we can use the new custom style used by WPE port, which is based on adwaita, but simplified to avoid the huge minimum control sizes, the usage of gradients and transparencies, etc. We can still use the GTK API to get the selection colors, to keep some consistency with the actual GTK theme, but that won't be possible with GTK4 either.
Comment 1 Carlos Garcia Campos 2020-02-24 04:38:08 PST
Created attachment 391524 [details]
Patch
Comment 2 Carlos Garcia Campos 2020-02-24 04:45:04 PST
Created attachment 391525 [details]
Patch
Comment 3 Carlos Garcia Campos 2020-02-24 05:49:38 PST
This will require a major rebaseline that I'll do once the patch lands.
Comment 4 Michael Catanzaro 2020-02-24 06:24:21 PST
Resolves: bug #175067 and bug #185855
Comment 5 Adrian Perez 2020-02-24 07:58:31 PST
Comment on attachment 391525 [details]
Patch

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

> Source/WebCore/platform/adwaita/ThemeAdwaita.hSource/WebCore/platform/wpe/ThemeWPE.h:-46
> -

This looks like a bogus line removal.

> Source/WebCore/platform/gtk/RenderThemeGadget.h:-39
> -class RenderThemeGadget {

It makes me *so* happy to see this hairy code go away!
Comment 6 Carlos Garcia Campos 2020-02-25 01:53:58 PST
Comment on attachment 391525 [details]
Patch

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

>> Source/WebCore/platform/adwaita/ThemeAdwaita.hSource/WebCore/platform/wpe/ThemeWPE.h:-46
>> -
> 
> This looks like a bogus line removal.

Not really, I decided to group virtual methods together. I agree it's out of scope of this bug, though.
Comment 7 Carlos Garcia Campos 2020-02-25 01:56:00 PST
Committed r257299: <https://trac.webkit.org/changeset/257299>
Comment 8 Simon Fraser (smfr) 2020-03-16 16:57:44 PDT
Can we talk about naming here? I just saw RenderThemeAdwaita.h and had no idea what it was.

If you're introducing a new theme, I think it should be announced on webkit-dev, and undergo some naming bikeshedding. "Adwaita" is about as confusing as "Nicosia".
Comment 9 Adrian Perez 2020-03-16 18:20:21 PDT
(In reply to Simon Fraser (smfr) from comment #8)
> Can we talk about naming here? I just saw RenderThemeAdwaita.h and had no
> idea what it was.
> 
> If you're introducing a new theme, I think it should be announced on
> webkit-dev, and undergo some naming bikeshedding. "Adwaita" is about as
> confusing as "Nicosia".

Adwaita is the name of the default GTK theme. So this is basically
a theme that resembles the GTK default somewhat using only WebKit
functionality for rendering the controls — so we can use it also in
the WPE port.
Comment 10 Antti Koivisto 2020-03-17 02:46:27 PDT
RenderThemeGTK?
Comment 11 Adrian Perez 2020-03-17 02:49:42 PDT
(In reply to Antti Koivisto from comment #10)
> RenderThemeGTK?

No. It's also used by WPE now.
Comment 12 Antti Koivisto 2020-03-17 04:02:11 PDT
Well, it is still a GTK theme. 

RenderThemeGTKAndWPE :D
Comment 13 Michael Catanzaro 2020-03-17 09:55:27 PDT
Hm, it's a bit different from Nicosia because the name Nicosia was largely arbitrary and WebKit-specific; we could replace Nicosia with FINLAN and it would make nearly as much sense. In contrast, Adwaita is the name of an existing popular upstream project that should be immediately recognizable to anyone working on Linux, and it's certainly easier to write than RenderThemeGTKAndWPE. The likelihood that we'll eventually have multiple hardcoded themes is quite high; Ubuntu, our biggest distributor, is going to be very unhappy and is sure to want a RenderThemeYaru. elementary OS will be very unhappy and want a RenderThemeGranite. Etc. We can't please everyone, but it's unlikely that Adwaita will remain the only supported theme.

We should probably blog about this very soon, by the way, because it's a real big deal and will affect LTS distros when they update to 2.30. It's also likely to cause a serious adoption problem; it's possible that Ubuntu won't be willing to update to 2.30 until Yaru support exists, but we're not going to want to write a Yaru theme ourselves, and Ubuntu's not likely to want to do it themselves either, so stalemate is likely....
Comment 14 Carlos Garcia Campos 2020-03-19 05:24:08 PDT
(In reply to Michael Catanzaro from comment #13)
> Hm, it's a bit different from Nicosia because the name Nicosia was largely
> arbitrary and WebKit-specific; we could replace Nicosia with FINLAN and it
> would make nearly as much sense. In contrast, Adwaita is the name of an
> existing popular upstream project that should be immediately recognizable to
> anyone working on Linux, and it's certainly easier to write than
> RenderThemeGTKAndWPE. The likelihood that we'll eventually have multiple
> hardcoded themes is quite high; Ubuntu, our biggest distributor, is going to
> be very unhappy and is sure to want a RenderThemeYaru. elementary OS will be
> very unhappy and want a RenderThemeGranite. Etc. We can't please everyone,
> but it's unlikely that Adwaita will remain the only supported theme.

That's not the plan, though. The idea is to have our own single theme and assume that we are going to lose the consistency with other apps. We have started the default theme based on adwaita to minimize the impact for users who don't change the default theme.

> We should probably blog about this very soon, by the way, because it's a
> real big deal and will affect LTS distros when they update to 2.30. It's
> also likely to cause a serious adoption problem; it's possible that Ubuntu
> won't be willing to update to 2.30 until Yaru support exists, but we're not
> going to want to write a Yaru theme ourselves, and Ubuntu's not likely to
> want to do it themselves either, so stalemate is likely....

For GTK3 the only solution I see is to bring back the old code and add a new setting (useSystemAppearance), disabled by default, that apps can use to keep the consistency. I understand it's not the same for a web browser like ephy, that would be inconsistent with other apps, than other apps like evo that would be inconsistent with itself (web views will have a scrollbar while other widgets will have others).

There's no solution for GTK4, foreign drawing is not supported and it's not even possible to ask GTK for the selection colors to keep a minimum consistency.

I don't know what else we can do from WebKit, to be honest.
Comment 15 Michael Catanzaro 2020-03-19 09:02:48 PDT
(In reply to Carlos Garcia Campos from comment #14)
> I don't know what else we can do from WebKit, to be honest.

I think we should just tell distros that they are welcome to maintain their own RenderTheme classes in WebKit if they want consistency with their system theme, but we will only maintain the Adwaita theme. Maybe they'll be OK with it looking like Adwaita, maybe not. Let's just give them a heads-up now so they're not surprised when 2.30 is released?
Comment 16 Michael Catanzaro 2020-03-19 13:23:35 PDT
(In reply to Carlos Garcia Campos from comment #14)
> For GTK3 the only solution I see is to bring back the old code and add a new
> setting (useSystemAppearance), disabled by default, that apps can use to
> keep the consistency.

Compromise proposal: do this, but enabled by default, and only for scrollbars. All the web form controls can use Adwaita theme. Scrollbar is going to be more important than form controls since having a non-native scrollbar will look really out of place. The rest is just web content, and it's not expected that web content look native. Then when GTK 4 comes around... well, I don't know what to do for GTK 4, but I think retaining native scrollbar would be enough to avoid most complaints. Maybe we could try moving the scrollbar to the UI process, except when it's actually themed by websites (in which case it has to be in the web process no matter what).

Maybe we could add a config file or some other way of configuring a color scheme on top of Adwaita that Yaru and Granite (and other themes) could use, without creating a completely separate RenderTheme class that would be tough to maintain.
Comment 17 Adrian Perez 2020-03-19 13:49:17 PDT
(In reply to Michael Catanzaro from comment #16)
> (In reply to Carlos Garcia Campos from comment #14)
> > For GTK3 the only solution I see is to bring back the old code and add a new
> > setting (useSystemAppearance), disabled by default, that apps can use to
> > keep the consistency.
> 
> Compromise proposal: do this, but enabled by default, and only for
> scrollbars. All the web form controls can use Adwaita theme. Scrollbar is
> going to be more important than form controls since having a non-native
> scrollbar will look really out of place. The rest is just web content, and
> it's not expected that web content look native. Then when GTK 4 comes
> around... well, I don't know what to do for GTK 4, but I think retaining
> native scrollbar would be enough to avoid most complaints. Maybe we could
> try moving the scrollbar to the UI process, except when it's actually themed
> by websites (in which case it has to be in the web process no matter what).
> 
> Maybe we could add a config file or some other way of configuring a color
> scheme on top of Adwaita that Yaru and Granite (and other themes) could use,
> without creating a completely separate RenderTheme class that would be tough
> to maintain.

Wild idea: can we expose somehow in the WebKitWebView API the information
that an embedder might need to render a scrollbar? I think the following
additions each scrollbar should suffice:

 - Current position, range [0.0, 1.0].
 - Size of the viewport vs. page height (to know how long the thumb
   needs to be).
 - Whether the web view needs to show a scrollbar (e.g. FALSE if content
   does not need it, or if CSS has been used to override the default
   scrollbar look).
 - A new method to scroll to a given position.

Am I being too naïve with this idea?
Comment 18 Adrian Perez 2020-03-19 13:53:31 PDT
(In reply to Adrian Perez from comment #17)

> > [...]
> 
> Wild idea: can we expose somehow in the WebKitWebView API the information
> that an embedder might need to render a scrollbar? I think the following
> additions each scrollbar should suffice:
> 
>  - Current position, range [0.0, 1.0].
>  - Size of the viewport vs. page height (to know how long the thumb
>    needs to be).
>  - Whether the web view needs to show a scrollbar (e.g. FALSE if content
>    does not need it, or if CSS has been used to override the default
>    scrollbar look).
>  - A new method to scroll to a given position.
> 
> Am I being too naïve with this idea?

I forgot to mention that this would be only for the topmost scrollbar, the
one attached to the web view itself, and I would not try to handle elements
inside web pages (iframes, textareas, etc.) — rendering for those would still
be done by WebKit.

Related (possibly complimentary) idea: style the controls that WebKit{GTK,WPE}
renders in a way that they do not recall any particular toolkit and are as
neutral as possible, so they blend well in most environments. Currently they
look a tad too much like GTK's Adwaita theme, and while it is not too bad,
the illusion falls apart a little bit when using other themes — specially
when it comes to the scrollbar attached to the sides of a web view, I think
we need a more subtle look there.
Comment 19 Carlos Garcia Campos 2020-03-20 02:12:46 PDT
(In reply to Michael Catanzaro from comment #15)
> (In reply to Carlos Garcia Campos from comment #14)
> > I don't know what else we can do from WebKit, to be honest.
> 
> I think we should just tell distros that they are welcome to maintain their
> own RenderTheme classes in WebKit if they want consistency with their system
> theme, but we will only maintain the Adwaita theme. Maybe they'll be OK with
> it looking like Adwaita, maybe not. Let's just give them a heads-up now so
> they're not surprised when 2.30 is released?

Do you mean distros maintaining a RenderTheme upstream or downstream? I don't think it will work in any case. In case of being upstream we will end up maintaining it, every time a change is made in base class it will break, etc. I prefer to keep the old code to be honest. And it would be a lot of work for downstream patches, I'm afraid.
Comment 20 Carlos Garcia Campos 2020-03-20 02:18:08 PDT
(In reply to Michael Catanzaro from comment #16)
> (In reply to Carlos Garcia Campos from comment #14)
> > For GTK3 the only solution I see is to bring back the old code and add a new
> > setting (useSystemAppearance), disabled by default, that apps can use to
> > keep the consistency.
> 
> Compromise proposal: do this, but enabled by default, and only for
> scrollbars. 

Sounds good to me.

> All the web form controls can use Adwaita theme. Scrollbar is
> going to be more important than form controls since having a non-native
> scrollbar will look really out of place. The rest is just web content, and
> it's not expected that web content look native. Then when GTK 4 comes
> around... well, I don't know what to do for GTK 4, but I think retaining
> native scrollbar would be enough to avoid most complaints.

Makes sense.

> Maybe we could
> try moving the scrollbar to the UI process, except when it's actually themed
> by websites (in which case it has to be in the web process no matter what).

That would be a lot more work and very difficult to do it right.

> Maybe we could add a config file or some other way of configuring a color
> scheme on top of Adwaita that Yaru and Granite (and other themes) could use,
> without creating a completely separate RenderTheme class that would be tough
> to maintain.

Benjamin Otte suggested something similar, GTK themes could include a file with color definitions to be used by WebKit and/or other browsers (at least the selection colors). That wouldn't fix the scrollbars, though.
Comment 21 Carlos Garcia Campos 2020-03-20 02:22:30 PDT
(In reply to Adrian Perez from comment #17)
> (In reply to Michael Catanzaro from comment #16)
> > (In reply to Carlos Garcia Campos from comment #14)
> > > For GTK3 the only solution I see is to bring back the old code and add a new
> > > setting (useSystemAppearance), disabled by default, that apps can use to
> > > keep the consistency.
> > 
> > Compromise proposal: do this, but enabled by default, and only for
> > scrollbars. All the web form controls can use Adwaita theme. Scrollbar is
> > going to be more important than form controls since having a non-native
> > scrollbar will look really out of place. The rest is just web content, and
> > it's not expected that web content look native. Then when GTK 4 comes
> > around... well, I don't know what to do for GTK 4, but I think retaining
> > native scrollbar would be enough to avoid most complaints. Maybe we could
> > try moving the scrollbar to the UI process, except when it's actually themed
> > by websites (in which case it has to be in the web process no matter what).
> > 
> > Maybe we could add a config file or some other way of configuring a color
> > scheme on top of Adwaita that Yaru and Granite (and other themes) could use,
> > without creating a completely separate RenderTheme class that would be tough
> > to maintain.
> 
> Wild idea: can we expose somehow in the WebKitWebView API the information
> that an embedder might need to render a scrollbar? I think the following
> additions each scrollbar should suffice:
> 
>  - Current position, range [0.0, 1.0].
>  - Size of the viewport vs. page height (to know how long the thumb
>    needs to be).
>  - Whether the web view needs to show a scrollbar (e.g. FALSE if content
>    does not need it, or if CSS has been used to override the default
>    scrollbar look).
>  - A new method to scroll to a given position.
> 
> Am I being too naïve with this idea?

In WebKit1, we coordinated the view scrolling with the GtkScrollWindow and it was nightmare, and a source of weird bugs. It was very difficult to keep it in sync in the same process, I can't imagine between two processes.
Comment 22 Michael Catanzaro 2020-03-20 06:03:17 PDT
Maybe we can get Benjamin to give us something to allow foreign drawing of just scrollbars alone. He probably won't be terribly happy about that, but foreign drawing for just one particular widget is probably only 2% the complexity of the original foreign drawing API.
Comment 23 Benjamin Otte 2020-03-20 06:14:55 PDT
GTK is using a CSS engine and widgets are combinations of elements, so asking for "just the styling of the scrollbar" is like asking webkitgtk for "just the styling of the Youtube player".
Comment 24 Adrian Perez 2020-03-20 09:08:40 PDT
Well, definitely I was being naïve when suggesting to expose
API to allow the embedder to render the scrollbars (thanks for
the background, Carlos).

All things considered, it is starting to look like the lesser
of evils might be allowing somehow for GTK themes to provide
styling information used by WebKit to render scrollbars 🤷‍♂️️