[RTL][Music] Translated text on music tiles is not right aligned in Arabic

VERIFIED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::Music
P2
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Brogan Zumwalt [Inactive], Assigned: squib)

Tracking

unspecified
2.2 S10 (17apr)
ARM
Gonk (Firefox OS)
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(feature-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [3.0-Daily-Testing])

Attachments

(7 attachments)

(Reporter)

Description

3 years ago
Created attachment 8563645 [details]
Screenshot

Description:
When language is set to Arabic, the text on the tiles in the music app that appears in Arabic (e.g. "Unknown Artist", "Unknown Album", etc.) is not right aligned. 

Repro Steps:
1) Update a Flame to 20150212010213
2) Set language to Arabic
3) Have a song with an unknown/blank artist and/or album title
4) Launch Music app


Actual:
Translated text on tiles in music app are not right aligned when language is set to Arabic.

Expected:
All translated text in music app is right aligned when language is set to Arabic.

Environmental Variables:
Device: Flame 3.0 Master
Build ID: 20150212010213
Gaia: d5a71cedb37dd45f439f672489db3994b349ac43
Gecko: 3094601af679
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Repro frequency: 3/3, 100%
See attached: screenshot
(Reporter)

Comment 1

3 years ago
Issue DOES occur on Flame 2.2

Translated text on tiles in music app are not right aligned when language is set to Arabic.

Device: Flame 2.2
Build ID: 20150212002504
Gaia: 791e53728cd8018f1d7cf7efe06bbeb1179f0370
Gecko: 5dec207fcbeb
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage?][rtl-impact]
status-b2g-v2.2: --- → affected
Flags: needinfo?(ktucker)
feature-b2g: --- → 2.2+
Priority: -- → P2
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(ktucker)
Assignee: nobody → nefzaoui
Created attachment 8565376 [details] [review]
[gaia] anefzaoui:bug-1132666 > mozilla-b2g:master
Attachment #8565376 - Flags: review?(dkuo)

Comment 3

3 years ago
Ahmed,

Dominic is probably out because of new year. Please route review request to :squib (Jim Porter)

Thanks
Hema
Comment on attachment 8565376 [details] [review]
[gaia] anefzaoui:bug-1132666 > mozilla-b2g:master

(In reply to Hema Koka [:hema] from comment #3)
> Ahmed,
> 
> Dominic is probably out because of new year. Please route review request to
> :squib (Jim Porter)
> 
> Thanks
> Hema

Thanks for letting me know!
Attachment #8565376 - Flags: review?(dkuo) → review?(squibblyflabbetydoo)
(Assignee)

Comment 5

3 years ago
Comment on attachment 8565376 [details] [review]
[gaia] anefzaoui:bug-1132666 > mozilla-b2g:master

This looks sane, I guess. I'm not really a good person to ask about RTL issues. Is it normal for Latin text to be left-aligned in the same UI as Arabic text is right-aligned, or should we pick an alignment based on the user's locale?
Attachment #8565376 - Flags: review?(squibblyflabbetydoo) → review+
Comment on attachment 8565376 [details] [review]
[gaia] anefzaoui:bug-1132666 > mozilla-b2g:master

Hey Stephany,
Sorry to add even more load to your work but could you please give quick insight on this or assign someone to do ?
Thanks :)
Attachment #8565376 - Flags: ui-review?(swilkes)

Comment 7

3 years ago
Jim, according to the pattern, Latin character text is right-aligned in instances where the rest of the Arabic UI is RTL.

Comment 8

3 years ago
Comment on attachment 8565376 [details] [review]
[gaia] anefzaoui:bug-1132666 > mozilla-b2g:master

Seeing if Tif can review this patch on our big, busy FL day.
Attachment #8565376 - Flags: ui-review?(swilkes) → ui-review?(tshakespeare)
Comment on attachment 8565376 [details] [review]
[gaia] anefzaoui:bug-1132666 > mozilla-b2g:master

Per Steph's comment 7  - Arabic is right aligned but Latin is still left-aligned.
Attachment #8565376 - Flags: ui-review?(tshakespeare) → ui-review-
Created attachment 8568224 [details]
latin characters are left-aligned

See my previous comment
I guess then that makes the purpose of the bug is invalid, and then so does the bug itself :)
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INVALID
no no, the bug is still very valid - if you look at the original screenshot and STR, everything was left aligned. The patch fixes the alignment of items translated to Arabic (e.g. "unknown album") but Latin characters (e.g. Crystal Method) are still left-aligned when the should also be right aligned (Steph's comment)
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(In reply to Tiffanie Shakespeare from comment #12)
> no no, the bug is still very valid ... but Latin characters (e.g.
> Crystal Method) are still left-aligned when they should also be right aligned
> (Steph's comment)

Right, you can see the inconsistency if you go into the now playing screen. There you will find that the Latin chars are right-aligned as expected. 

Jim, Ahmed is no longer working on bugs for 2.2. Are you available to take over on this one to fix the alignment?
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 14

3 years ago
I can look at this.

Tif: Just to be clear, *all* the artist/album info - no matter the language of each item - should be left-aligned when the UI's locale is LTR, and right-aligned when the UI's locale is RTL, correct?
Assignee: nefzaoui → squibblyflabbetydoo
Status: REOPENED → ASSIGNED
Flags: needinfo?(squibblyflabbetydoo) → needinfo?(tshakespeare)
Yes, we want to consistently align the text. So everything should be right-aligned, including Latin strings, when the locale is a RTL language.

In looking at the patch, the Arabic was being right-aligned, but the English was not.

Thanks Jim!
Flags: needinfo?(tshakespeare)

Comment 16

3 years ago
Dear Jim,
Could you kindly provide ETA date (Target Milestone) for this issue? Thank you very much!
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 17

3 years ago
I'll try to get to this by the end of the week, but I'm looking at other 2.2 blockers at the moment.
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Updated

3 years ago
Target Milestone: --- → 2.2 S7 (6mar)
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15195/
Flags: in-moztrap+
Created attachment 8575060 [details] [review]
[gaia] jimporter:music-rtl-tiles > mozilla-b2g:master
(Assignee)

Comment 20

3 years ago
Comment on attachment 8575060 [details] [review]
[gaia] jimporter:music-rtl-tiles > mozilla-b2g:master

Hub: You know a little about RTL, I think. Could you look at this?
Attachment #8575060 - Flags: review?(hub)
(Assignee)

Updated

3 years ago
Target Milestone: 2.2 S7 (6mar) → 2.2 S8 (20mar)
Comment on attachment 8575060 [details] [review]
[gaia] jimporter:music-rtl-tiles > mozilla-b2g:master

Looks good to me.
Attachment #8575060 - Flags: review?(hub) → review+
(Assignee)

Comment 22

3 years ago
Comment on attachment 8575060 [details] [review]
[gaia] jimporter:music-rtl-tiles > mozilla-b2g:master

Tif: Just wanted to make sure this looks right for you. Unfortunately, I don't have any MP3s with Arabic metadata to test...
Attachment #8575060 - Flags: ui-review?(tshakespeare)
Hey Jim I'm having issues testing the patch. Arabic disappears from the language list when I install your patch :-/

I also found another bug while trying to get this working - when English is truncated it's truncated on the left (e.g. ...stal Method) I don't know if you want to file a new bug for this or tackle in this one since it's related to switching the alignment to the right.
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 24

3 years ago
(In reply to Tiffanie Shakespeare from comment #23)
> Hey Jim I'm having issues testing the patch. Arabic disappears from the
> language list when I install your patch :-/

Can you use Mirrored English instead? That should be available in debug builds.

> I also found another bug while trying to get this working - when English is
> truncated it's truncated on the left (e.g. ...stal Method) I don't know if
> you want to file a new bug for this or tackle in this one since it's related
> to switching the alignment to the right.

That's probably a different bug; I'd guess it's related to bug 1140154.
Flags: needinfo?(squibblyflabbetydoo)
Mirrored English works and looks like the bug is solved.

I checked out the patch in bug 1140154 and it doesn't seem to fix the truncation issue. So IDK if you want that patch to update with a fix or update it here.

Thanks Jim!
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 26

3 years ago
Well, then I don't know. I'm not really good with RTL stuff, but this might be a bug in Gecko.
Flags: needinfo?(squibblyflabbetydoo)

Comment 27

3 years ago
Hi Lancy,
Can you verify whether the patch https://bugzilla.mozilla.org/attachment.cgi?id=8575060 fix the issue? Thanks!
Flags: needinfo?(yulan.zhu)

Updated

3 years ago
Target Milestone: 2.2 S8 (20mar) → 2.2 S9 (3apr)
Hey Jim - can you send my a zip file of the music app with the patch? I want to see if I can get Arabic working with the changes you made. 

Also, who can we check with to nail down where the weird truncation issue is occurring? This definitely needs to be fixed.

Thanks!
Flags: needinfo?(squibblyflabbetydoo)
Jim - n/m on the second issue. Dylan helped me track done a bug about this issue occurring in other apps too.

https://bugzilla.mozilla.org/show_bug.cgi?id=1143599
(Assignee)

Comment 30

3 years ago
I have no idea how to generate a zip file for a given app. It *should* work if you just check out my branch and run

  APP=music make install-gaia

from the terminal. That would only update the music app, and not the rest of the system.
Flags: needinfo?(squibblyflabbetydoo)
Jim, we resolved to just land this one based on hub's r+ and Tif will do her UI review afterward. Can you go ahead and land this on master?
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Updated

3 years ago
Flags: needinfo?(squibblyflabbetydoo)
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed
http://docs.taskcluster.net/tools/task-graph-inspector/#TRhCD7tlSJy1HgM-9TxBQw

The pull request failed to pass integration tests. It could not be landed, please try again.
http://docs.taskcluster.net/tools/task-graph-inspector/#_1jUKHY0RaWtfdWXbIMmOQ

The pull request failed to pass integration tests. It could not be landed, please try again.
Can you provide an update on this bug?
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 35

3 years ago
I'm trying to figure out why the tests are failing in a way that prevents landing. I'm not sure if it's my fault or just the inherent instability of our tests, though.
Flags: needinfo?(squibblyflabbetydoo)

Updated

3 years ago
Whiteboard: [3.0-Daily-Testing] → [3.0-Daily-Testing], MGSEI-RTL-3F

Comment 36

3 years ago
Hi Jim,
DO you have any update for this issue?
Thanks!
Flags: needinfo?(squibblyflabbetydoo)
(In reply to Josh Cheng [:josh] from comment #27)
> Hi Lancy,
> Can you verify whether the patch
> https://bugzilla.mozilla.org/attachment.cgi?id=8575060 fix the issue? Thanks!

Hi Josh,

After building the patch with flame 3.0 latest gaia, this issue dose not exist.
See the attachment:Verify1_Translated text.png
Reproducing rate:0/10

Flame 3.0 build:
Build ID               20150412160203
Gaia Revision          6414097adfb054260e47cf72a9d1b80eaecef408
Gaia Date              2015-04-13 06:24:39
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0a46652bd992
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150412.192808
Firmware Date          Sun Apr 12 19:28:21 EDT 2015
Bootloader             L1TC000118D0
Flags: needinfo?(yulan.zhu) → needinfo?(jocheng)
Created attachment 8592107 [details]
Verify1_Translated text.png
Lancy, can you verify that english is also right-aligned? Thanks!
Flags: needinfo?(yulan.zhu)
Hi Tiffanie,
For Arabic language will only exist via building gaia with the patch, we tried this method, but something is wrong with en-US language package. 
Instead, if we build the image with the patch, the Arabic language does not exist. Can we verify this issue in English via building the image with the patch?
Flags: needinfo?(yulan.zhu) → needinfo?(tshakespeare)
Sorry Lancy I'm not quite following and re-reading my question I apologize for not being more clear. :-/

At one point (see comment 15) artists in Arabic or the translated word "unknown" was getting right-aligned but artists in English (e.g. Daft Punk) were being left-aligned.

In the screenshot you posted in comment 38, I only see Arabic and some other language or characters - I'm not sure what it is. 

I wanted to double check that an artist in English was still being right-aligned. Thanks :)
Flags: needinfo?(tshakespeare)
(Assignee)

Updated

3 years ago
Flags: needinfo?(squibblyflabbetydoo)
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed
http://docs.taskcluster.net/tools/task-graph-inspector/#jLcYtyoRRBe5vcqj4HbXVw

The pull request failed to pass integration tests. It could not be landed, please try again.

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED

Updated

3 years ago
Flags: needinfo?(yulan.zhu)
Created attachment 8594624 [details]
Verify2_Flame3.0_Pass.png

According to the steps in comment 0, this issue is verified pass on latest Flame 3.0 build.
See attachment: Verify2_Flame3.0_Pass.png
Reproducing rate:0/10

Flame 3.0 build:
Build ID               20150419160202
Gaia Revision          c6b04efa0f31a584e6ee0a46dd2b64c1e3c29adc
Gaia Date              2015-04-17 21:10:53
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/a32e3b93c8d8
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150419.193329
Firmware Date          Sun Apr 19 19:33:40 EDT 2015
Bootloader             L1TC000118D0
Flags: needinfo?(yulan.zhu)
QA Whiteboard: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage+][rtl-impact][MGSEI-Triage+]
status-b2g-master: affected → verified

Comment 45

3 years ago
Hi Jim,
Can you also raise approval and pull request for 2.2?
Thanks!
Flags: needinfo?(jocheng) → needinfo?(squibblyflabbetydoo)
(In reply to Tiffanie Shakespeare from comment #41)
> Sorry Lancy I'm not quite following and re-reading my question I apologize
> for not being more clear. :-/
> 
> At one point (see comment 15) artists in Arabic or the translated word
> "unknown" was getting right-aligned but artists in English (e.g. Daft Punk)
> were being left-aligned.
> 
> In the screenshot you posted in comment 38, I only see Arabic and some other
> language or characters - I'm not sure what it is. 
> 
> I wanted to double check that an artist in English was still being
> right-aligned. Thanks :)

Hi Tiffanie,
 Sorry for the late reply, could you please review the attachment in comment 44? Translated text is left-aligned in English, and this issue has been fixed in latest Flame 3.0 build.
Flags: needinfo?(tshakespeare)
Comment on attachment 8575060 [details] [review]
[gaia] jimporter:music-rtl-tiles > mozilla-b2g:master

Cool and thanks Lancy!
Flags: needinfo?(tshakespeare)
Attachment #8575060 - Flags: ui-review?(tshakespeare) → ui-review+
(Assignee)

Comment 48

3 years ago
Comment on attachment 8575060 [details] [review]
[gaia] jimporter:music-rtl-tiles > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A
[User impact] if declined: RTL locales will look bad if there is LTR artist/album names with parens in them
[Testing completed]: Manually tested
[Risk to taking this patch] (and alternatives if risky): Low risk, just a small CSS change
[String changes made]: None
Attachment #8575060 - Flags: approval-gaia-v2.2?
Flags: needinfo?(squibblyflabbetydoo)
(Assignee)

Comment 49

3 years ago
(In reply to Jim Porter (:squib) from comment #48)
> Comment on attachment 8575060 [details] [review]
> [gaia] jimporter:music-rtl-tiles > mozilla-b2g:master

Er, I'm confused. This just changes the alignment of text so that it's consistent in RTL

Updated

3 years ago
Attachment #8575060 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/0816f66ea2d30a0a6dcd9b33c1b7008ec80fa540
status-b2g-v2.2: affected → fixed
Target Milestone: 2.2 S9 (3apr) → 2.2 S10 (17apr)

Comment 51

3 years ago
Created attachment 8597045 [details]
verified_v2.2_pass.png

This issue has been verified passed on latest build of Flame 2.2 with the same steps in comment 0.
See attachment:verified_v2.2_pass.png
Rate:0/3

Device: Flame 3.0 (pass)
Build ID               20150423160207
Gaia Revision          0c5e2ee1173f3c53379ef3cd10de714836258fe8
Gaia Date              2015-04-23 16:10:10
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/22a157f7feb7
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150423.193607
Firmware Date          Thu Apr 23 19:36:18 EDT 2015
Bootloader             L1TC000118D0

Comment 52

3 years ago
Sorry for the wrong device information in comment 51, update it:

Device: Flame 2.2 (pass)
Build ID               20150423162502
Gaia Revision          b838d0e7c163e66660dcb6e387d8339944a7a30e
Gaia Date              2015-04-23 02:32:46
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5fe76b26e55f
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150423.195827
Firmware Date          Thu Apr 23 19:58:39 EDT 2015
Bootloader             L1TC000118D0
Status: RESOLVED → VERIFIED
status-b2g-v2.2: fixed → verified
QA Whiteboard: [QAnalyst-Triage+][rtl-impact][MGSEI-Triage+] → [QAnalyst-Triage+][rtl-impact][MGSEI-Triage+][MGSEI-RTL-3F]
Whiteboard: [3.0-Daily-Testing], MGSEI-RTL-3F → [3.0-Daily-Testing]
You need to log in before you can comment on or make changes to this bug.