If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

RTL in Notifications toaster is broken

VERIFIED FIXED in Firefox OS v2.1

Status

Firefox OS
Gaia::System
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Nefzaoui, Assigned: Nefzaoui)

Tracking

(Blocks: 1 bug, {regression})

unspecified
2.1 S5 (26sep)
All
Gonk (Firefox OS)
regression
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(b2g-v2.1 verified, b2g-v2.2 verified)

Details

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
After the notification toaster visual refresh, notification toasters seem to be broken in RTL.
(Assignee)

Updated

3 years ago
Blocks: 906270
Can we get more info about what is broken and a screenshot?
(Assignee)

Comment 2

3 years ago
Created attachment 8484493 [details]
Bug Screnshot 2014-09-04-22-02-18.png

Will be working on it, too. :)
Assignee: nobody → nefzaoui.ahmed
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8493538 [details] [review]
Link to Github pull-request

It's ready.
Review, please?
Thanks
Attachment #8493538 - Flags: review?(timdream)
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.
Attachment #8493538 - Flags: review?(timdream)
Attachment #8493538 - Flags: review+
Attachment #8493538 - Flags: feedback?(gmarty)
Please identify the regressed bug if you could find it.
Keywords: regression

Comment 6

3 years ago
Thanks, Tim. UI looks good. Can you please request approval uplift if this is low risk? Thanks!
Flags: needinfo?(timdream)
We need to wait for the UI to land on master first and Ahmed should be the person asking for approval :).
Flags: needinfo?(timdream)
Comment on attachment 8493538 [details] [review]
Link to Github pull-request

Looks all good to me.
Attachment #8493538 - Flags: feedback?(gmarty) → feedback+
Ahmed, could you rebase your patch so I could merge it?
Flags: needinfo?(nefzaoui.ahmed)
(Assignee)

Comment 10

3 years ago
Done.
Thanks! :)
Flags: needinfo?(nefzaoui.ahmed)
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → fixed
Resolution: --- → FIXED
(Assignee)

Comment 12

3 years ago
Created attachment 8494462 [details] [review]
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
Attachment #8494462 - Flags: approval-gaia-v2.1?
Blocks: 1042713
Attachment #8494462 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
v2.1: https://github.com/mozilla-b2g/gaia/commit/3bad521efb82b8c1f371d1db4022858a2d2f169b
status-b2g-v2.1: affected → fixed
Keywords: checkin-needed
Target Milestone: --- → 2.1 S5 (26sep)
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.1: fixed → verified
status-b2g-v2.2: fixed → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Mass Edit: adding the [rtl-meta]
Whiteboard: [rtl-meta]
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][rtl-impact]
Whiteboard: [rtl-meta]
Blocks: 1101117
No longer blocks: 906270
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15681/
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.