883673 - [BB][Headers] RTL is messed up

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Ahmed Nefzaoui [:Nefzaoui]]

User Agent: Mozilla/5.0 (Windows NT 6.2; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130511120803

Steps to reproduce:

All the apps are affected by this bug, which is when using an RTL-language (Arabic), headers are messed up

Comment 1

[Alexandre BM (:rednaks)]

Thanks Ahmed for re-opening this discussion, there is an old patch that still not merged if it can help ! bug 834464
I think that you also worked on the headers issue right ?

Comment 2

[Ahmed Nefzaoui [:Nefzaoui]]

Yes I worked on them before but to be more specific, The right thing was to open a bug for all headers, since all the apps are affected by this bug, while the other one is meant for Settings.. And for the patch of bug 834464 Yes it really needs to be merged ASAP if it's confirmed as fixing the bug itself..

Comment 3

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: Patch making RTL headers bug free

Comment 4

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: PR to Github

A much more mature patch fixing all the bugs appearing on Headers in RTL mode.
With the minimum possible of additions.

Comment 5

[Ahmed Nefzaoui [:Nefzaoui]]

Since bug 925929 seems to be ready, should I make changes for this comment to suit up with it?

Comment 6

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: RTL Headers after patch

Comment 8

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: RTL Headers' fix

Since headers' visual refresh has landed. This is the new patch. :)

Comment 9

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: Pull Request to proposed patch

Comment 10

[Fabien Cazenave [:kaze]]

Comment on attachment 8357706 [details] [review]
Pull Request to proposed patch

LGTM but please address my comment on github. Thanks for your good work!

Comment 11

[Fabien Cazenave [:kaze]]

Comment on attachment 8357706 [details] [review]
Pull Request to proposed patch

Pavel: could you please review this patch? On second thought I’m not really sure this patch is as safe as I expected.

Comment 12

[Ahmed Nefzaoui [:Nefzaoui]]

BTW, I just added the PR by kaze to my PR's branch, it should be fine now
And sorry for delay :)

Comment 13

[Fabien Cazenave [:kaze]]

(In reply to Fabien Cazenave [:kaze] from comment #11)
> Pavel: could you please review this patch? On second thought I’m not really
> sure this patch is as safe as I expected.

Ooops, forget the nonsense. The patch makes perfect sense to me — sue me if bad things happen.

Comment 14

[Fabien Cazenave [:kaze]]

Comment on attachment 8357706 [details] [review]
Pull Request to proposed patch

Merged on master: https://github.com/mozilla-b2g/gaia/commit/91ca31746a6253349daa589cb6cf7e3c6cc43c46

Comment 15

[Fabien Cazenave [:kaze]]

Thanks a lot Ahmed!

Comment 16

[Peter Bylenga [:PBylenga]]

Mass Edit: adding the [rtl-meta]