[RTL][Notifications] "+" sign is located incorrectly with a phone number on notifications

VERIFIED FIXED in Firefox OS v2.2

Status

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

People

(Reporter: YeojinC, Assigned: mikehenrty)

Tracking

unspecified
2.2 S10 (17apr)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: [3.0-Daily-Testing], systemsfe)

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Created attachment 8589373 [details]
VoiceMailLockscreen.png

Description:
When a phone number has the + sign and a country code, the + is located at the right side of the phone number in RTL in notifications, such as voicemail notifications or missed call from an unsaved foreign number. 

Pre-Requisite: Have a SIM with unchecked voicemails in the device

Repro Steps:
1) Update a Flame to 20150407010204.
2) Set the device language in Arabic during FTU.
3) When FTU is finished, pull down the notification tray.
4) Observe the phone number of the voicemail.

Actual:
+ is located on the right side of the phone number.

Expected:
+ is located on the left side of the phone number.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150407010204
Gaia: c710bac533b76635161315bf907d004e000549cb
Gecko: ab0490972e1e
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

Repro frequency: 5/5
See attached: screenshot
(Reporter)

Comment 1

3 years ago
This issue also reproduces on Flame 2.2.

Result: + is located on the right side of the phone number.

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
BuildID: 20150407002501
Gaia: 5e09637414269728f6f1bc0152d0160f3b6b380e
Gecko: 245f37f44017
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
QA Whiteboard: [QAnalyst-Triage?][rtl-impact]
(Reporter)

Updated

3 years ago
Flags: needinfo?(ktucker)
Blocks: 1064539
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Component: Gaia::Dialer → Gaia::System
Flags: needinfo?(ktucker)
Whiteboard: [3.0-Daily-Testing] → [3.0-Daily-Testing], systemsfe
Triage P1 -- nominating as this conveys the wrong information on a main screen
blocking-b2g: --- → 2.2?
Priority: -- → P1
I think this should be under notifications-rtl. Moving
Blocks: 1072027
No longer blocks: 1064539
I'm guessing this has to do with the issue in Bug 883884
Depends on: 883884

Updated

3 years ago
blocking-b2g: 2.2? → 2.2+

Comment 5

3 years ago
Hi Gregor,
Can you help to find someone who can fix this? Thanks!
Flags: needinfo?(anygregor)
Flags: needinfo?(anygregor)
Assignee: nobody → mhenretty
Yeojin: I believe this is a regression, but can not identify from when by doing some initial testing. Can you please investigate this further and possibly identify a regression window please? If this is in fact a regression please add the needed keyword.
Notifications have been covered and tested by Marigold for a moment now, so I'd be surprised if this isn't a regression that this wasn't caught earlier. Can you please confirm that you've already tested this? thanks
Flags: needinfo?(ychung)
(Reporter)

Comment 7

3 years ago
This issue was found while verifying bug 1150953. I assume that no one has filed it before since they thought it was the same issue as bug 1150953. I'll add qawanted for further investigation to see if this issue is a regression.
Flags: needinfo?(ychung)
Keywords: qawanted
FWIW, this attachment from bug 1147011 shows the issue happening at least on March 24th 2015:

https://bug1147011.bugzilla.mozilla.org/attachment.cgi?id=8582523
The following is the test results from nightly production builds on a 2 weeks interval which covered a little over 2 months in total:

20150202010229 - repro
20150215010209 - repro
20150226010233 - repro
20150313010238 - repro
20150326010205 - repro
20150409010203 - repro

repro = plus sign is shown to the right of phone number in Arabic. Test shows this is not a regression.
QA Whiteboard: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage?][rtl-impact]
Flags: needinfo?(ktucker)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(ktucker)
Target Milestone: --- → 2.2 S10 (17apr)
Created attachment 8592445 [details] [review]
[gaia] mikehenrty:bug-1152127-LRM-control > mozilla-b2g:master
Speaking with Ted and Simon, this is probably not a bug in the platform, but more like a feature request for FXOS. This will not be fixed with bug 883884.
No longer depends on: 883884
Blocks: 1154450
Comment on attachment 8592445 [details] [review]
[gaia] mikehenrty:bug-1152127-LRM-control > mozilla-b2g:master

Since this is a feature request and not a bidi algorithm bug, our approach here is to prepend the LRM character to the phone number in the voicemail notification. In the future, l10n will automatically detect phone number substitutions and insert the appropriate control characters for us (bug 1154438). Until then, each app that puts phone numbers in notifications will have to handle this themselves.

Kevin, wanna have a look?
Attachment #8592445 - Flags: review?(kgrandon)
Comment on attachment 8592445 [details] [review]
[gaia] mikehenrty:bug-1152127-LRM-control > mozilla-b2g:master

I am not an RTL expert or anything, but this code looks good to me. It also looks like you have a failing unit test, so please be sure to address that before landing. Thanks!
Attachment #8592445 - Flags: review?(kgrandon) → review+
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Comment on attachment 8592445 [details] [review]
[gaia] mikehenrty:bug-1152127-LRM-control > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
Not a regression, but a P1 for RTL.

[User impact] if declined:
Wrong notification display for international phone numbers in RTL.

[Testing completed]:
Manually tested, fixed unit tests.

[Risk to taking this patch] (and alternatives if risky):
Only affects the display of the phone numbers in voicemail notifications. Small, and low risk.

[String changes made]: none.
Attachment #8592445 - Flags: approval-gaia-v2.2?

Updated

3 years ago
Attachment #8592445 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/89a94a8b9c9087da916751697a61ba15bc0688c3
status-b2g-v2.2: affected → fixed
status-b2g-master: affected → fixed
(Reporter)

Comment 17

3 years ago
This issue is verified fixed on Flame Master and 2.2.

Result: + is located on the left side of the phone number.

Environmental Variables:
Device: Flame 3.0 (KK, 319mb, full flash)
Build ID: 20150416010206
Gaia: 629097847567e51095a454e7e63186a6e2ac0307
Gecko: a35163f83d22
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Environmental Variables:
Device: Flame 2.2 (KK, 319mb, full flash)
Build ID: 20150416002504
Gaia: 8e24d8b7f5e7c74c3004b22710dda0dac3e04ead
Gecko: 41388836b5c6
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: [QAnalyst-Triage+][rtl-impact] → [QAnalyst-Triage?][rtl-impact]
status-b2g-v2.2: fixed → verified
status-b2g-master: fixed → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.