Closed Bug 1156571 Opened 10 years ago Closed 10 years ago

[RTL][Music] Songs with parentheses are not displayed correctly in Music app

Categories

(Firefox OS Graveyard :: Gaia::Music, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

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

VERIFIED FIXED
2.2 S11 (1may)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: pcheng, Assigned: hub)

References

Details

(Whiteboard: [3.0-Daily-Testing])

Attachments

(3 files)

Attached audio song used for testing with parentheses (deleted) —
Description: In Music app tile view, as well as active song playing view, parentheses are not displayed correctly in Arabic. STR: 0) Set device language to Arabic 1) Put the attached song to the device 2) Go to Music app observe the song on tile view 3) Start playing the song, and observe the song playing screen 4) Tap on screen to invoke the share menu Expected on step 2, 3, and 4: Parentheses are displayed correctly. Actual on step 2, 3, and 4: Parentheses are mismatched. See screenshots on next comment. Device: Flame 3.0 BuildID: 20150420010204 Gaia: cb41d8421da5dc4f16ea566ea2917a9b7f828154 Gecko: 50b95032152c Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b Version: 40.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0
Attached image screenshots.png (deleted) —
2.2 is also affected. Device: Flame 2.2 BuildID: 20150420002502 Gaia: c15a2b6d3a783813959c2b3bffd2a131f4270b9e Gecko: cc02ee38b252 Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429 Version: 37.0 (2.2) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 Attaching screenshots of this issue.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [3.0-Daily-Testing]
OS: Linux → Gonk (Firefox OS)
Hardware: x86 → ARM
Blocks: music-rtl
QA Whiteboard: [QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage?]
This is a regression: it got fixed a month back in Bug 1140154 Triage: P2 -- nominating
blocking-b2g: --- → 2.2?
Keywords: regression
Priority: -- → P2
Note: only the behavior observed at step 3 of the STR is a regression, the rest is not regression.
Also, I realize I must have been looking at screenshots from Bug 1140154 instead of this one, so I thought this was exactly the same issue on the same screen. Looking once more at the screenshots here, I realize they are not from exactly the same screen
QA Whiteboard: [rtl-impact][QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
I'm disappointed that we didn't get these cases handled as part of bug 1140154. I suspect the fix here is just to add <bdi> elements around the song name in the gaia-header and in the overlay where the artist name is displayed. I've asked Hub to take this one.
Assignee: nobody → hub
Blocking Reason: poor ux and a missed test case from previous attempt to fix.
blocking-b2g: 2.2? → 2.2+
Comment on attachment 8596146 [details] [gaia] hfiguiere:bug1156571-bidi-fixes > mozilla-b2g:master This should fix the issues.
Attachment #8596146 - Flags: review?(dkuo)
(In reply to Delphine Lebédel [:delphine - use need info] from comment #2) > This is a regression: it got fixed a month back in Bug 1140154 > Triage: P2 -- nominating Bug 1140154 only fixed the part in the album detail view(sublist view), Music app didn't handle bdi parentheses before(it's reproducible on v2.1 already) so this is something we missed to implement.
Keywords: regression
(In reply to David Flanagan [:djf] from comment #5) > I'm disappointed that we didn't get these cases handled as part of bug > 1140154. I suspect the fix here is just to add <bdi> elements around the > song name in the gaia-header and in the overlay where the artist name is > displayed. > > I've asked Hub to take this one. Sorry about bug 1140154, I worked on it and my patch was mainly about adjusting styles of the list elements and sublist view in RTL, but didn't pay attention to the other views also needed to handle the parentheses in RTL. I guess there are some similar issues among the other gaia apps(Settings and Ringtones for example), and we should probably notify the owners that, parentheses should be handled by bdi elements.
Comment on attachment 8596146 [details] [gaia] hfiguiere:bug1156571-bidi-fixes > mozilla-b2g:master Hub, thanks for working on this, overall the changes looks good to me, note that because the html of player view is in both index.html and open.html, so be sure to make the same changes into open.html and keep the open activity working as usual before landing it.
Attachment #8596146 - Flags: review?(dkuo) → review+
Keywords: checkin-needed
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8596146 [details] [gaia] hfiguiere:bug1156571-bidi-fixes > mozilla-b2g:master [Approval Request Comment] [Bug caused by] (feature/regressing bug #): RTL blocker [User impact] if declined: RTL bugs [Testing completed]: manual testing [Risk to taking this patch] (and alternatives if risky): [String changes made]: none
Attachment #8596146 - Flags: approval-gaia-v2.2?(bbajaj)
Attachment #8596146 - Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
This issue is verified fixed on Flame 3.0 and 2.2. All the areas that were mentioned in bug write-up now display correctly formatted parentheses in Music app. Device: Flame 3.0 Master (full flashed 319MB KK) BuildID: 20150427010202 Gaia: b4c949cdc780893897c9b45c1adea46e2eb694ff Gecko: 37d60e3b8be6 Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b Version: 40.0a1 (3.0 Master) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0 Device: Flame 2.2 (full flashed 319MB KK) BuildID: 20150427002504 Gaia: 265ca0bc9408c21fc4b25a259fcee7fb642cd06b Gecko: 1908685d798d Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429 Version: 37.0 (2.2) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [rtl-impact][QAnalyst-Triage+] → [rtl-impact][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [rtl-impact][QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Test case has been added in moztrap: https://moztrap.mozilla.org/manage/case/16088/
Flags: in-moztrap+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: