Bug 159121 - [GTK] Expose contentDirectionPolicy preference
Summary: [GTK] Expose contentDirectionPolicy preference
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Linux
: P2 Normal
Assignee: Michael Catanzaro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-25 14:45 PDT by Yosef Or Boczko
Modified: 2016-06-30 00:05 PDT (History)
5 users (show)

See Also:


Attachments
Patch (10.52 KB, patch)
2016-06-29 15:58 PDT, Michael Catanzaro
cgarcia: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yosef Or Boczko 2016-06-25 14:45:14 PDT
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
Comment 1 Michael Catanzaro 2016-06-25 16:04:00 PDT
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?
Comment 2 Myles C. Maxfield 2016-06-27 14:03:53 PDT
(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
Comment 3 Myles C. Maxfield 2016-06-28 14:04:38 PDT
(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
Comment 4 Michael Catanzaro 2016-06-28 19:51:06 PDT
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+.
Comment 5 Michael Catanzaro 2016-06-28 19:58:23 PDT
(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.
Comment 6 Michael Catanzaro 2016-06-29 08:35:07 PDT
Probably best if we stick with WebKit project defaults.
Comment 7 Michael Catanzaro 2016-06-29 15:58:01 PDT
Created attachment 282384 [details]
Patch
Comment 8 Michael Catanzaro 2016-06-29 15:58:50 PDT
Note the name I picked for the setting is terrible; suggestions welcome.
Comment 9 Anders Carlsson 2016-06-29 17:41:13 PDT
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.
Comment 10 Michael Catanzaro 2016-06-29 20:25:53 PDT
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?
Comment 11 Carlos Garcia Campos 2016-06-30 00:05:28 PDT
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.