1153917 - Missing strings when importing contacts in the FTU

Status: RESOLVED WORKSFORME

(Firefox OS Graveyard :: Gaia::First Time Experience, defect, P2)

Description

[Oliver Nelson [:oliverthor]]

http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk.ui.adhoc/787/HTML_Report/

http://jenkins1.qa.scl3.mozilla.com/job/flame-kk-319.mozilla-central.nightly.ui.functional.non-smoke.2/155/HTML_Report/

PreReq:
* have a gmail account with contacts attached to it (the more, the easier to repro [bigger window for performing step]
Repro Steps:
1) Open Settings
2) Connect to a WiFi Network
3) Return to home
4) Open Contacts
5) Tap the 'gear' for Settings
6) Import Contacts -> Gmail
7) Sign into your gmail account w/ contacts
8) Proceed through UI, and select all contacts to import
9) Once confirming and import contacts, drag down notification tray and disconnect from WiFi connection
10) Observe UI

Actual:
UI displays placeholder text on error message when import from gmail has been interrupted

Expected: 
UI displays proper error message that network connection was lost while attempting to import contacts

Traceback (most recent call last):
Traceback (most recent call last):
  File "/var/jenkins/1/workspace/flame-kk.ui.adhoc/.env/local/lib/python2.7/site-packages/marionette_client-0.9-py2.7.egg/marionette/marionette_test.py", line 290, in run
    testMethod()
  File "/var/jenkins/1/workspace/flame-kk.ui.adhoc/tests/python/gaia-ui-tests/gaiatest/tests/functional/contacts/test_import_gmail_no_network.py", line 55, in test_import_gmail_no_network
    u'Friends\u2019 photos will not be added to your contacts. Continue anyway?')
AssertionError: u'Confirmation message' != u'Friends\u2019 photos will not be added to your contacts. Continue anyway?'
- Confirmation message
+ Friends\u2019 photos will not be added to your contacts. Continue anyway?


Environmental Variables:
Device firmware (base) 	L1TC100118D0
Device firmware (date) 	13 Apr 2015 08:35:55
Device firmware (incremental) 	eng.cltbld.20150413.043544
Device firmware (release) 	4.4.2
Device identifier 	flame
Device memory 	219772 kB
Device serial 	e47cd897
Device uptime 	0 days 0 hours 2 minutes 15 seconds
Gaia date 	10 Apr 2015 14:38:19
Gaia revision 	3c68964cb9fd
Gecko build 	20150413010203
Gecko revision 	0a46652bd992
Gecko version 	40.0a1

Reproducible manually: YES
** This was also reproduced with a hotmail account with identical results

Repro frequency: 2/2

Comment 1

[Martijn Wargers (dead)]

I can reproduce this too.
This is clearly a product bug.

We should find out what caused it and notify responsible developers.

Comment 2

[Martijn Wargers (dead)]

I can't reproduce in:
Build ID               20150410010202
Gaia Revision          e768af6558957ddb0f6a9ce579ea41c3e3d0b203
Gaia Date              2015-04-10 00:05:39
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/fec90cbfbaad
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150403.042432
Firmware Date          Fri Apr  3 04:24:41 EDT 2015
Bootloader             L1TC000118D0

I can reproduce this in:
Build ID               20150410160204
Gaia Revision          3c68964cb9fdba7cf0f6829b7f44562acaf1f1d7
Gaia Date              2015-04-10 14:38:19
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/eb3a1c0262e4
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150403.042432
Firmware Date          Fri Apr  3 04:24:41 EDT 2015
Bootloader             L1TC000118D0

Comment 3

[Martijn Wargers (dead)]

Could this somehow be a regression from bug 1066347?

Comment 4

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

I'm not sure. My initial guess is that it should not be a regression from that bug, but bug 1066347 might have expose this bug in more scenarios.

Basically, the problem is that apps/communications/contacts/index.html is opening shared/pages/import/import.html which uses a string from /shared/locales/facebook/facebook.properties but the file is not linked from contacts/index.html.

If my understanding is correct, since shared/pages/import/import.html is using facebook.properties, it should be linked in *all* places where import.html is used which means contacts and FTU according to my naive grep.

Drs, Sam, does it sound plausible? Do you intend to have facebook.properties resources available in your apps because you use import.html ?

Comment 5

[Johan Lorenzo [:jlorenzo]]

Confirmed. Bug 1066347 introduced this regression. I backed it out locally and got the error message back again.

[Blocking Requested - why for this release]: Regression which prevents a user from seeing an error message.

Comment 6

[Johan Lorenzo [:jlorenzo]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> I'm not sure. My initial guess is that it should not be a regression from
> that bug, but bug 1066347 might have expose this bug in more scenarios.

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #5)
> Confirmed. Bug 1066347 introduced this regression. I backed it out locally
> and got the error message back again.

Oops, sorry, mid-air collision between your comment and mine. It might be a regression from somewhere else.

Comment 7

[Doug Sherk (:drs) (inactive)]

Redirecting to Francisco, the Contacts owner.

Comment 8

[Sam Foster [:sfoster] (he/him)]

The dialog I get is the #confirmation-message. That's oddly named as it should be an error. But, whatever, it lives in the FTU app so its the FTU that should be loading that properties file and localizing the dialog. In the console I see 

    "L10nError: "connectionLost" not found in en-US in app://ftu.gaiamobile.org/index.html" "
    at reportMissingEntity (app://ftu.gaiamobile.org/shared/js/l10n.js:2103:9)"

The connectionLost string is only defined in facebook.properties, which is also odd as I was importing from gmail. IIRC the gmail/facebook import options can be toggled on/off so we're asking for trouble locating these strings in one file but relying on them being there for both. We should move or duplicate these to the import_contacts properties file?

Comment 9

[Sam Foster [:sfoster] (he/him)]

Its great that the test failure caught this, but as we just had a dupe filed for the same issue I'm making the bug title more general. Not sure if this belongs in FTE component, I'll leave it in l10n for now as its shared strings and files last touched by the l10n team.

Comment 11

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

Is it the same bug as bug 1153917?

Comment 12

[Martijn Wargers (dead)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #11)
> Is it the same bug as bug 1153917?

This is bug 1153917, so I guess you meant a different bug here.

Comment 13

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

Oh yeah, I meant bug 1154415.

Comment 14

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

(In reply to Sam Foster [:sfoster] from comment #9)
> Its great that the test failure caught this, but as we just had a dupe filed
> for the same issue I'm making the bug title more general. Not sure if this
> belongs in FTE component, I'll leave it in l10n for now as its shared
> strings and files last touched by the l10n team.

Sam, so, I feel like while l10n.js change exposed the bug (the bug existed before, it was just not visible when we optimized l10n resources), the cause is that FTU app does not load necessary resources.

Would it make sense to reassign the bug to FTU or would you expect my team to fix it?

Comment 15

[Sam Foster [:sfoster] (he/him)]

> Sam, so, I feel like while l10n.js change exposed the bug (the bug existed
> before, it was just not visible when we optimized l10n resources), the cause
> is that FTU app does not load necessary resources.
> 
> Would it make sense to reassign the bug to FTU or would you expect my team
> to fix it?

I'm happy to fix it, but as we're talking late-l10n string changes for v2.2, I just wanted your input on what fix to implement. ISTM we need to add the missing strings to https://github.com/mozilla-b2g/gaia/blob/master/shared/locales/import_contacts/import_contacts.en-US.properties, and ensure FTU loads that.

Comment 16

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

I think it's 3.0 only. In 2.2 we're covered in optimized case because ab-CD.json collects all entries from all files in the app, so I'd rather have this properly fixed on master by selecting:

 - files we need in dialer
 - files we need in FTU
 - files we need in contacts
 - files we share between them.

Comment 17

[Sam Foster [:sfoster] (he/him)]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #16)
> I think it's 3.0 only. In 2.2 we're covered in optimized case because
> ab-CD.json collects all entries from all files in the app

Ok great. I'm moving this to FTE and will take it from here. 
Francisco, I'll flag you on the patch as I think contacts and FTU are equally affected here if I'm updating that shared file.

Comment 18

[Johan Lorenzo [:jlorenzo]]

[Blocking Requested - why for this release]: Like the other fallouts of bug 1066347, we should prevent this kind of regressions in 2.5

Comment 19

[Gregor Wagner [:gwagner]]

Is this still a valid bug with Facebook contacts removed?

Comment 20

[Martijn Wargers (dead)]

test_import_gmail_no_network.py is now passing again, see bug 1203734.
I guess that means this bug can be closed.

Comment 21

[Martijn Wargers (dead)]

I'm marking this worksforme, please reopen if you can still this.