1151105 - [Settings]Attempting to enter Call Settings will prevent entering all Settings sub menus

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

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

Attachment: Settings menus not working logcat

Description:
When the user attempts to enter Call Settings, the button will highlight but the screen will not change.  After this, no other settnings sub menus will function.


Repro Steps:
1) Update a Flame to 20150403010203
2) Open Settings
3) Press Call Settings


Actual:
Settings sub menus no longer open.


Expected:
The sub menus continue to open.


Environmental Variables:
Device: Flame 3.0
Build ID: 20150403010203
Gaia: 7969b367a7da62877c3a24a26d3cb5fda89d766c
Gecko: 70a113676b21
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0


Repro frequency: 10/10
See attached: video clip, logcat

Comment 1

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

This issue does NOT occur on Flame 2.2

Actual Results: Settings sub menus open normally.

Environmental Variables:
Device: Flame 2.2
BuildID: 20150403002503
Gaia: 022eeb91197ba4a9adfd67bd6db5aa03cc69eb31
Gecko: 77fdc6fbcae9
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
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

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

Please see this video: https://youtu.be/uBfr3ZUBszY

Comment 3

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

[Blocking Requested - why for this release]:

This breaks all the sub menus in seetings so nominating 3.0?

Comment 4

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

Bug 1087811 seems to have caused this issue.

B2g-inbound Regression Window

Last Working 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150402090834
Gaia: 46e47f3f29f6650be8c70af42aaa298b1894120c
Gecko: b7d2589a0415
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

First Broken 
Environmental Variables:
Device: Flame 3.0
BuildID: 20150402094334
Gaia: 62042ffcc8c6cca0f51ad23f5c2b979fc153b5a7
Gecko: f0489a76061b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Last Working gaia / First Broken gecko - Issue does NOT occur
Gaia: 46e47f3f29f6650be8c70af42aaa298b1894120c
Gecko: f0489a76061b

First Broken gaia / Last Working gecko - Issue DOES occur
Gaia: 62042ffcc8c6cca0f51ad23f5c2b979fc153b5a7
Gecko: b7d2589a0415

Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/46e47f3f29f6650be8c70af42aaa298b1894120c...62042ffcc8c6cca0f51ad23f5c2b979fc153b5a7

Comment 5

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

James, can you take a look at this please? This might have been caused by the landing for bug 1087811.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] jrburke:bug1151105-usestrict-call-settings > mozilla-b2g:master

Comment 7

[James Burke [:jrburke]]

Comment on attachment 8588278 [details] [review]
[gaia] jrburke:bug1151105-usestrict-call-settings > mozilla-b2g:master

With the change in bug 1087811, "use strict" inside functions are correctly kept after a build.

call_icc_handler.js was running into problems with "use strict" rules because the `(function(window, document) { })(this, document);` wrapper under "use strict" inside a function mean that "this" is set to `undefined` for functions that are not called as part of an object method invocation. More details in the second paragraph here:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode#.22Securing.22_JavaScript

The function passing in those global values was not really doing much, just might help with slightly smaller minification for those variables. However, it does not help with protection from globals (we explicitly want those globals), and it does lead to this use strict hazard.

While call_icc_handler.js is the source of this bug, I went ahead and changed other files that were using this idiom, as they have the potential to be a hazard if they do become wrapped in an outer function with 'use strict' set inside it (the r.js build tool still strips top level "use strict" calls since mixing non-strict and strict code can lead to bugs, but now keeps 'use strict' indented inside function wrappers).

So the end result is by fixing a correctness issue with the build, it triggered this use strict behavior -- if the file was run in source mode in the browser, it would have failed this way too.

Comment 8

[James Burke [:jrburke]]

Manually merged this to fix this issue. While this is not our policy to do this, we are entering a holiday weekend where Monday is a holiday for the likely people that will be reviewing, and I would prefer to not break people trying to use nightly builds with this problem.

I am also preferring to do this merge manually over backing out of bug 1087811 because bug 1087811 has been in master for a few days now and if people have started merging app changes that use r.js and assume they can use newer ES6 features enabled by the changes in bug 1087811, that could lead to other breakages if it is backed out.

Also, the fix is very straight-forward, and is the correct change due to 'use strict' rules.

I am still going to leave review open on this though, and if the reviewer would like me to do changes I am happy to do them quickly. I apologize for pushing code into the app outside of normal process, but also feel it is the least disruptive change that will restore this functionality to the app.

Merged in master:
https://github.com/mozilla-b2g/gaia/commit/3aec1208a1ce8ec65ffcc94355fe4f7f641dd573

from pull request:
https://github.com/mozilla-b2g/gaia/pull/29339

Comment 10

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8588278 [details] [review]
[gaia] jrburke:bug1151105-usestrict-call-settings > mozilla-b2g:master

Thank you for making the change and the clarification, James! I never aware of this before. 

I found that there are a few scripts with this problem in settings app, I will file a follow up bug tracking it.

Comment 11

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

This issue is verified fixed on the latest Nightly 3.0 build.

Actual Results: The user can enter the call settings sub menu.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150408010203
Gaia: 84cbd4391fb7175d5380fa72c04d68873ce77e6d
Gecko: 078128c2600a
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0