1050615 - RTL Support for Status bar

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Ahmed Nefzaoui [:Nefzaoui]]

The Status bar (including Rocketbar) needs few fixes for it be RTL-compatible.
It is currently buggy when the OS is used in Arabic or any other RTL language.

Comment 1

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: Github pull-request

Patch is ready,
Review please?
Also, attaching screenshot for ui-review.

Thanks

Comment 2

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: Status Bar RTL - Screenshot after patch - 2014-08-09-05-41-09

Stephany,
Could you please take a look and tell me if this is accurate ? :)
Thanks.

Comment 3

[Ahmed Nefzaoui [:Nefzaoui]]

Please hold on reviewing, planning to rebase over the changes made in Bug 1042105.

Comment 4

[Stephany Wilkes]

OK! It looks really good Ahmed, and I am happy to re-review after rebase. Thanks so much!

Comment 5

[Stephany Wilkes]

Comment on attachment 8470407 [details]
Status Bar RTL - Screenshot after patch -  2014-08-09-05-41-09

Looks good!

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8470406 [details] [review]
Github pull-request

gmarty knows the titlebar much better than me now. Let's redirect the review to him.

Comment 7

[Guillaume Marty [:gmarty]]

Comment on attachment 8470406 [details] [review]
Github pull-request

It looks good to me! However, it needs to be rebased and some values have been updated by Bug 1056965 (like https://github.com/mozilla-b2g/gaia/pull/22693/files#diff-19076406e0c835797eb201e0143532c7R642 and possibly others too).

I can't tell anything about the code in chrome.css, so that's still relevant that someone else have a look at this patch.

Comment 8

[Ahmed Nefzaoui [:Nefzaoui]]

Comment on attachment 8470406 [details] [review]
Github pull-request

Rebased, updated and cleaned the code.
Decided to eventually use -moz-padding-start and its family to reduce the need for future RTL-specific changes when original values change.. :)
PS: This patch doesn't cover the fact that rocketbar is still fired when tapping the left part of the statusbar (while it should be fired by tapping the right part in RTL) which is a js-related bug I'm guessing.

So just to be safe, asking for review again.
Thanks!

Comment 9

[Guillaume Marty [:gmarty]]

Comment on attachment 8470406 [details] [review]
Github pull-request

Fine by me. Good job on using -moz-padding-start and -moz-margin-start!

But once again, I can tell for chrome.css, so someone more familiar should definitely gives its approval before landing.

Comment 10

[Carsten Book [:Tomcat]]

there seems to be merge conflicts at this pull request, could you take a look, thanks!

Comment 11

[Ahmed Nefzaoui [:Nefzaoui]]

Rebased and conflict solved.

Comment 12

[Carsten Book [:Tomcat]]

thanks!

master: https://github.com/mozilla-b2g/gaia/commit/55b05c5cf06bad7b177408abb9b9bc6f9dec4b09

Comment 13

[John Thomas [:Johnt]]

This issue issue is verified fixed on the Flame 2.2 KK Shallow Flash (319mb)

Result: Status appears as expected. As mentioned in Comment 8 the patch does not cover the flickering status bar when pressed on the left side of the screen.

Flame 2.2

Device: Flame 2.2 (319mb)(Kitkat Base)(Shallow Flash)
Build ID: 20141126040207
Gaia: 41b7be7c67167f367c3c4982ff08651d55455373
Gecko: 7bcc6573d204
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 14

[Peter Bylenga [:PBylenga]]

Mass Edit: adding the [rtl-meta]

Comment 15

[Lancy(Leave from Mozilla)]

Test cases has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15554/
https://moztrap.mozilla.org/manage/case/15555/