[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

Triage -- P2

The same bug exists for the lockscreen, and we might as well fix them both in the same patch.

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.

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

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?

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

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.

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

Comment on attachment 8598970 [details] [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.

Comment on attachment 8598970 [details] [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!

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] [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

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.

Comment on attachment 8598970 [details] [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.

(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.

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.

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.

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

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!

(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.

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

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

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

Hi David, Could you please help to confirm whether it will uplift to v2.2 or not? Thanks :)

Comment on attachment 8598970 [details] [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

(In reply to David Flanagan [:djf] from comment #27) > Comment on attachment 8598970 [details] > [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

Comment on attachment 8598970 [details] [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.

v2.2: https://github.com/mozilla-b2g/gaia/commit/63a79277938f20ac4907a4f8802180ced44cf5cd

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