1166203 - [RTL][Contacts]The "+" symbol at Import/Export Contacts views is displayed at wrong side.

Status: RESOLVED WORKSFORME

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

Description

[Sue]

Attachment: Export contacts.png

[1.Description]:
[RTL][Flame v2.2][Contacts]The "+" symbol at Import/Export Contacts views is displayed at right side of phone number.
See attachment:Import contacts.png & Export contacts.png

[2.Testing Steps]: 
Prerequisite: Insert a valid SIM card and Have some contacts in device.
1. Set system language as Arabic.
2. Launch Contacts.
3. Tap Settings icon.
4. Tap Import Contacts and observe the view.
5. Back to Settings view and tap Export Contacts.
6. Observe the view.

[3.Expected Result]: 
4&6. The "+" symbol should be displayed at left side of phone number.

[4.Actual Result]: 
4&6. The "+" symbol is displayed at right side of phone number.

[5.Reproduction build]: 
Device: Flame 2.2 (affected)
Build ID               20150518162501
Gaia Revision          5212c658a651e04d6d84dfc1bce06b499c0d0d96
Gaia Date              2015-05-18 12:10:52
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9e9e0d0f45f0
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150518.211109
Firmware Date          Mon May 18 21:11:21 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (unaffected)
Build ID               20150518010206
Gaia Revision          afea16de7a76c3b6d15c35fb4c37bac71c8ddc6a
Gaia Date              2015-05-17 03:33:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/35918b0441b4
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150518.042019
Firmware Date          Mon May 18 04:20:30 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (unaffected)
Build ID               20150518002503
Gaia Revision          f73891b8fcc5f34de81868640754f7cc331fa709
Gaia Date              2015-05-17 21:36:27
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/8785a53b8d6e
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150518.041439
Firmware Date          Mon May 18 04:15:05 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (unaffected)
Build ID               20150518010206
Gaia Revision          afea16de7a76c3b6d15c35fb4c37bac71c8ddc6a
Gaia Date              2015-05-17 03:33:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/35918b0441b4
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150518.043911
Firmware Date          Mon May 18 04:39:27 EDT 2015
Bootloader             HHZ12f

[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
Free Test

[8. Note]:
We use the same SIM card to testing, and the Nexus 5 won't show the phone number.

Comment 1

[Sue]

Attachment: Import contacts.png

Comment 2

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

There seems to have been a regression between the May 17th and May 18th builds. Asking for a regression window. 
Also, nominating

Comment 3

[Johan Lorenzo [:jlorenzo]]

Comms triage: UI issue breaking the first time experience for RTL readers.

Comment 4

[John Thomas [:Johnt]]

I was unable to get a "+" symbol to appear in the Import and Export Contacts menus. I tried this by using a foreign SIM as well.

Comment 5

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

Yes for the record same here...

Comment 6

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

We don't have the kind of SIM required to reproduce this issue. Adding keyword to exclude this in our queries.

Comment 7

[Norry.L.F(Leave from Mozilla)]

Attachment: VerifyRegressionwindow.png

Find the b2g-inbound Regression Window on v3.0

Last Broken Environmental Variables:
Build ID               20150512205042
Gaia Revision          013f90e9d6c202467d455578e84851563a8a5a1b
Gaia Date              2015-05-13 03:28:18
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/9946f17574f4
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150512.192015
Firmware Date          Tue May 12 19:20:26 EDT 2015
Bootloader             L1TC000118D0

First Working Environmental Variables:
Build ID               20150512212541
Gaia Revision          702e614de7941c49ead2594cbbeface93fc9155e
Gaia Date              2015-05-13 03:59:58
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/c92c2224cd6b
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150512.192015
Firmware Date          Tue May 12 19:20:26 EDT 2015
Bootloader             L1TC000118D0

Last Broken Gaia & First Working Gecko - issue DOES repro
Gaia Revision          013f90e9d6c202467d455578e84851563a8a5a1b
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/c92c2224cd6b

Last Broken Gecko & First Working Gaia - issue does NOT repro
Gaia Revision          702e614de7941c49ead2594cbbeface93fc9155e
Gecko Revision         https://hg.mozilla.org/integration/b2g-inbound/rev/9946f17574f4

Comment 8

[Norry.L.F(Leave from Mozilla)]

Hi Josh,

We have found the regression build with STR in comment 0, please check comment 7. 
Thanks.

Comment 9

[Norry.L.F(Leave from Mozilla)]

Also NI Delphine, and hope you can help with this bug.

Comment 10

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

Hi Norry - best I can do is ask someone from Comms team to look at this.
Julien: can you or someone from your team please take a look at this? thanks!

Comment 11

[Julien Wajsberg [:julienw]]

Francisco, looks like this is fixed in v3 but not in v2.2. I think this has been fixed by bug 1154438 in master, but that in v2.2 we'll need a manual dir=auto or something.

Comment 12

[Josh Cheng [:josh]]

Requesting Ted as bug 1154438 owner to raise 2.2 approval.

Comment 13

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

The problem that we have here, is that the composition happening in the l10n library is the following:

SIM {{number}}: {{msisdn}}

Unless we change the string: simNumberWithMSISDN, we cannot wrap the number by itself.

As far as I know we are late to  make changes in strings, ni :gandalf to see if he has a solution.

Comment 14

[Julien Wajsberg [:julienw]]

I think this is fixed by bug 1154438, but it's not in v2.2 either... :/

Comment 15

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

(In reply to Julien Wajsberg [:julienw] from comment #14)
> I think this is fixed by bug 1154438, but it's not in v2.2 either... :/

That will be ideal, just waiting for :gandalf to know if that patch is safe to upload to 2.2

Comment 16

[Zibi Braniecki [:zbraniecki][:gandalf]]

Unfortunately, backporting bug 1154438 is not an option because it depends on bug 1157726 which landed only on master. NI'ing Ted to confirm.

String changes are probably impossible, NI'ing Flod to confirm.

No other solution in sight :(

Comment 17

[Francesco Lodolo [:flod]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #16)
> String changes are probably impossible, NI'ing Flod to confirm.

Eventually the official answer needs to come from release-drivers (Josh Cheng for 2.2), but yes, we're way past string freeze, and the general idea is to avoid any string change.
https://groups.google.com/forum/#!topic/mozilla.dev.gaia/4VZ_3Ory-Uw

Comment 18

[Ted Clancy [:tedders1]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #16)
> Unfortunately, backporting bug 1154438 is not an option because it depends
> on bug 1157726 which landed only on master. NI'ing Ted to confirm.

That's true. But I'll see if I can get uplift approval for bug 1157726 (which also means getting uplift approval for bug 1151908, bug 1161932 and bug 1163583). And then I'll see if I can get uplift for bug 1154438.

I'll do that later today. I'll leave myself needinfo'd on this so I don't forget.

Comment 19

[Josh Cheng [:josh]]

(In reply to Ted Clancy [:tedders1] from comment #18)
> (In reply to Zibi Braniecki [:gandalf][:zibi] from comment #16)
> > Unfortunately, backporting bug 1154438 is not an option because it depends
> > on bug 1157726 which landed only on master. NI'ing Ted to confirm.
> 
> That's true. But I'll see if I can get uplift approval for bug 1157726
> (which also means getting uplift approval for bug 1151908, bug 1161932 and
> bug 1163583). And then I'll see if I can get uplift for bug 1154438.
> 
> I'll do that later today. I'll leave myself needinfo'd on this so I don't
> forget.

Hi Ted,
Could you identify any patch among bugs you mentioned here includes string change per comment 13 from Francesco?
If there must be string change to fix this bug. I would rather leave it as non 2.2 blocker since this is only observed when import/export contact.
Thanks.

Comment 20

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Josh Cheng [:josh] from comment #19)
> Hi Ted,
> Could you identify any patch among bugs you mentioned here includes string
> change per comment 13 from Francesco?
> If there must be string change to fix this bug. I would rather leave it as
> non 2.2 blocker since this is only observed when import/export contact.
> Thanks.

I can confirm on Ted's behalf. The patches make l10n.js handle bi-directional switches and will not require any string changes.

Comment 21

[Julien Wajsberg [:julienw]]

I'd add that this l10n change makes a lot of our workarounds in all apps to fix similar cases unnecessary. If we forgot such cases it would fix them too.

Comment 22

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

I guess that we are still on the process of uplifting dependencies. Will wait for Ted's input here, since going with the proper l10n solution sounds the right thing to do :)

Comment 23

[Ted Clancy [:tedders1]]

> I can confirm on Ted's behalf. The patches make l10n.js handle bi-directional switches and will not require any string changes.

That's correct!

> I'd add that this l10n change makes a lot of our workarounds in all apps to fix similar cases unnecessary. If we forgot such cases it would fix them too.

That is also correct!

> I guess that we are still on the process of uplifting dependencies.

And this is correct too!

Comment 24

[Josh Cheng [:josh]]

Thank you Zibi, Julien, Francisco and Ted!

Hi Ryan,
I am going to approve all dependencies.
According to Ted, they must landed with order of:
First, 1151908. Then 1157726. Then 1161932 and 1163583. And finally 1154438. Any other order will break the build and/or tests.

BTW. I cannot access bug 1163583 which not be able to approve it. Can anyone help?
Thanks!

Comment 25

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

Hi folks, any luck with the uplift process?

Comment 26

[Ted Clancy [:tedders1]]

I believe everything has been uplifted.

Comment 27

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

Great, thanks Ted!

Just tested and working fine!

Thanks everyone for helping here.