1321981 - [RTL] Swiping gesture between Top Sites, Bookmarks and History in a New Tab is reversed

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect, P2)

Description

[Itiel]

In latest Nightly builds, swiping from one section in a New Tab to another is reversed.

STR:
1. Install RTL Nightly build of Firefox for Android
2. Open a New Tab
3. Swipe RIGHT to go to Bookmarks -> nothing happens
4. Swipe LEFT (nothing should happen) -> Firefox goes to Bookmarks

In short, swiping right should go left, and swiping left should go right. Same as in LTR builds.

Comment 1

[Ioana Chiorean]

I am able to reproduce this. 
Max, is this something the framework permits?

Comment 2

[Max Liu [:maliu]]

(In reply to Ioana Chiorean from comment #1)
> I am able to reproduce this. 
> Max, is this something the framework permits?

The root cause of this bug is that we use ViewPager from google supoprt library that does not support RTL. Better solution could be replacing it with RecyclerView and some scrolling tweak. Unfortunately, we probable have not enough time to fix it before 53 merge.

Comment 3

[Ioana Chiorean]

Max any updates here?

Comment 4

[Max Liu [:maliu]]

(In reply to Ioana Chiorean from comment #3)
> Max any updates here?

I found a solution for this bug. But I need to check with Sebastian to confirm our external open source library policy. The patch should be low risk for uplift. I'll try to land the patch before next week;

Comment 5

[Max Liu [:maliu]]

Attachment: Bug 1321981 - [RTL] ViewPager Support - part 1. Import thirdparty RtlViewPager,

Review commit: https://reviewboard.mozilla.org/r/122236/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122236/

Comment 6

[Max Liu [:maliu]]

Attachment: Bug 1321981 - [RTL] ViewPager Support - part 2. Notify dataset change with calling super,

Review commit: https://reviewboard.mozilla.org/r/122238/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122238/

Comment 7

[Max Liu [:maliu]]

Attachment: Bug 1321981 - [RTL] ViewPager Support - part 3. Skip set current item if adapter is empty,

Review commit: https://reviewboard.mozilla.org/r/122240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122240/

Comment 8

[Max Liu [:maliu]]

Attachment: Bug 1321981 - [RTL] ViewPager Support - part 4. Switch to RtlViewPager,

Review commit: https://reviewboard.mozilla.org/r/122242/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122242/

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849458 [details]
Bug 1321981 - [RTL] ViewPager Support - part 1. Import thirdparty RtlViewPager,

https://reviewboard.mozilla.org/r/122236/#review124380

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849459 [details]
Bug 1321981 - [RTL] ViewPager Support - part 2. Notify dataset change with calling super,

https://reviewboard.mozilla.org/r/122238/#review124382

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849460 [details]
Bug 1321981 - [RTL] ViewPager Support - part 3. Skip set current item if adapter is empty,

https://reviewboard.mozilla.org/r/122240/#review124384

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8849461 [details]
Bug 1321981 - [RTL] ViewPager Support - part 4. Switch to RtlViewPager,

https://reviewboard.mozilla.org/r/122242/#review124386

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/094d01c3c450
[RTL] ViewPager Support - part 1. Import thirdparty RtlViewPager, r=sebastian
https://hg.mozilla.org/integration/autoland/rev/e0c93b49a773
[RTL] ViewPager Support - part 2. Notify dataset change with calling super, r=sebastian
https://hg.mozilla.org/integration/autoland/rev/b03a53a62cfe
[RTL] ViewPager Support - part 3. Skip set current item if adapter is empty, r=sebastian
https://hg.mozilla.org/integration/autoland/rev/2ed5a83f2ae2
[RTL] ViewPager Support - part 4. Switch to RtlViewPager, r=sebastian

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/094d01c3c450
https://hg.mozilla.org/mozilla-central/rev/e0c93b49a773
https://hg.mozilla.org/mozilla-central/rev/b03a53a62cfe
https://hg.mozilla.org/mozilla-central/rev/2ed5a83f2ae2

Comment 15

[Itiel]

This seems to work very well in latest Nightly. Can someone with a tablet also confirm?