1176769 - [Settings][Media Storage] Default Media Location selector will appear over top of and after the confirmation prompt the first time it is accessed.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Input Mgmt, defect)

Description

[Martin Shuman [:Marty]]

Attachment: logcat_media_location.txt

Description:
The first time the Default Media Location is accessed, the option selector will appear over top of warning confirmation pop up.  Any changes the user makes the first time it appears will not be preserved or applied, and the user will have to reselect their desired option after seeing and confirming the warning prompt, and being presented with the selector again.

If the user selects to change this option again, the confirmation prompt will properly appear first, and the selector will only appear once after that, functioning properly.

This issue occurs the first time this option is selected each time the Settings App is launched.

Prerequisite: Have an SD card in the device.

Repro Steps:
1) Update a Flame to 20150622010206
2) Launch the Settings app
3) Navigate to Media storage
4) Scroll down and select the Default Media Location option

Actual:
The location selector appears first, then the confirmation prompt appears after the location is selected, and then the location selector appears again.  Changes made to the first selector are not applied.

Expected:
The confirmation message appears first, and the location selector appears after the warning is confirmed. Changes made in the selector are properly applied.

Environmental Variables:
Device: Flame 3.0
Build ID: 20150622010206
Gaia: d8bac9577a537b5e6bafc3f7ed088af5ea60c99e
Gecko: a38c23ccca0a
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 41.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Repro frequency: 10/10
See attached: logcat, video (URL)

Comment 1

[Martin Shuman [:Marty]]

This issue DOES occur on Spark 3.0 builds.
The location selector appears first, then the confirmation prompt appears after the location is selected, and then the location selector appears again.  Changes made to the first selector are not applied.

Environmental Variables:
Device: Aries 3.0
Build ID: 20150619225606
Gaia: 4c06ed88ddccaba8dc941e5006bd2a9e57306f07
Gecko: 7c1a6b1151a1539186b950a144387e2d7f378d1b
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 41.0a1 (3.0)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

--------------------------------------

This issue does NOT occur on Flame 2.2 builds.
The confirmation message appears first, and the location selector appears after the warning is confirmed. Changes made in the selector are properly applied.

Environmental Variables:
Device: Flame 2.2
Build ID: 20150621002506
Gaia: 1f8981d7872e3c0053571c26fb3edaf401844d75
Gecko: 9a0d8f7b1200
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 2

[Oliver Nelson [:oliverthor]]

[Blocking Requested - why for this release]:
UI Flow regression that imposes on the user to perform the same action multiple times. NI component owner for blocking decision. Also appears to be a Window Management issue, ni? that owner as well.

Adding regressionwindow-wanted

Comment 3

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

The changes for Bug 1162360 seem to have caused this issue.

Mozilla-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150614213841
Gaia: 1bf2da102560481748ff3f6202fbed5c4daa5832
Gecko: a493653ebbed
Version: 41.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

First Broken 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150615000842
Gaia: 1bf2da102560481748ff3f6202fbed5c4daa5832
Gecko: 85383f7a9d53
Version: 41.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Last Working gaia / First Broken gecko - Issue DOES occur
Gaia: 1bf2da102560481748ff3f6202fbed5c4daa5832
Gecko: 85383f7a9d53

First Broken gaia / Last Working gecko - Issue does NOT occur
Gaia: 1bf2da102560481748ff3f6202fbed5c4daa5832
Gecko: a493653ebbed

Gecko Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a493653ebbed&tochange=85383f7a9d53

Comment 4

[KTucker [:KTucker][Inactive 3/4/2016]]

Tim, can you take a look at this please? This looks to have been caused by the landing for bug 1162360.

Comment 5

[Hermes Cheng (inactive after July 27, 2015)]

Thanks. wait for Tim's response.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Ouch.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Github: https://github.com/mozilla-b2g/gaia/pull/30749

There is a race previously hidden by bug 1162360. Settings app is written such that it will call blur() on the <select> when the user click it (which causes it to focus). The two actions previously results no async message from forms.js as they cancelled out, but forms.js now begin to send the two events since bug 1162360.

Comment 8

[Etienne Segonzac (:etienne)]

Comment on attachment 8627559 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/30749

The change looks good, please add a unit test to cover it, I think we can go with something like
```
  test('prevent blur race', function() {
    rafStub.restore();
    this.sinon.stub(window, 'requestAnimationFrame');

    vs.handleEvent(fakeDateInputMethodContextChangeEvent);
    window.requestAnimationFrame.yield();

    vs.handleEvent(fakeBlurInputMethodContextChangeEvent);
    window.requestAnimationFrame.yield();

    assert.isTrue(vs.element.hidden);
  });

```

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Unit test added. This patch is now blocked on Gij6 test.

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I can now produce the failure on Linux. The keyboard got removed prematurely. It's possible we are revealing a race by fixing value selector here.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Without digging too deep, I have found the issue of test failure to be this in ValueSelector#hide:

    if (this.element.hidden) {
      return;
    }

This generates TypeError when this.element is null (meaning when the value selector is not actually initialized but when we are asked to handle a blur event).

So I have fix it to be

    if (!this.element || this.element.hidden) {
      return;
    }

And the tests passes locally.

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Finally passes...

master: https://github.com/mozilla-b2g/gaia/commit/b04dad3301337408048da05f2f637cf6bf540213

Comment 13

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is verified fixed on the latest Nightly Flame 2.5 build.

Actual results: The warning prompt will appear first.

Environmental Variables:
Device: Flame 2.5
BuildID: 20150707010204
Gaia: e6506d68e01489b57616f8b74e8773f938ce62b3
Gecko: e7e69cc8c07b
Gonk: a4f6f31d1fe213ac935ca8ede7d05e47324101a4
Version: 42.0a1 (2.5) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0