Closed Bug 1082741 Opened 10 years ago Closed 10 years ago

[FxA] Unable to progress past Age Verification page when creating a new FxA

Categories

(Firefox OS Graveyard :: FxA, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S7 (24Oct)
blocking-b2g 2.2+
Tracking Status
b2g-v2.1 --- unaffected
b2g-v2.2 --- verified

People

(Reporter: smiko, Assigned: jhirsch, NeedInfo)

References

()

Details

(Keywords: regression, smoketest)

Attachments

(2 files)

Attached file FxA.txt
Description: When creating a new Firefox Account in Settings, Year of Birth

Repro Steps:
1: Update a Flame to 20141014040203
2: Open Settings > Firefox Accounts > Create or Sign in
3: Enter a valid email address that is not linked to a Firefox account.
4: Tap on "Year of Birth"

Actual: The "Year of Birth" list does not populate

Expected: The "Year of Birth" populates.

Flame 2.2(319mb/full flash)
Device: Flame 2.2
BuildID: 20141014040203
Gaia: 4f86c631e0465c0e56ccebeb1324fd28be9ea32f
Gecko: 54217864bae9
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 36.0a1 (2.2)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Notes:
1: When this issue occurs, the user can view the age dropdown by hitting the X. However, after selecting an age, the user is returned to the first account creation screen. (see video)

Repro frequency: 100%

Link to failed test case: https://moztrap.mozilla.org/manage/case/13006/

See attached: logcat

Video clip: http://youtu.be/2v5zxOYGMGQ
This issue does NOT repro on Flame 2.2 (319mb/Full flash/20141013040202) or Flame 2.1 (319mb/full flash)

Actual Result: The "Year of Birth" list populates.

Device: Flame 2.2
BuildID: 20141013040202
Gaia: 3b81896f04a02697e615fa5390086bd5ecfed84f
Gecko: f547cf19d104
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.1(319mb)
Device: Flame 2.1
BuildID: 20141014001201
Gaia: 7e2e65a9668123b54c8cce5dacfdba6f4bd4672b
Gecko: 2325da834971
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: smoketest
[Blocking Requested - why for this release]:
Functional Regression of a core feature that fails smoke tests.

Requesting a window.
blocking-b2g: --- → 2.2?
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Assignee: nobody → 6a68
QA Contact: jmitchell
blocking 2.2+
blocking-b2g: 2.2? → 2.2+
I see a SyntaxError in the debugger when I tap the COPPA date selector:

    SyntaxError: JSON.parse: unexpected character at line 1 column 2 of the JSON data

When I trace the error in the debugger, it looks (at first glance) like the ValueSelector.show method is trying to JSON.parse an object (detail.choices) that has already been JSON.parsed, lines 195-196[1]:

    195    if (detail.choices) {
    196      detail.choices = JSON.parse(detail.choices);

It's odd that there's no guard clause here to ensure detail.choices hasn't already been parsed from text into an object. However, I doubt this is the actual root cause, since git blame tells me that this code hasn't been touched in a _long_ time (since october of last year).

Bisecting gaia now to pinpoint the regression.

[1] https://github.com/mozilla-b2g/gaia/blob/2a536e4df82410178d8440cc710d8f838a95a0b9/apps/system/js/value_selector/value_selector.js
B2G-Inbound Regression Window:

Last Working:
Device: Flame 2.2
Build ID: 20141012213754
Gaia: 6d5087045b2cdaff2cbd23917f3b3b737d55a523
Gecko: bd568cea1478
Version: 35.0a1 (2.2)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0


Broken by: 	Bug 1049439 - change appWindowManager from singleton to class

First Broken:
Device: Flame 2.2
Build ID: 20141012232250
Gaia: c44510bb576ade5edce60c13eb8d18ee2c7a2ed2
Gecko: 57dfd6498cf8
Version: 35.0a1 (2.2)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0


Gaia/Gecko Swap
Last Working Gaia First Broken Gecko: Issue DOES NOT reproduce
Gaia: 6d5087045b2cdaff2cbd23917f3b3b737d55a523
Gecko: 57dfd6498cf8
First Broken Gaia Last Working Gecko: Issue DOES reproduce
Gaia: c44510bb576ade5edce60c13eb8d18ee2c7a2ed2
Gecko: bd568cea1478

Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/6d5087045b2cdaff2cbd23917f3b3b737d55a523...c44510bb576ade5edce60c13eb8d18ee2c7a2ed2
Blocks: 1049439
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Flags: needinfo?(6a68)
Thanks very much, Joshua.

Using today's m-c build of gecko, I can confirm that gaia commit c0ad769 introduces the regression.

Digging a bit deeper, it looks like the select element's change event is being fired twice: first by AppWindowManager, then by SystemDialogManager, both in response to the same mozChromeEvent. Unfortunately, it looks like the intention is for SDM to catch the event first, based on this comment[1] inside the SystemDialogManager event handler code:

    SystemDialogManager.prototype.handleEvent = function sdm_handleEvent(evt) {
  ...
        case 'mozChromeEvent':
  ...
  -->     // Making sure app-window does not receive this event.
          evt.stopImmediatePropagation();
          this.states.activeDialog.broadcast('inputmethod-contextchange',
            evt.detail);
          break;
    }
  };

I added a couple of console.logs to the spots where AWM and SDM attach mozChromeEvent listeners, and it does indeed look like awm_start is called first, so presumably its handler is invoked first by Gecko.

It appears that what's actually happening is that the AWM is creating the select options window as a div with id AppWindow_1, but it is assigning it the wrong z-index, so that it's drawn underneath the fxa SystemDialog:

(inside the system app debugger, after clicking the select element in the fxa coppa screen):
  <  aw = document.getElementById('AppWindow_1')
  <  window.document.defaultView.getComputedStyle(aw).getPropertyValue('z-index')
  >  "6"
  <  fxaDialog = document.getElementById('fxa-dialog')
  <  window.document.defaultView.getComputedStyle(fxaDialog).getPropertyValue('z-index')
  >  "100"

Anyway, this is getting a bit deep into the system internals.

George, any sense of the best fix here? I suspect that you might want to adjust the load order of SDM and AWM, rather than simply reverting the commit that caused this regression. I'm happy to help out, if you're short on time and can describe the changes needed.

[1] https://github.com/mozilla-b2g/gaia/blob/ef637f1/apps/system/js/system_dialog_manager.js#L118
Flags: needinfo?(6a68) → needinfo?(gduan)
Sorry, it's my mistake. I think I should keep the same init order to prevent more regression coming.
Flags: needinfo?(gduan)
Attached file PR to master
Hi Alive,
in bug 1049439, I change the module initializing order which cause this bug. I think I should keep the same order as before to prevent hidden regression. 

could you check it? Thanks.
Attachment #8505166 - Flags: review?(alive)
Comment on attachment 8505166 [details] [review]
PR to master

Please add some comments in code to explain why we need to keep the order.
Attachment #8505166 - Flags: review?(alive) → review+
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
merge to master: https://github.com/mozilla-b2g/gaia/commit/93fb5070782a549977cf8c485c5d93de54ec657e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S7 (24Oct)
Verified Fixed on the latest Flame 2.2 build.
User is able to progress past Age Verification properly when creating a new Firefox Account.

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20141103040202
Gaia: bc168c17474dabbcceaa349e9bc7c95654435aec
Gecko: 5999e92e89ff
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga) → needinfo?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: