1058790 - [RTL] International numbers are not correctly handled in Contacts

Status: VERIFIED FIXED

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

Description

[Johan Lorenzo [:jlorenzo]]

Build Information
Gaia      4d1d0ea5a82cddeeab497774cfa1703639e3c7d9
Gecko     https://hg.mozilla.org/mozilla-central/rev/dc352a7bf234
BuildID   20140826040204
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

Prerequisites
Have a contact with an international indicator (like [phone])

Steps to Reproduce:
1. Go to contacts
2. Go to the contacts details

Expected Results:
The displayed phone number should be [phone]

Actual Results:
You're able to see: 16501234567+

Reproduction Frequency: 100%

Comment 1

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

This also happens when editing the contact's number, in the input field. And probably that's the cause.

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

A quick review of the places where we need to change this:

- Contact Detail (this is shared with dialer)
- Contact Form
- List (if we have just a phone as contact information will appear)
- ICE Contacts (settings and list) if we setup a ICE contacts that just have a phone number

Comment 3

[Francisco Jordano [:arcturus] [:francisco]]

Hi Stephany,

I though that RTL bugs are not blockers, any reason for this one?

Comment 4

[Stephany Wilkes]

That's not correct. Sorry for the confusion around the way this is being handled.

Phoxygen is doing the engineering work for RTL, but RTL is still landing in 2.2 and will be shown as part of 2.2 at MWC. We (Mozilla QA, release, UX, L10N) are supporting Phoxygen on the landing and review, etc. There are 2.2 bugs that we've agreed, with Phoxygen, block 2.2. If a bug had 2.2+ it is because we agreed it should with Phoxygen last week. Please speak with Bhavana or myself before moving any RTL 2.2+ or ? to -. Thanks!

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Thanks for the clarification Stephany, will take a look to this.

Comment 6

[Stephany Wilkes]

Appreciate it! We want to ensure everyone can complete their calls to everywhere. :)

Comment 7

[Dylan Oliver [:doliver]]

RTL update: marking required bugs as feature-b2g:2.2+ (and removing blocking flags)

Comment 8

[Johan Lorenzo [:jlorenzo]]

For better visibility, it's a usually a good practice to paste a (some) moztrap link(s) in the comment when we flip an in-moztrap flag.

Do you agree guys?

Comment 9

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

Yes Johan, that's what has been communicated to Marigold team when they started doing this.
Please follow this in future, thanks!

Comment 10

[Hubert Lu[:hlu] <[e-mail]>]

Hi Johan and Delphine, 
    Yes. Moztrap link should be in the comment while commenter change flag to be "in-moztrap+". We have been communicated this rule to Marigold already.

Comment 11

[Lancy(Leave from Mozilla)]

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

Comment 12

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Pointer to PR 27821

In this PR I've tried to address the numbers where we display them:

- Contact list (if we just have the number)
- Detail view
- Contacts form
- ICE settings

Comment 13

[Jose Manuel Cantera]

Comment on attachment 8557109 [details] [review]
Pointer to PR 27821

LGTM

thanks

Comment 14

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8557109 [details] [review]
Pointer to PR 27821

Reasking for r? since I needed to change unit tests and ui tests to accommodate the new markup change.

Comment 15

[Jose Manuel Cantera]

Comment on attachment 8557109 [details] [review]
Pointer to PR 27821

oh, didn't check the Treeherder when I reviewed the patch. 

The follow-up changes seem to be trivial so r=me

thanks, Francisco and good work

Comment 16

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8557109 [details] [review]
Pointer to PR 27821

Looks good, thanks!

Comment 17

[Francisco Jordano [:arcturus] [:francisco]]

Landed:

https://github.com/mozilla-b2g/gaia/commit/ed2b99f7ba4a4517d0aa7d08b9f3d783181acd9a

Comment 18

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8557109 [details] [review]
Pointer to PR 27821

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
RTL
[User impact] if declined:
Bad user experience
[Testing completed]:
Unit test and integration tests passing
[Risk to taking this patch] (and alternatives if risky):
Low
[String changes made]: None

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Coler]

This issue has been verified successfully on Flame 2.2/3.0.
Reproduce rate:0/5
Attachment:Verify_RTL.png

Flame 2.2 build:

Build ID               20150204002509
Gaia Revision          a4c4cc86303a554facb8f45b7e764e5c4473c3de
Gaia Date              2015-02-04 00:41:59
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8669c26fd4a5
Gecko Version          37.0a2
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150204.035932
Firmware Date          Wed Feb  4 03:59:42 EST 2015
Bootloader             L1TC000118D0

Flame 3.0 build:

Build ID               20150204010225
Gaia Revision          dfebaaa8aab43470f482d09d71137bab840c3ae9
Gaia Date              2015-02-03 18:49:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/0c2f7434c325
Gecko Version          38.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150204.043856
Firmware Date          Wed Feb  4 04:39:07 EST 2015
Bootloader             L1TC000118D0

Comment 21

[Coler]

Attachment: Verify_RTL.png

Comment 22

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Since this bug has been verified in comment 20 and after 15 of my own attempts on each branch I was unable to repro. Marking this bug as verified fixed.