WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
236476
[GTK] Sync gtk-overlay-scrolling setting to web process
https://bugs.webkit.org/show_bug.cgi?id=236476
Summary
[GTK] Sync gtk-overlay-scrolling setting to web process
Michael Catanzaro
Reported
2022-02-10 17:21:34 PST
Instead of checking only this environment variable, let's check the GtkSetting value as well. This is possible since
r280077
now that we have GtkSettingsManager and GtkSettingsManagerProxy. Note the environment variable surprisingly actually overrides the value in settings. Also note the environment variable is specific to GTK 3 only.
Attachments
Patch
(10.69 KB, patch)
2022-02-10 17:24 PST
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Patch
(10.60 KB, patch)
2022-02-11 06:03 PST
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Patch
(11.12 KB, patch)
2022-02-21 15:06 PST
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Michael Catanzaro
Comment 1
2022-02-10 17:24:57 PST
Created
attachment 451624
[details]
Patch
Michael Catanzaro
Comment 2
2022-02-10 17:29:05 PST
Comment on
attachment 451624
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451624&action=review
> Source/WebCore/platform/adwaita/ScrollbarThemeAdwaita.cpp:74 > - static bool shouldUuseOverlayScrollbars = g_strcmp0(g_getenv("GTK_OVERLAY_SCROLLING"), "0"); > - return shouldUuseOverlayScrollbars; > + return shouldUseOverlayScrollbars();
Note this isn't quite right, because it's no longer static, so ScrollbarThemeAdwaita will check for settings changes, but RenderThemeScrollbar actually is still static so it won't check... I should change one or the other before landing this, probably RenderThemeScrollbar. Opinions welcome.
Michael Catanzaro
Comment 3
2022-02-11 06:01:20 PST
I suppose I'll change it to be not static.
Michael Catanzaro
Comment 4
2022-02-11 06:03:06 PST
Created
attachment 451683
[details]
Patch
Michael Catanzaro
Comment 5
2022-02-17 11:57:01 PST
Ping reviewers.
Alice Mikhaylenko
Comment 6
2022-02-17 12:00:54 PST
Do scrollbar invalidate if you change the setting in runtime? Existing code listens to "notify::gtk-theme-name", so I suppose you'll need to listen to "notify::gtk-overlay-scrolling" as well.
Michael Catanzaro
Comment 7
2022-02-17 12:25:06 PST
Comment on
attachment 451683
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451683&action=review
> Source/WebKit/UIProcess/gtk/GtkSettingsManager.cpp:209 > g_signal_connect_swapped(m_settings, "notify::gtk-cursor-blink", G_CALLBACK(settingsChangedCallback), this); > g_signal_connect_swapped(m_settings, "notify::gtk-cursor-blink-time", G_CALLBACK(settingsChangedCallback), this); > g_signal_connect_swapped(m_settings, "notify::gtk-primary-button-warps-slider", G_CALLBACK(settingsChangedCallback), this); > + g_signal_connect_swapped(m_settings, "notify::gtk-overlay-scrolling", G_CALLBACK(settingsChangedCallback), this);
We have notifies for everything here.
Alice Mikhaylenko
Comment 8
2022-02-17 12:32:09 PST
That's UI process, I mean on the web process side.
Alice Mikhaylenko
Comment 9
2022-02-17 12:33:04 PST
To be more specific, check this line in ScrollbarThemeGtk.cpp: g_signal_connect(gtk_settings_get_default(), "notify::gtk-theme-name", G_CALLBACK(themeChangedCallback), nullptr);
Michael Catanzaro
Comment 10
2022-02-17 13:26:54 PST
Well it won't *immediately* notice changes, but it should notice next time scrollbars are drawn, because the change does get synced to the web process, yes? It's checking the setting each time. The setting is just a bool, so there isn't anything else to sync like there is in themeChangedCallback.
Alice Mikhaylenko
Comment 11
2022-02-17 13:44:06 PST
Well, hence I wonder if it should call themeChanged() so it notices the changes immediately? :)
Michael Catanzaro
Comment 12
2022-02-21 15:06:34 PST
OK, sure.
Michael Catanzaro
Comment 13
2022-02-21 15:06:57 PST
Created
attachment 452780
[details]
Patch
EWS
Comment 14
2022-03-08 06:51:44 PST
Committed
r290988
(
248166@main
): <
https://commits.webkit.org/248166@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 452780
[details]
.
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