Bug 198707 - [GTK] The Previous/Next gesture should handle RTL
Summary: [GTK] The Previous/Next gesture should handle RTL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: Other
Hardware: Unspecified Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-09 23:40 PDT by Adrien Plazas
Modified: 2019-06-20 08:56 PDT (History)
9 users (show)

See Also:


Attachments
Patch (6.35 KB, patch)
2019-06-20 07:18 PDT, Alexander Mikhaylenko
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrien Plazas 2019-06-09 23:40:00 PDT
The Previous/Next gesture (two-fingers horizontal drag) should be inversed on RTL languages, as the newer page should move to/come from the left instead of the right.

This affects GNOME Web: https://gitlab.gnome.org/GNOME/epiphany/issues/820.
Comment 1 Alexander Mikhaylenko 2019-06-20 02:28:14 PDT
Hm, it looks like WebKit in general doesn't support at least GtkInspector's text direction override. Scrollbars are still on the right, for example.
Comment 2 Alexander Mikhaylenko 2019-06-20 02:47:40 PDT
Nevermind, it seems to be flipped for RTL pages only.

However, `m_webPageProxy.userInterfaceLayoutDirection()` seems to always return LTR direction, which is why it doesn't work.

I wonder if it would work with an actual RTL locale.

Other than that, I need to adjust `ViewGestureController::PendingSwipeTracker::scrollEventGetScrollingDeltas()` and drawing. The pages comes from the correct side, but the order is reversed: the current page is on the bottom when you're going back, and on the top if you're going forward. It should be the other way.
Comment 3 Alexander Mikhaylenko 2019-06-20 07:18:26 PDT
Created attachment 372552 [details]
Patch
Comment 4 Build Bot 2019-06-20 07:22:04 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 5 Michael Catanzaro 2019-06-20 07:32:50 PDT
(In reply to Alexander Mikhaylenko from comment #2)
> Nevermind, it seems to be flipped for RTL pages only.
> 
> However, `m_webPageProxy.userInterfaceLayoutDirection()` seems to always
> return LTR direction, which is why it doesn't work.
> 
> I wonder if it would work with an actual RTL locale.

Currently we have this in our PageClientImpl.h:

WebCore::UserInterfaceLayoutDirection userInterfaceLayoutDirection() override { return WebCore::UserInterfaceLayoutDirection::LTR; }

Something like this in our PageClientImpl.cpp would probably suffice:

WebCore::UserInterfaceLayoutDirection PageClientImpl::userInterfaceLayoutDirection()
{
    return gtk_widget_get_direction(m_viewWidget) == GTK_TEXT_DIR_RTL ?
        WebCore::UserInterfaceLayoutDirection::RTL : WebCore::UserInterfaceLayoutDirection::LTR;
}
Comment 6 Michael Catanzaro 2019-06-20 07:33:23 PDT
Next time, I should read your patch *before* wasting time investigating the problem you've already solved. :)
Comment 7 Michael Catanzaro 2019-06-20 07:34:50 PDT
Comment on attachment 372552 [details]
Patch

This could improve a bunch of layout test results. I suppose we'll find out.
Comment 8 WebKit Commit Bot 2019-06-20 08:55:58 PDT
Comment on attachment 372552 [details]
Patch

Clearing flags on attachment: 372552

Committed r246635: <https://trac.webkit.org/changeset/246635>
Comment 9 WebKit Commit Bot 2019-06-20 08:56:00 PDT
All reviewed patches have been landed.  Closing bug.