[RTL] International numbers are not correctly handled in Contacts

VERIFIED FIXED in Firefox OS v2.2

Status

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

People

(Reporter: jlorenzo, Assigned: arcturus)

Tracking

(Blocks: 1 bug)

unspecified
2.2 S5 (6feb)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(feature-b2g:2.2+, ux-b2g:2.2, b2g-v2.0 affected, b2g-v2.1 affected, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [2.1-Arabic-RTL-bug-bash][p=2])

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
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 +1 650 123 4567)

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

Expected Results:
The displayed phone number should be +16501234567

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

Reproduction Frequency: 100%

Updated

3 years ago
Blocks: 1064489

Updated

3 years ago
No longer blocks: 906270
This also happens when editing the contact's number, in the input field. And probably that's the cause.
(Reporter)

Updated

3 years ago
See Also: → bug 1102340
(Reporter)

Updated

3 years ago
QA Contact: jlorenzo
Target Milestone: --- → 2.2 S1 (5dec)
Assignee: nobody → francisco
Whiteboard: [2.1-Arabic-RTL-bug-bash] → [2.1-Arabic-RTL-bug-bash][p=2]
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
Status: NEW → ASSIGNED

Updated

3 years ago
blocking-b2g: --- → 2.2+
ux-b2g: --- → 2.2
Priority: -- → P1
Hi Stephany,

I though that RTL bugs are not blockers, any reason for this one?
Flags: needinfo?(swilkes)

Comment 4

3 years ago
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!
Flags: needinfo?(swilkes)
Thanks for the clarification Stephany, will take a look to this.

Comment 6

3 years ago
Appreciate it! We want to ensure everyone can complete their calls to everywhere. :)
Depends on: 1116803
RTL update: marking required bugs as feature-b2g:2.2+ (and removing blocking flags)
blocking-b2g: 2.2+ → ---
feature-b2g: --- → 2.2+
Flags: in-moztrap-
Flags: in-moztrap- → in-moztrap+
(Reporter)

Comment 8

3 years ago
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?
Flags: needinfo?(lebedel.delphine)
Flags: needinfo?(hlu)
Flags: needinfo?(echang)
Yes Johan, that's what has been communicated to Marigold team when they started doing this.
Please follow this in future, thanks!
Flags: needinfo?(lebedel.delphine)
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.
Flags: needinfo?(hlu)
Test case has been added in moztrap:
https://moztrap.mozilla.org/manage/case/15396/
Flags: needinfo?(echang)
Created attachment 8557109 [details] [review]
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
Attachment #8557109 - Flags: review?(jmcf)

Comment 13

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

LGTM

thanks
Attachment #8557109 - Flags: review?(jmcf) → review+
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.
Attachment #8557109 - Flags: review?(jmcf)
Attachment #8557109 - Flags: review?(dave.hunt)
Attachment #8557109 - Flags: review+

Comment 15

3 years ago
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
Attachment #8557109 - Flags: review?(jmcf) → review+
Comment on attachment 8557109 [details] [review]
Pointer to PR 27821

Looks good, thanks!
Attachment #8557109 - Flags: review?(dave.hunt) → review+
Landed:

https://github.com/mozilla-b2g/gaia/commit/ed2b99f7ba4a4517d0aa7d08b9f3d783181acd9a
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
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
Attachment #8557109 - Flags: approval-gaia-v2.2?

Updated

3 years ago
Attachment #8557109 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/2af7b3230449d872e278b39f554da457d16b7036
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
Target Milestone: 2.2 S1 (5dec) → 2.2 S5 (6feb)

Comment 20

3 years ago
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
Status: RESOLVED → VERIFIED
status-b2g-v2.2: fixed → verified
status-b2g-master: fixed → verified

Comment 21

3 years ago
Created attachment 8559552 [details]
Verify_RTL.png

Updated

3 years ago
Status: VERIFIED → RESOLVED
Last Resolved: 3 years ago3 years ago
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.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?][rtl-impact]
Flags: needinfo?(pbylenga)
QA Whiteboard: [QAnalyst-Triage?][rtl-impact] → [QAnalyst-Triage+][rtl-impact]
Flags: needinfo?(pbylenga)
You need to log in before you can comment on or make changes to this bug.