Bug 159121 – [GTK] Expose contentDirectionPolicy preference

• Status: NEW
• Component: WebKitGTK

[Yosef Or Boczko]

In the last version, the position of the scrollbars
have been moved to the left side in RTL.

It look wrong to me, I think it should be reversed, to be
shown on the right side in RTL.

WebKitGTK+ 2.13.2.
Tested on epiphany from git master (3.21.3).

See https://bugzilla.gnome.org/show_bug.cgi?id=768041

[Michael Catanzaro]

I'm confused, because in any other application (tested nautilus and gedit) I can see scrollbars are on the left when running with LANG=he_IL.utf-8... so why shouldn't they be on the left for web sites as well? If it's just "because that's what other browsers do," I guess that expectation will change once this change hits Apple devices, right? Striving for consistency with the rest of the desktop seems more important to me.

I'll note that some of the layout tests are skipped with the comment "RTL scrollbars are only supported on Mac." We certainly do support RTL in GTK and expect our scrollbars to do... something that is familiar to folks who use RTL locales. I see there is a new ScrollableArea::systemLanguageIsRTL function that is only implemented on OS X. Guess: is it possible that the scrollbars are always on the left on OS X, but in GTK the position varies based on the language used on the web page rather than system locale? If this is the problem, I can see why that would be problematic. I guess if we were to implement that function, the scrollbars would then always be on the left (in RTL locales); that's the opposite of what you requested, but would that be desirable for Hebrew, Yosef?

[Myles C. Maxfield]

(In reply to comment #1)
> I'm confused, because in any other application (tested nautilus and gedit) I
> can see scrollbars are on the left when running with LANG=he_IL.utf-8... so
> why shouldn't they be on the left for web sites as well? If it's just
> "because that's what other browsers do," I guess that expectation will
> change once this change hits Apple devices, right? Striving for consistency
> with the rest of the desktop seems more important to me.
> 
> I'll note that some of the layout tests are skipped with the comment "RTL
> scrollbars are only supported on Mac." We certainly do support RTL in GTK
> and expect our scrollbars to do... something that is familiar to folks who
> use RTL locales. I see there is a new ScrollableArea::systemLanguageIsRTL
> function that is only implemented on OS X. Guess: is it possible that the
> scrollbars are always on the left on OS X, but in GTK the position varies
> based on the language used on the web page rather than system locale? If
> this is the problem, I can see why that would be problematic. I guess if we
> were to implement that function, the scrollbars would then always be on the
> left (in RTL locales); that's the opposite of what you requested, but would
> that be desirable for Hebrew, Yosef?

For the macOS port, I've implemented a WK2 API userInterfaceDirectionPolicy on the WKWebViewConfigurationPolicy which allows changing between two modes:
- Web content selecting the placement of scrollbars (via dir="rtl")
- Embedding app selecting the placement of scrollbars (via the macOS platform's mechanism for doing so)

The intention is that web browsers will opt for the first mode and web views in native apps will opt for the second mode. The first mode is the default.

Please see https://trac.webkit.org/changeset/200116

[Myles C. Maxfield]

(In reply to comment #2)
> (In reply to comment #1)
> > I'm confused, because in any other application (tested nautilus and gedit) I
> > can see scrollbars are on the left when running with LANG=he_IL.utf-8... so
> > why shouldn't they be on the left for web sites as well? If it's just
> > "because that's what other browsers do," I guess that expectation will
> > change once this change hits Apple devices, right? Striving for consistency
> > with the rest of the desktop seems more important to me.
> > 
> > I'll note that some of the layout tests are skipped with the comment "RTL
> > scrollbars are only supported on Mac." We certainly do support RTL in GTK
> > and expect our scrollbars to do... something that is familiar to folks who
> > use RTL locales. I see there is a new ScrollableArea::systemLanguageIsRTL
> > function that is only implemented on OS X. Guess: is it possible that the
> > scrollbars are always on the left on OS X, but in GTK the position varies
> > based on the language used on the web page rather than system locale? If
> > this is the problem, I can see why that would be problematic. I guess if we
> > were to implement that function, the scrollbars would then always be on the
> > left (in RTL locales); that's the opposite of what you requested, but would
> > that be desirable for Hebrew, Yosef?
> 
> For the macOS port, I've implemented a WK2 API userInterfaceDirectionPolicy
> on the WKWebViewConfigurationPolicy which allows changing between two modes:
> - Web content selecting the placement of scrollbars (via dir="rtl")
> - Embedding app selecting the placement of scrollbars (via the macOS
> platform's mechanism for doing so)
> 
> The intention is that web browsers will opt for the first mode and web views
> in native apps will opt for the second mode. The first mode is the default.
> 
> Please see https://trac.webkit.org/changeset/200116

Anders added a C API for this in https://trac.webkit.org/changeset/201118

[Michael Catanzaro]

Thanks Myles.

(In reply to comment #2) 
> For the macOS port, I've implemented a WK2 API userInterfaceDirectionPolicy
> on the WKWebViewConfigurationPolicy which allows changing between two modes:
> - Web content selecting the placement of scrollbars (via dir="rtl")
> - Embedding app selecting the placement of scrollbars (via the macOS
> platform's mechanism for doing so)
> 
> The intention is that web browsers will opt for the first mode and web views
> in native apps will opt for the second mode.

Yosef, this means that the new behavior is intentional -- the scrollbars will now change position depending on the web site's text direction.

> The first mode is the default.
> 
> Please see https://trac.webkit.org/changeset/200116

OK. I think we will want the default to be the opposite for GTK, to optimize for native apps; then we can change the setting in just Epiphany rather than in every app using WebKitGTK+.

[Michael Catanzaro]

(In reply to comment #4)
> OK. I think we will want the default to be the opposite for GTK, to optimize
> for native apps; then we can change the setting in just Epiphany rather than
> in every app using WebKitGTK+.

Or perhaps that's wrong, and only native apps that have thought about RTL support should have access to the setting?

Yosef, your feedback would be much appreciated; it's hard for me to guess what the expected behavior should be here.

[Michael Catanzaro]

Probably best if we stick with WebKit project defaults.

[Michael Catanzaro]

Created attachment 282384 [details]
Patch

[Michael Catanzaro]

Note the name I picked for the setting is terrible; suggestions welcome.

[Anders Carlsson]

Comment on attachment 282384 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:3206
> +    priv->preferences->setUserInterfaceDirectionPolicy(static_cast<unsigned>(enabled ?
> +        WebCore::UserInterfaceDirectionPolicy::Content : WebCore::UserInterfaceDirectionPolicy::System));

This isn't going to work once any web views have been created with these preferences. 

There's a reason why we made this a WKWebViewConfiguration property in the modern API, we don't support changing it and I don't think it makes sense as a preference.

[Michael Catanzaro]

OK, I see: our other WebKitSettings can all be meaningfully changed after a web view is created, but not this one.

We could create a WebKitWebViewConfiguration object to hold such settings, containing only this setting for now, make it a new construct-only property of WebKitWebView... and add webkit_web_view_new_with_configuration for good measure.

Or we could just add a comment to the setting that says changes only affect new web views. I have no preference (heh) here. Carlos, what do you think?

[Carlos Garcia Campos]

Comment on attachment 282384 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:40
> +#include <WebCore/page/Settings.h>

We shouldn't need WebCore settings here.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:150
> +    PROP_USE_CONTENT_DIRECTION_POLICY

This is confusing name. Also, I'm surprised the internal setting is called UserInterfaceDirectionPolicy while it only affects scrollbars, all other elements in the page follow the web page direction.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1298
> +     * Whether or not web content should be allowed to select the
> +     * placement of scrollbars. By default, scrollbars will appear on
> +     * right when page content uses left-to-right text direction, and on
> +     * the left when page content uses right-to-left text direction.
> +     * This setting allows you to disable that behavior, so that
> +     * scrollbars always appear on the side dictated by the current
> +     * locale.

We need Yosef input to understand what the expected behavior is. I was surprised when I saw the bug report in ephy.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:3182
> +    return settings->priv->preferences->userInterfaceDirectionPolicy() == static_cast<unsigned>(WebCore::UserInterfaceDirectionPolicy::Content);

We don't cast WebCore values, we use conversion methods instead. In this case you should probably just compare to kWKUserInterfaceDirectionPolicyContent

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:3206
>> +        WebCore::UserInterfaceDirectionPolicy::Content : WebCore::UserInterfaceDirectionPolicy::System));
> 
> This isn't going to work once any web views have been created with these preferences. 
> 
> There's a reason why we made this a WKWebViewConfiguration property in the modern API, we don't support changing it and I don't think it makes sense as a preference.

Web view configuration properties are usually exposed as construct-only properties of WebKitWebView. We don't need to add a _new method for every single construct-only property, if we choose the right default value this won't be commonly used so apps needing this could just use g_object_new.