[RTL][Notifications]The ellipsis of long LTR music name is located at wrong side in notification bar and lockscreen

VERIFIED FIXED in Firefox OS v2.2

Status

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

People

(Reporter: Sue, Assigned: djf)

Tracking

(Blocks: 1 bug)

unspecified
2.2 S11 (1may)
ARM
Gonk (Firefox OS)
Bug Flags:
in-moztrap +

Firefox Tracking Flags

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

Details

(Whiteboard: [2.2-nexus-5-l])

Attachments

(6 attachments)

(Reporter)

Description

3 years ago
Created attachment 8598379 [details]
ellipsis_in_notification_bar.png

[1.Description]:
[RTL][Flame v2.2 & v3.0][Nexus 5 2.2 & 3.0][Notifications]When user plays a music with long LTR name, the ellipsis is located at wrong side on the ongoing music process in notification bar. 
See attachment:ellipsis_in_notification_bar.png

[2.Testing Steps]: 
Prerequisite: Have a music with long LTR name in device. 
1. Set system language as Arabic.
2. Launch Music app.
3. Play the music which has the long LTR name.
4. Pull down the notification bar and observe the ongoing music process on notification bar.

[3.Expected Result]: 
4. The ellipsis should be located at right side of LTR name.

[4.Actual Result]: 
4. The ellipsis is located at left side of LTR name.

[5.Reproduction build]: 
Device: Flame 2.2 (affected)
Build ID               20150427002504
Gaia Revision          265ca0bc9408c21fc4b25a259fcee7fb642cd06b
Gaia Date              2015-04-24 19:13:28
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1908685d798d
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150427.040113
Firmware Date          Mon Apr 27 04:01:24 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (affected)
Build ID               20150427160201
Gaia Revision          0636405f0844bf32451a375b2d61a2b16fe33348
Gaia Date              2015-04-27 16:42:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/caf25344f73e
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150427.192938
Firmware Date          Mon Apr 27 19:29:49 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (affected)
Build ID               20150427002504
Gaia Revision          265ca0bc9408c21fc4b25a259fcee7fb642cd06b
Gaia Date              2015-04-24 19:13:28
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1908685d798d
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150427.035747
Firmware Date          Mon Apr 27 03:58:04 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (affected)
Build ID               20150427160201
Gaia Revision          0636405f0844bf32451a375b2d61a2b16fe33348
Gaia Date              2015-04-27 16:42:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/caf25344f73e
Gecko Version          40.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150427.192534
Firmware Date          Mon Apr 27 19:25:51 EDT 2015
Bootloader             HHZ12f

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test
(Reporter)

Updated

3 years ago
QA Whiteboard: [rtl-impact]
status-b2g-v2.2: --- → affected
status-b2g-master: --- → affected
See Also: → bug 1158043
(Assignee)

Updated

3 years ago
Assignee: nobody → dflanagan
Triage -- P2
Priority: -- → P2
(Assignee)

Comment 2

3 years ago
The same bug exists for the lockscreen, and we might as well fix them both in the same patch.
Summary: [RTL][Notifications]The ellipsis of long LTR music name is located at wrong side in notification bar. → [RTL][Notifications]The ellipsis of long LTR music name is located at wrong side in notification bar and lockscreen
(Assignee)

Comment 3

3 years ago
This bug is related to bug 1150749 which added <bdi> elements to the notification screen and lockscreen to hold the artist and track name so that parentheses would display correctly. 

So to fix the parenthesis bug, the markup is now like this:

  <div><bdi>title</bdi> - <bdi>artist</bdi></div>

The text-overflow:ellipsis style is on the div element, but because the title and artist strings are isolated in bdi tags, it just uses the global rtl to decide where to put the ellipsis.

If we could assume that the title and artist names would always be in the same language and could use a single bdi element, then we could just put text-overflow:ellipsis on the bdi element and the problem would be solved.

But bug 1150749 actually showed sample screenshots where one was in Arabic and one was in English, so I think we do need both bdi elements.
(Assignee)

Comment 4

3 years ago
Created attachment 8598938 [details]
test tracks with bidi metadata

This zip file contains 8 .ogg files. They are just short recordings of my voice, but each has different metadata for artist and track name.

ee.ogg has an english track name and english artist name.
ea.ogg has an english track name and arabic artist name.
ae.ogg has an arabic track name and an english artist name.
aa.ogg has both names in arabic.

The long_ variants of these files have longer track and artist names so that we can check whether the ellipsis is displayed on the right side.

In all of the files, the track name contains parentheses so we can test that those display correctly in RTL locales.

I have no idea what the Arabic text means. I cut-and-pasted it from http://generator.lorem-ipsum.info/_arabic
(Assignee)

Comment 5

3 years ago
Comment on attachment 8598938 [details]
test tracks with bidi metadata

Delphine: do these sample tracks seem like they will be useful? Can you tell whether the Arabic is well-enough formed for them to be valid test cases?
Attachment #8598938 - Flags: feedback?(lebedel.delphine)
They seem like they are valid, however I'm not a native speaker so can't tall if the Arabic is well-enough formed.  Flagging localizers for this. thanks
Flags: needinfo?(rami223)
Flags: needinfo?(nefzaoui)
(Assignee)

Comment 7

3 years ago
I think I can fix this bug by changing the markup from:

  <div><bdi>title</bdi> - <bdi>artist</bdi></div>

To

  <bdi>title - <bdi>artist</bdi></bdi>

With this layout, the language of the song determines the overall direction.  If the song title is LTR then we have one of these:

   song - artist
   song - tsitra

And in these cases, the ellipsis is on the right if it is needed.

If the song is RTL, then we have:

   artist - gnos
   tsitra - gnos

And the ellipsis is on the left if it is needed.

This may not be perfect if artist name is a different directionality than the song name and if it is the artist name that is being truncated with the ellipsis, but I don't know how else to fix it.
(Assignee)

Comment 8

3 years ago
I suppose that a proper fix would be to redesign the UI so that the artist and song title are on separate lines and they can each have their own ellipsis on the apprpriate side based on their own direction.  But until we can make a change like that, I'll attach the patch that does what is described in comment #7
Created attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master
(Assignee)

Comment 10

3 years ago
Comment on attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master

Delphine, Rami, Ahmed: Does this patch look like it does the right thing to you?  (If you can't run the patch, could you at least give feedback on what I describe in comment #7?)  Basically, this patch uses the directionality of the song name to decide on the position of the ellipsis, even if the artist name is the thing being truncated and the artist name has a different directionality than the song name.  This doesn't seem perfect, but it is the closest I can figure out how to get.

Ted: I'm asking for your feedback because this patch modifies what you ddid for bug 1150749

Dominic: I think you're probably the original author of these controls, so you're probably the right person to review this.
Attachment #8598970 - Flags: review?(dkuo)
Attachment #8598970 - Flags: feedback?(tclancy)
Attachment #8598970 - Flags: feedback?(rami223)
Attachment #8598970 - Flags: feedback?(nefzaoui)
Attachment #8598970 - Flags: feedback?(lebedel.delphine)

Comment 11

3 years ago
Comment on attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master

David,

I think your patch does what you said in comment 7 so looks good to me, also I agree we might need to re-design the ui, or this problem seems no perfect solution for it, but the patch should be fine to handle most of the cases, thanks for working on this!
Attachment #8598970 - Flags: review?(dkuo) → review+
(Assignee)

Comment 12

3 years ago
I updated the unit tests, but didn't get them quite right. I'll update the patch with test tweaks that should not affect Dominic's r+
Comment on attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master

Sorry I'm not the right person for asking feedback and review here, so taking the flags off me. thanks
Attachment #8598970 - Flags: feedback?(lebedel.delphine)
Attachment #8598938 - Flags: feedback?(lebedel.delphine)
(Assignee)

Comment 14

3 years ago
Delphine: you set this as a P2 bug. Now that the 2.2 FC deadline has passed, I don't know if RTL bugs should still be nominated for uplift or not.
Flags: needinfo?(lebedel.delphine)
Comment on attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master

David, I don't think removing the <bdi> tags around the track's title is the right thing to do. If the user interface is in English, and the artist name is in English, but the title is in Arabic, won't the dash end up on the wrong side? It won't be between the title and artist like we expect. (That was the problem in Bug 1150749.)

But don't worry, this bug will be fixed by Bug 883884, which I'm currently working on. (I wish I could give you a workaround in the meantime, but I don't think there is a good one.)

If you want, you can mark this bug as dependent on 883884.
Attachment #8598970 - Flags: feedback-
(In reply to Ted Clancy [:tedders1] from comment #15)
If the user interface is in English, and the artist name
> is in English, but the title is in Arabic, won't the dash end up on the
> wrong side? It won't be between the title and artist like we expect.

Oh wait, nevermind. I'm lying. The dash will end up on the left, but so will the Artist name, so it will still look okay. It's not in the order the user will be expecting, but I think that's okay.

Updated

3 years ago
Attachment #8598970 - Flags: feedback+
(Assignee)

Comment 17

3 years ago
Created attachment 8600069 [details]
ltr locale, english artist, arabic song name

Ted: The mdash is still in the middle in the case you describe, though the song name is on the right of the dash instead of the left. Note (see comment #7) that there are still two bdi tags, but now one is nested inside the other rather than having them independent.

I suspect that by far the most common case will be ltr song name and ltr artist name displayed in an rtl locale, and I'm going to land this to get the ellipsis handled correctly in that case, even if the bidi handling is imperfect.
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed
Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.
(Assignee)

Comment 19

3 years ago
I'm going to override autolander on that. dkuo is the appropriate reviewer for this patch.

Landed on master: https://github.com/mozilla-b2g/gaia/commit/4e2ff35414a885831fdd8932a5ccb226b99eb030
(Assignee)

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-b2g-master: affected → fixed
Resolution: --- → FIXED
Hey David! I think that's more a question for RelMan (that said, I'm still seeing things getting approval for uplift on 2.2 today). thanks!
Flags: needinfo?(lebedel.delphine) → needinfo?(bbajaj)
Comment on attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master
Attachment #8598970 - Flags: feedback?(tclancy)

Updated

3 years ago
Attachment #8598970 - Flags: feedback-
(In reply to David Flanagan [:djf] from comment #14)
> Delphine: you set this as a P2 bug. Now that the 2.2 FC deadline has passed,
> I don't know if RTL bugs should still be nominated for uplift or not.

I think it depends on user impact/risk, but I would highly avoid taking anything less critical on 2.2 at this time.
Flags: needinfo?(bbajaj)
(Reporter)

Comment 23

3 years ago
Created attachment 8601322 [details]
verify_v3.0_pass.png

This issue has been verified passed on latest build of Flame 3.0 and Nexus 5 3.0 with the same steps in comment 0.
Result: The ellipsis of long LTR music name is shown correctly in notification bar and lockscreen.
See attachment:verify_v3.0_pass.png
Rate:0/5

Device: Flame 3.0 (pass)
Build ID               20150504160201
Gaia Revision          70077825aab2c7a79611befb40a5fe7e610d5443
Gaia Date              2015-05-04 18:09:33
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/102d0e9aa9e1
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150504.193019
Firmware Date          Mon May  4 19:30:30 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 3.0 (pass)
Build ID               20150504160201
Gaia Revision          70077825aab2c7a79611befb40a5fe7e610d5443
Gaia Date              2015-05-04 18:09:33
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/102d0e9aa9e1
Gecko Version          40.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150504.193136
Firmware Date          Mon May  4 19:31:54 EDT 2015
Bootloader             HHZ12f
(Reporter)

Updated

3 years ago
QA Whiteboard: [rtl-impact] → [rtl-impact][MGSEI-Triage+]
status-b2g-master: fixed → verified

Comment 24

3 years ago
From image, I can see that the song name in Arabic (right aligned) and the artist name in English correctly aligned.
Looks ok from my side
Flags: needinfo?(rami223)

Updated

3 years ago
Attachment #8598970 - Flags: feedback?(rami223) → feedback+
(Reporter)

Comment 25

3 years ago
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/16230/
Flags: in-moztrap+
(Reporter)

Comment 26

3 years ago
Hi David,
Could you please help to confirm whether it will uplift to v2.2 or not? Thanks :)
Flags: needinfo?(dflanagan)
(Assignee)

Comment 27

3 years ago
Comment on attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master

I didn't get a clear answer from Delphine or Bhavana about whether this patch should be uplifted, so I guess I'll go ahead and nominate it and let someone else decide, since the reporter has pinged me about uplift.

I honestly don't know how urgent RTL bugs like this are still considered for 2.2. This particular bug is kind of a corner case since it only affects where the ellipsis goes when the song name or artist name is too long. But it does affect text that appears on the lockscreen, so I suppose that could make it more visible and higher priority to fix.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): not a regression, just a very tricky bidi corner case

[User impact] if declined: ellipsis may be on the wrong side for long song names or artist names when direction of locale and music metadata do not match

[Testing completed]: yes

[Risk to taking this patch] (and alternatives if risky): a relatively small patch that has been on master for a couple of weeks, so not very risky.

[String changes made]: none
Flags: needinfo?(dflanagan)
Attachment #8598970 - Flags: approval-gaia-v2.2?(jocheng)
(In reply to David Flanagan [:djf] from comment #27)
> Comment on attachment 8598970 [details] [review]
> [gaia] davidflanagan:bug1159092 > mozilla-b2g:master
> 
> I didn't get a clear answer from Delphine or Bhavana about whether this
> patch should be uplifted, so I guess I'll go ahead and nominate it and let
> someone else decide, since the reporter has pinged me about uplift.

Hey David - yeah I'm an l10n-driver so it's not my decision if this gets uplifted or not. Need info'ing Josh for decision since he's RelMan now is the right way to go forwards :) thanks!
Hi Josh,

Could you help to confirm if it will uplift to v2.2? 

Thanks
Flags: needinfo?(jocheng)

Comment 30

3 years ago
Comment on attachment 8598970 [details] [review]
[gaia] davidflanagan:bug1159092 > mozilla-b2g:master

Approving and please NI if causing any issue.

Hi Norry,
Please verify on 2.2 with LTR and RTL to make sure no regression.
Thanks.
Flags: needinfo?(jocheng) → needinfo?(fan.luo)
Attachment #8598970 - Flags: approval-gaia-v2.2?(jocheng) → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/63a79277938f20ac4907a4f8802180ced44cf5cd
status-b2g-v2.2: affected → fixed
Target Milestone: --- → 2.2 S11 (1may)
(Reporter)

Comment 32

3 years ago
Created attachment 8609955 [details]
verify_v2.2_pass.png

This issue has been verified as pass on latest build of Flame 2.2 and Nexus 5 2.2 by STRs in comment 0.
Result:The ellipsis of long LTR music name is shown correctly in notification bar and lockscreen.
See attachment:verify_v2.2_pass.png
Rate:0/5

Device: Flame 2.2 (pass)
Build ID               20150524002504
Gaia Revision          de5942bd99b801d4d04b676f1e70a77e93115d95
Gaia Date              2015-05-22 19:38:55
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1bd024f4e171
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150524.040835
Firmware Date          Sun May 24 04:08:46 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (pass)
Build ID               20150524002504
Gaia Revision          de5942bd99b801d4d04b676f1e70a77e93115d95
Gaia Date              2015-05-22 19:38:55
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/1bd024f4e171
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150524.043322
Firmware Date          Sun May 24 04:33:38 EDT 2015
Bootloader             HHZ12f
Flags: needinfo?(fan.luo)
(Reporter)

Updated

3 years ago
Status: RESOLVED → VERIFIED
status-b2g-v2.2: fixed → verified
Flags: needinfo?(nefzaoui)
Attachment #8598970 - Flags: feedback?(nefzaoui)
You need to log in before you can comment on or make changes to this bug.