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)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: pcheng, Assigned: hub)
References
Details
(Whiteboard: [3.0-Daily-Testing])
Attachments
(3 files)
(deleted),
audio/mpeg
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
text/x-github-pull-request
|
dkuo
:
review+
bajaj
:
approval-gaia-v2.2+
|
Details |
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
Reporter | ||
Comment 1•10 years ago
|
||
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.
Reporter | ||
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.2:
--- → affected
status-b2g-master:
--- → affected
Flags: needinfo?(ktucker)
Whiteboard: [3.0-Daily-Testing]
Reporter | ||
Updated•10 years ago
|
OS: Linux → Gonk (Firefox OS)
Hardware: x86 → ARM
Reporter | ||
Updated•10 years ago
|
Blocks: music-rtl
QA Whiteboard: [QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage?]
Comment 2•10 years ago
|
||
This is a regression: it got fixed a month back in Bug 1140154
Triage: P2 -- nominating
Reporter | ||
Comment 3•10 years ago
|
||
Note: only the behavior observed at step 3 of the STR is a regression, the rest is not regression.
Comment 4•10 years ago
|
||
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
Updated•10 years ago
|
QA Whiteboard: [rtl-impact][QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Comment 5•10 years ago
|
||
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
Comment 6•10 years ago
|
||
Blocking Reason: poor ux and a missed test case from previous attempt to fix.
blocking-b2g: 2.2? → 2.2+
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8596146 [details]
[gaia] hfiguiere:bug1156571-bidi-fixes > mozilla-b2g:master
This should fix the issues.
Attachment #8596146 -
Flags: review?(dkuo)
Comment 9•10 years ago
|
||
(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
Comment 10•10 years ago
|
||
(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 11•10 years ago
|
||
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+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Updated•10 years ago
|
Keywords: checkin-needed
Comment 12•10 years ago
|
||
Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/42fd6933c82796870eaaacbed667f86217243d75
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8596146 -
Flags: approval-gaia-v2.2?(bbajaj) → approval-gaia-v2.2+
Updated•10 years ago
|
Comment 14•10 years ago
|
||
Target Milestone: --- → 2.2 S11 (1may)
Reporter | ||
Comment 15•10 years ago
|
||
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)
Updated•10 years ago
|
QA Whiteboard: [rtl-impact][QAnalyst-Triage?] → [rtl-impact][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Comment 16•10 years ago
|
||
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.
Description
•