Closed Bug 1082444 Opened 7 years ago Closed 7 years ago

[Settings] settings app is not working correctly after open configure activity from Firefox Account check settings.

Categories

(Firefox OS Graveyard :: FxA, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

RESOLVED FIXED
2.2 S1 (5dec)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: dongkyu31, Assigned: jhirsch)

References

()

Details

(Whiteboard: [LibGLA,TD123196,QE3, B][p=1])

Attachments

(13 files, 1 obsolete file)

1. Title : [Settings] settings app is not working correctly after open configure activity from Firefox Account check settings.

2. Precondition :
 - No SIM inserted

3. Tester's Action : 
  a. Tap [Settings] icon on Home screen.
  b. Tap [Firefox Accounts] menu in settings app
  c. Tap [Create account or sign in] button
  d. Tap [Privacy Notice] link
  e. Tap [Check settings] button 
  f. Tap [X] icon at upper left on Dialog.
  g. Tap [X] icon at upper left on the [Firefox Accounts] screen.
  h. Settings app filtered By connectivity screen is shown.
  i. Tap [Done] button at upper right on the settings activity.
 
4. Detailed Symptom :  [Done] button is not working and [Settings] app is not working correctly. 

5. Expected :  [Done] button is working and Settings app is working correctly.
6. Reproducibility: Y
   1) Frequency Rate : 100%

7. Platform version : 32.0
8. Git commit info : 2014-10-01 23:55:51, 9725d188
9. OS version : v2.0
Hi Dongkyu, 

I can't reproduce the bug on the latest build on flame 2.0.
After step g, I can't see the step h as you described.
What I saw is the page of create account or sign in of "Firefox Accounts" again which is the same as the screen as step c.
Not sure which steps make this different outcome. 
Please provide the video of STR, and also the device information including build ID and Git commit info.
Thanks!
Flags: needinfo?(dongkyu31)
Attached video CAM00249.mp4
Please check my video clip.
STR is little different but we just need to access "online content" without "NW".

---------------------------------------------
Build-ID        20140904160718
Version         32.0
FW-Release      4.4.2
FW-Incremental  27
FW-Date         Thu Sep  4 14:59:02 CST 2014
---------------------------------------------
Flags: needinfo?(dongkyu31)
Hi Arthur,Could you please kindly help look into the issue ?

Please refer to the video, after 0:11 when the the page shows no internet connection, 
"check settings " and the " X" on the upper left corner are both unresponsive

Thank you very much!
Flags: needinfo?(arthur.chen)
Is there any limitations on circular activities? It seems after performing the STR the settings app was not brought to the foreground.
Flags: needinfo?(arthur.chen) → needinfo?(alive)
(In reply to Arthur Chen [:arthurcc] from comment #4)
> Is there any limitations on circular activities? It seems after performing
> the STR the settings app was not brought to the foreground.

The flow seems to me is:
Settings App -> FxAccount System Dialog -> EntrySheet(by FxAccount module) -> MozActivity to Settings

So the problem is the entry sheet as well as the FxAccount SystemDialog does not close.

Hi Jerad do you want to fix this issue? We just need to make FxAccount listen to "activityrequesting" event to close itself I think.
Flags: needinfo?(alive) → needinfo?(6a68)
Component: Gaia::Settings → Gaia::System::Window Mgmt
Hi Alive,

> Hi Jerad do you want to fix this issue? We just need to make FxAccount
> listen to "activityrequesting" event to close itself I think.

Sure! I'm happy to take this one.
Assignee: nobody → 6a68
Flags: needinfo?(6a68)
We're not currently using an activity to load the privacy/ToS pages in the EntrySheet, so there doesn't seem to be a clean way to detect when the user clicks a link inside those pages. I could set a click handler on the EntrySheet itself, but that seems unnecessarily complex.

The best solution is probably to simply close the FxA dialog when the privacy/ToS page is loaded. This will make it one click longer to get back to the signup flow, but keeps the code simple.

Refiling under the FxA component.
Status: UNCONFIRMED → NEW
Component: Gaia::System::Window Mgmt → FxA
Ever confirmed: true
Hi Jared, could you please kindly update the status of the bug per comment7 ? 

Thank you very much!!
Flags: needinfo?(6a68)
Hi Rachelle - Sorry, I don't understand what status you'd like me to update?
Flags: needinfo?(6a68) → needinfo?(ryang)
Hi Jared, I am just thinking if any patch is available based on your idea in comment7 .

Thank you very much !
Flags: needinfo?(ryang) → needinfo?(6a68)
^_^ working on it now
Flags: needinfo?(6a68)
Hi Jared, thank you very much ! Any update or patch you could kindly share ? 

Thanks !
Flags: needinfo?(6a68)
Sorry for the delay, working on higher priority bugs right now, will upload a patch when it's ready
Flags: needinfo?(6a68)
Whiteboard: [LibGLA,TD123196,QE3, B][p=1]
Hi Jared, thanks for your efforts.
Since this is partner Launch blocker, please kindly share your progress.
Thank you for your understanding.
Flags: needinfo?(6a68)
[Blocking Requested - why for this release]:
partner Launch blocker
blocking-b2g: --- → 2.0?
Hi Rachelle,

Ouch! I'm sorry, I didn't know this was a partner launch blocker! (None of the blocker flags were set)

Unfortunately, I'll be on PTO Wednesday through Monday.

I'll get the patch submitted tonight and find someone to take over any additional needed changes while I'm away.

Sorry again about the miscommunication...I thought you were just very curious about this particular bug ;-)

Cheers,

Jared
Flags: needinfo?(6a68)
I have a work-in-progress branch that spenrose will take over tomorrow.

The code's at https://github.com/6a68/gaia/tree/bug-1082444-wip.

Sam will update the bug when it's ready for review; I expect ferjm will review it, he's most familiar with that part of the system app.
Hi Jared, thanks for your work.

not only I am curious....., but also this bug is partner confirmed blocker. 

Could you please kindly ni Sam here for tracking. Thank you very much
blocking-b2g: 2.0? → 2.0+
Assignee: 6a68 → spenrose
Attached file Link to Gaia PR (obsolete) —
Hey Fernando! Can you take a look at this 2.0 blocker? Thanks!
Attachment #8517759 - Flags: review?(ferjmoreno)
Comment on attachment 8517759 [details] [review]
Link to Gaia PR

Thank you Sam! The code looks good to me. I left a couple of minor comments in the PR. What I am missing is the tests for this piece of code. I cannot find previously existing ones. Would it be possible to add some tests, please? Thanks!
Attachment #8517759 - Flags: review?(ferjmoreno)
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #20)
> Comment on attachment 8517759 [details] [review]
> Link to Gaia PR
> 
> Thank you Sam! The code looks good to me. I left a couple of minor comments
> in the PR. What I am missing is the tests for this piece of code. I cannot
> find previously existing ones. Would it be possible to add some tests,
> please? Thanks!

Thanks Fernando! I have cleaned up the PR, though I may at the same time have destroyed it by including a merge from origin/master -- I can't tell. There are tests at apps/system/test/unit/fxa_test/screens/fxam_enter_email_test.js, but I wasn't able to add to them today. I will see what I can do tomorrow.
Sorry for the lack of movement here. I'll get the patch resubmitted as soon as possible.
Duplicate of this bug: 1101231
this is being worked on today
I have this working (branch updated), need to add tests per ferjm's comment 20. Should have this out for review tomorrow. Apologies again for the delays.
Assignee: spenrose → 6a68
Attached file Updated PR
Hi Fernando,

Would you mind taking a look at this 2.0 blocker patch? It's quite small and straightforward.

With my earlier patch, I wound up changing one thing after another, getting sucked into refactoring the FxA code...I'm going to leave all of that for another bug that doesn't need to land on the stabilized 2.0 branch.

This patch *just* addresses the original bug description by closing the EntrySheet on home, holdhome, and activityrequesting events. The only activity we need to worry about is if the user is offline, opens the EntrySheet, then is shown the net-error page with the "click here to enable wifi in settings" button.
  I've also added a quick onLine check, so that we don't waste time opening the EntrySheet if the user isn't online; instead, we just show the fxa offline error message.

I've added tests and they are passing for me locally. I have to run, but hopefully Gaia-try will be green.

I will be offline Thur-Fri for Thanksgiving, but I'll respond to any feedback while traveling on Monday.

Thanks!

Jared
Attachment #8517759 - Attachment is obsolete: true
Attachment #8529433 - Flags: review?(ferjmoreno)
Comment on attachment 8529433 [details] [review]
Updated PR

Thanks Jared, LGTM! Thank you for the tests as well.
I hope you had a nice Thanksgiving break :)
Attachment #8529433 - Flags: review?(ferjmoreno) → review+
Awesome, thanks Fernando!

Master: https://github.com/mozilla-b2g/gaia/commit/a1197de31876561129fcac0c658111322e040166
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Please request Gaia v2.0 and v2.1 approval on this when you get a chance.
Flags: needinfo?(6a68)
Target Milestone: --- → 2.2 S1 (5dec)
Comment on attachment 8529433 [details] [review]
Updated PR

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):

bug 1082444

[User impact] if declined:

The bug is caused by the privacy/terms page failing to display when the network is unavailable. The user is prompted to open the Settings app, but clicking the button doesn't actually take the user to that app.

Additionally, this bug is a 2.0 partner blocker.

[Testing completed]:

I haven't tested on 2.0, but the patch is fine on master. Not sure what the partner 2.0 timeline is, or how far back they branched.

[Risk to taking this patch] (and alternatives if risky):

Low risk: changes affect only the popup window opened by the FxA dialog. The patch is small, and its complexity is low.

[String changes made]: none
Flags: needinfo?(6a68)
Attachment #8529433 - Flags: approval-gaia-v2.1?(bbajaj)
Attachment #8529433 - Flags: approval-gaia-v2.0?(bbajaj)
Keywords: verifyme
Comment on attachment 8529433 [details] [review]
Updated PR

Brian, can you please make sure QA verifies this on 2.0/2.1 post landing.
Attachment #8529433 - Flags: approval-gaia-v2.1?(bbajaj)
Attachment #8529433 - Flags: approval-gaia-v2.1+
Attachment #8529433 - Flags: approval-gaia-v2.0?(bbajaj)
Attachment #8529433 - Flags: approval-gaia-v2.0+
Flags: needinfo?(brhuang)
Failed verification on Flame 2.2

"Network error" message appears and the "Check Settings" button is inactive

See YouTube video http://youtu.be/40igoqG_-nY

Flame 2.2

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141209040203
Gaia: 9e0b96c7b61c7ff943876ca93e2596d972437b80
Gecko: acf5660d2048
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage?][failed-verification]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][failed-verification] → [QAnalyst-Triage+][failed-verification]
Flags: needinfo?(ktucker)
Hi Norry, 

Please have someone to check this. Thx.

Brian
Flags: needinfo?(brhuang) → needinfo?(fan.luo)
Attached video Verify2.0&2.1.MP4
This issue can be reproduced on Flame 2.0 and Flame 2.1

Attched: Verify2.0&2.1.MP4, logcat_v2.0_2219.txt, logcat_v2.1_2251.txt
Found time:v2.0(22:19),v2.1(22:51)
Rate: 100%

Flame 2.0 versions:
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2d0860bd0225
Build-ID        20141209160202
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141209.192800
FW-Date         Tue Dec  9 19:28:11 EST 2014
Bootloader      L1TC00011880

Flame 2.1 versions:
Gaia-Rev        c226db212db4d824c09617cd6dc407b2d4258d9b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/cf8bebfa4703
Build-ID        20141209170126
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141209.212104
FW-Date         Tue Dec  9 21:21:15 EST 2014
Bootloader      L1TC00011880
Flags: needinfo?(fan.luo)
Attached video Flame 2.2.MP4
This issue can't be reproduced on Flame 2.2. 

When tap "Prrivacy Notice", it will display "Unable to connect. Please connect to a network from Settings.", and tap "OK" button, it will back to Firefox accounts page. This behavior is the same as what comment 26 said:
"I've also added a quick onLine check, so that we don't waste time opening the EntrySheet if the user isn't online; instead, we just show the fxa offline error message."
Please see verify video

Flame 2.2 versions:
Gaia-Rev        4cdeee67b449db90aae9384337311547c280093c
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/d7c76fe69e9a
Build-ID        20141209160204
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141209.192623
FW-Date         Tue Dec  9 19:26:39 EST 2014
Bootloader      L1TC00011880
(In reply to sarsenyev from comment #32)
> Failed verification on Flame 2.2
> 
> "Network error" message appears and the "Check Settings" button is inactive
> 
> See YouTube video http://youtu.be/40igoqG_-nY

The video can't be played on my side, so I am not sure what's the behavior in comment 32.

Dear Hubert, 
Per talked, please recheck and confirm it on your side. Thank you very much.
Flags: needinfo?(hlu)
(In reply to sarsenyev from comment #32)
> Failed verification on Flame 2.2
> 
> "Network error" message appears and the "Check Settings" button is inactive
> 
> See YouTube video http://youtu.be/40igoqG_-nY
> 
> Flame 2.2
> 
> Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
> BuildID: 20141209040203
> Gaia: 9e0b96c7b61c7ff943876ca93e2596d972437b80
> Gecko: acf5660d2048
> Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
> Version: 37.0a1 (2.2 Master)
> Firmware: V188-1
> User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Hi sarsenyev,
   According to comment 37, it will display "Unable to connect. Please connect to a network from Settings." message with OK button. My reproduce result is the same as comment 37. Please help check it again and provide steps/video if you can. Thanks a lot.
Flags: needinfo?(hlu) → needinfo?(sarsenyev)
Sorry, for some reason, the YouTube is failed to upload my video, 
Here is a new link http://youtu.be/XtkhpsGxrXc

Prerequisites:
The Internet is OFF

Steps to repro:
1. Go to "Settings"
2. Navigate to "FireFox Accounts" and select "Create Account or Sing In"
3. Open the "Notification bar" and activate internet either DATA or WIFI
4. Tap the "Privacy Notice" link
5. Turn off Internet from the "Notification bar"
6. Tap the "Mozilla Privacy Policy" from the "Privacy Notice" page

Expected result:
"Check Settings" option is working on the "No Internet Connection" screen

Actual result:
"Check Settings" option is inactive

Flame 2.2

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141210040201
Gaia: e17c5656dbf517d48fb61ac9bc92119e023fd717
Gecko: be1f49e80d2d
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 37.0a1 (2.2 Master)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Flags: needinfo?(sarsenyev)
Attached video Verify1.MP4
According to Comment 41 STR, I can repro this bug on Flame 2.2, please see attachment. 

But once you had entered the step 6 with network on, the bug will not repro any more. Please see video: Verify1.mp4.

Gaia-Rev        e17c5656dbf517d48fb61ac9bc92119e023fd717
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/be1f49e80d2d
Build-ID        20141210040201
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141210.074809
FW-Date         Wed Dec 10 07:48:20 EST 2014
Bootloader      L1TC00011880
Attached video verify_video.MP4
Hi Hubert,
Could you help with it, thanks.
According to comment 40, this issue has been fixed on Flame 2.0 & 2.1, but I could repro this issue on Flame 2.0/2.1 via steps of comment 41. 
Note: Once you had entered the step 6 with network on, the bug will not repro any more. I think it’s on account of browsing cookies and stored data.
See attachment: verify_video.MP4 and logcat_v2.0_1100.txt and logcat_v2.1_1053.txt

Flame 2.0 build:
Gaia-Rev        f3b9806f687fbbd7eba6b0e1f6ebb8bde09840ea
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/12aea1649f5a
Build-ID        20141216000202
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141216.031730
FW-Date         Tue Dec 16 03:17:40 EST 2014
Bootloader      L1TC00011880

Flame 2.1 build:
Gaia-Rev        79286eafe67707d1330966c1b1413b2d0de595d9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/222a62b532db
Build-ID        20141216001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141216.034704
FW-Date         Tue Dec 16 03:47:14 EST 2014
Bootloader      L1TC00011880
Flags: needinfo?(hlu)
Attached file logcat_v2.0_1100.txt
Attached file logcat_v2.1_1053.txt
Attached video Woodduck.MP4
According to steps of comment 41, the problem is verified still happen in latest build of Woodduck 2.0M
Found time: 23:11
See attachment: Woodduck.MP4 and logcat_2.0M_2311.txt
Rate: 100%

Woodduck build:
Gaia-Rev        7c4ec3494cf465bff688cb4a62f3c2d11a035e14
Gecko-Rev       aa9ece9c8a749095834e7a980166543d985a3af5
Build-ID        20141218050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2
FW-Incremental  1418850327
FW-Date         Thu Dec 18 05:05:50 CST 2014
Attached file logcat_2.0M_2311.txt
I guess the fixed problem is that when there is no data and wifi connection, after step d of comment 0, it won't see the step e, it will remind you "Unable to connect", and you can only tap "OK" button back to Firefox Accounts screen.
Hubert, could you help to check it, thanks.
Hmm. Alive, is there an easy way to prevent the terms/privacy pages from being cached? We can set some no-cache headers on the server, but I wonder if it's possible using the attributes of the iframe?
Flags: needinfo?(alive)
(In reply to Jared Hirsch [:_6a68] [NEEDINFO pls] from comment #50)
> Hmm. Alive, is there an easy way to prevent the terms/privacy pages from
> being cached? We can set some no-cache headers on the server, but I wonder
> if it's possible using the attributes of the iframe?

No, there is no API for that. What's the real problem?
Flags: needinfo?(alive)
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #51)
> (In reply to Jared Hirsch [:_6a68] [NEEDINFO pls] from comment #50)
> > Hmm. Alive, is there an easy way to prevent the terms/privacy pages from
> > being cached? We can set some no-cache headers on the server, but I wonder
> > if it's possible using the attributes of the iframe?
> 
> No, there is no API for that. What's the real problem?

Hm wait,
browser.setAttribute('mozprivatebrowsing', 'true');

Not sure if private browsing help? From the description of desktop ver. of private browsing it should not cache anything.
(In reply to Norry.L.F from comment #34)
> Created attachment 8534226 [details]
> Verify2.0&2.1.MP4
> 
> This issue can be reproduced on Flame 2.0 and Flame 2.1
> 
> Attched: Verify2.0&2.1.MP4, logcat_v2.0_2219.txt, logcat_v2.1_2251.txt
> Found time:v2.0(22:19),v2.1(22:51)
> Rate: 100%
> 
> Flame 2.0 versions:
> Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
> Gecko-Rev      
> https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/2d0860bd0225
> Build-ID        20141209160202
> Version         32.0
> Device-Name     flame
> FW-Release      4.4.2
> FW-Incremental  eng.cltbld.20141209.192800
> FW-Date         Tue Dec  9 19:28:11 EST 2014
> Bootloader      L1TC00011880
> 
> Flame 2.1 versions:
> Gaia-Rev        c226db212db4d824c09617cd6dc407b2d4258d9b
> Gecko-Rev      
> https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/cf8bebfa4703
> Build-ID        20141209170126
> Version         34.0
> Device-Name     flame
> FW-Release      4.4.2
> FW-Incremental  eng.cltbld.20141209.212104
> FW-Date         Tue Dec  9 21:21:15 EST 2014
> Bootloader      L1TC00011880


Hi, Norry,

After I traced the timeline of this bug, you can reproduce this bug just because the patch wasn't merged into v2.0 and v2.1.

:)
Hi, Jared,

Thanks for your help! :)
I am happy to see you that completing the unit test test cases before merge this patch. (+1 like)

-----------------------------------------------------------
Hi, folks,

I think this bug can be triggered via following scenarios.

1) Tap "Check Settings" on FxOS account page with no internet access. The "Check Settings" button cannot work.
   => Jared has helped solved this problem and Sue verified this.
      The patch did is hide the "Check Settings" button, right?
      But, I have some concerns on this. The "Check Settings" button is in order to help user quick access WIFI settings.
      Why do we hide "Check Settings" button?
  
2) Tap "Privacy Notice" on FxOS account page with internet access, and then tap "Mozilla Privacy Policy" from the "Privacy Notice" page with no internet access. User can see "Check Settings" button and it cannot work.
   => You also can reproduce this bug without Jared's patch.


So, the second scenario is not solved yet.
I suggest that we can file a follow up instead of backing out this patch.
What do you think?

Have a nice day. :)
Hi William,

I think it makes sense to file a followup, since the only remaining issue is, if you have recently visited the privacy page, it will be cached. So, if you lose connectivity, reopen the cached privacy page, and click a link within that page, you get the broken settings activity button. This seems like a very remote edge case to me.

Does someone need to agree to this decision on the partner side?

Cheers,

Jared
Add NI?whsu to follow up.

Thanks  Jared! :)
Happy New Year!
Flags: needinfo?(hlu) → needinfo?(whsu)
Blocks: 1118669
Thanks everyone!

A follow up is submitted. (Bug 1118669)
No longer blocks: 1118669
Flags: needinfo?(whsu)
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.