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

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P1)

Description

[Yeojin Chung [:YeojinC]]

Attachment: 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

Comment 1

[Yeojin Chung [:YeojinC]]

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

Comment 2

[Delphine Lebédel [:delphine - Use needinfo]]

Triage P1 -- nominating as this conveys the wrong information on a main screen

Comment 3

[Delphine Lebédel [:delphine - Use needinfo]]

I think this should be under notifications-rtl. Moving

Comment 4

[Delphine Lebédel [:delphine - Use needinfo]]

I'm guessing this has to do with the issue in Bug 883884

Comment 5

[Josh Cheng [:josh]]

Hi Gregor,
Can you help to find someone who can fix this? Thanks!

Comment 6

[Delphine Lebédel [:delphine - Use needinfo]]

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

Comment 7

[Yeojin Chung [:YeojinC]]

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.

Comment 8

[Michael Henretty [:mikehenrty][:mhenretty]]

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

Comment 9

[Pi Wei Cheng [:piwei] (inactive)]

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.

Comment 10

[GitHub Autolander Bot]

Attachment: [gaia] mikehenrty:bug-1152127-LRM-control > mozilla-b2g:master

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

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.

Comment 12

[Michael Henretty [:mikehenrty][:mhenretty]]

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?

Comment 13

[Kevin Grandon :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!

Comment 14

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/45caed646ba16994c005cac700213fce107be8ec

Comment 15

[Michael Henretty [:mikehenrty][:mhenretty]]

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.

Comment 16

[Michael Henretty [:mikehenrty][:mhenretty]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/89a94a8b9c9087da916751697a61ba15bc0688c3

Comment 17

[Yeojin Chung [:YeojinC]]

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