1062964 - RTL in Notifications toaster is broken

Status: VERIFIED FIXED

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

Description

[Ahmed Nefzaoui [:Nefzaoui]]

After the notification toaster visual refresh, notification toasters seem to be broken in RTL.

Comment 1

[Gregor Wagner [:gwagner]]

Can we get more info about what is broken and a screenshot?

Comment 2

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: Bug Screnshot 2014-09-04-22-02-18.png

Will be working on it, too. :)

Comment 3

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: Link to Github pull-request

It's ready.
Review, please?
Thanks

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8493538 [details] [review]
Link to Github pull-request

Thanks for the fix... I am not sure if we need an UI review here though.

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Please identify the regressed bug if you could find it.

Comment 6

[Stephany Wilkes]

Thanks, Tim. UI looks good. Can you please request approval uplift if this is low risk? Thanks!

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

We need to wait for the UI to land on master first and Ahmed should be the person asking for approval :).

Comment 8

[Guillaume Marty [:gmarty]]

Comment on attachment 8493538 [details] [review]
Link to Github pull-request

Looks all good to me.

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Ahmed, could you rebase your patch so I could merge it?

Comment 10

[Ahmed Nefzaoui [:Nefzaoui]]

Done.
Thanks! :)

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

https://github.com/mozilla-b2g/gaia/commit/201b500c4caa7f3644d6146edbe96f675fff46a6

Please set approval-gaia-v2.1 to your patch since UX would love this patch to reach v2.1.

Comment 12

[Ahmed Nefzaoui [:Nefzaoui]]

Attachment: PR - Uplift to v2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1042713
[User impact] if declined: Broken Notification toaster UI when using the device in a RTL language (e.g. Arabic, which is shipping in 2.1)
[Testing completed]: On device, Flame
[Risk to taking this patch] (and alternatives if risky): No
[String changes made]: No

Comment 13

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/3bad521efb82b8c1f371d1db4022858a2d2f169b

Comment 14

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1 and 2.2.

Result: The icon on the notification is aligned correctly.

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141104001202
Gaia: 8b0cf889ae0d48a9eb7ecdcb9b67590de45cc5e5
Gecko: 388b03efe92d
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141104040207
Gaia: 3c50520982560ccba301474d1ac43706138fc851
Gecko: 54d05732f29b
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2) 
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 15

[Peter Bylenga [:PBylenga]]

Mass Edit: adding the [rtl-meta]

Comment 16

[Alissa（leave from Mozilla）]

Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15681/