WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
208129
[GTK] Stop using gtk foreign drawing API to style form controls
https://bugs.webkit.org/show_bug.cgi?id=208129
Summary
[GTK] Stop using gtk foreign drawing API to style form controls
Carlos Garcia Campos
Reported
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.
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2020-02-24 04:38:08 PST
Created
attachment 391524
[details]
Patch
Carlos Garcia Campos
Comment 2
2020-02-24 04:45:04 PST
Created
attachment 391525
[details]
Patch
Carlos Garcia Campos
Comment 3
2020-02-24 05:49:38 PST
This will require a major rebaseline that I'll do once the patch lands.
Michael Catanzaro
Comment 4
2020-02-24 06:24:21 PST
Resolves:
bug #175067
and
bug #185855
Adrian Perez
Comment 5
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!
Carlos Garcia Campos
Comment 6
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.
Carlos Garcia Campos
Comment 7
2020-02-25 01:56:00 PST
Committed
r257299
: <
https://trac.webkit.org/changeset/257299
>
Simon Fraser (smfr)
Comment 8
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".
Adrian Perez
Comment 9
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.
Antti Koivisto
Comment 10
2020-03-17 02:46:27 PDT
RenderThemeGTK?
Adrian Perez
Comment 11
2020-03-17 02:49:42 PDT
(In reply to Antti Koivisto from
comment #10
)
> RenderThemeGTK?
No. It's also used by WPE now.
Antti Koivisto
Comment 12
2020-03-17 04:02:11 PDT
Well, it is still a GTK theme. RenderThemeGTKAndWPE :D
Michael Catanzaro
Comment 13
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....
Carlos Garcia Campos
Comment 14
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.
Michael Catanzaro
Comment 15
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?
Michael Catanzaro
Comment 16
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.
Adrian Perez
Comment 17
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?
Adrian Perez
Comment 18
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.
Carlos Garcia Campos
Comment 19
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.
Carlos Garcia Campos
Comment 20
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.
Carlos Garcia Campos
Comment 21
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.
Michael Catanzaro
Comment 22
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.
Benjamin Otte
Comment 23
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".
Adrian Perez
Comment 24
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 🤷♂️️
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug