963225 - JavascriptException: TypeError: this._iccCard is null

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Anshul]

I am seeing the following error a lot of times in our marionette automation suite. This is causing our tests to fail intermittently.

JavascriptException: TypeError: this._iccCard is null at: app://system.gaiamobile.org/shared/js/operator_variant_helper.js line: 351

Comment 1

[Anshul]

Attachment: JavascriptException: TypeError: this._iccCard is null

Comment 2

[Anshul]

Attachment: pr.html

Comment 3

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8364569 [details]
pr.html

Anshul, thanks for the patch. I left a comment on the PR. Could you take a look a it please? Thanks!

Comment 4

[Gregor Wagner [:gwagner]]

Anshul, Jose, do you guys need help with this bug?

Comment 5

[Gregor Wagner [:gwagner]]

Anshul, let us know if you need help.

Comment 6

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Setting target milestone to 2/28 per QC request. 
Anshul, please let me know if it's inappropriate.

Comment 7

[José Antonio Olivera Ortega [:jaoo]]

I guess no, thanks. Anshul and I were trying to figure out why we have not seen this in our tests or even in the device. Anshul provided a patch we are currently under the discussion about how to solve this issue. We'll let you know if we need further help. Thanks.

Comment 8

[José Antonio Olivera Ortega [:jaoo]]

Anshul, left some comments on github and even a patch for the issue. It's so similar to yours and I've added a new unit test for the issue.

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Pointer to Github PR https://github.com/mozilla-b2g/gaia/pull/16492

Stealing the bug as Anshul and I agreed on github discussion. Clearing need-info flag as well.

Fabien, our friends at QC hit an issue while running their test. I managed to reproduce the issue somehow. I added a test case in which the mozIcc object is remove and I hit the issue Anshul commented in comment #0. The fix added LGTM since if the mozIcc object is removed is safe to skip the removing of the event listeners as the object is not available anymore. I've run the unit test and everything seems correct. Also I tested this on a device and everything is working fine. Could you take a look at it please? Thanks.

Comment 10

[Anshul]

Fabrice, I am seeing the same issue on 1.3 and wondering if we can uplift this to 1.3 as well. This is a low risk change and hence asking?

Comment 11

[Jason Smith [:jsmith]]

(In reply to Anshul from comment #10)
> Fabrice, I am seeing the same issue on 1.3 and wondering if we can uplift
> this to 1.3 as well. This is a low risk change and hence asking?

That's not a case for blocking - we don't block on bugs for the sake of them being low risk changes. We need a compelling argument against our blocking criteria. Hence, not blocking.

Comment 12

[[:fabrice] Fabrice Desré]

Anshul, you can still nominate the patch for 1.3 especially if that helps you run your tests suite. That looks safe to me.

Comment 13

[Anshul]

Jason, sorry I wasn't clear. This bug is breaking a lot of our marionette test cases on 1.3 so having this bug would mean we would be able to enable all those tests to catch regressions on 1.3 branch due to any unexpected changes to the interface.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Anshul from comment #13)
> Jason, sorry I wasn't clear. This bug is breaking a lot of our marionette
> test cases on 1.3 so having this bug would mean we would be able to enable
> all those tests to catch regressions on 1.3 branch due to any unexpected
> changes to the interface.

Okay - that's a good reason to consider nominating this, as that means this an automation blocker.

Comment 15

[Fabien Cazenave [:kaze]]

Comment on attachment 8379166 [details] [review]
Pointer to Github PR https://github.com/mozilla-b2g/gaia/pull/16492

Looks good to me, thanks for providing the background.

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Fabien Cazenave [:kaze] from comment #15)
> Comment on attachment 8379166 [details] [review]
> Pointer to Github PR https://github.com/mozilla-b2g/gaia/pull/16492
> 
> Looks good to me, thanks for providing the background.

Thanks Fabien!

I'll land the patch in this bug as soon as the bug becomes 1.3+ or 1.4+. AFAIK only 1.3+ or 1.4+ can land on master right now, is that correct?

Comment 17

[Gregor Wagner [:gwagner]]

This can land on master since it's 1.4+. The remaining question is if we should uplift to 1.3.
Please fix the lint errors and land.

Comment 18

[José Antonio Olivera Ortega [:jaoo]]

Lint errors fixed. Test passes, everything seems to work correctly and travis went green. Landed on Gaia master branch at https://github.com/mozilla-b2g/gaia/commit/41ddf8d12140f7310f037d65b1e8a04ba8cb7270

Comment 19

[bhavana bajaj [:bajaj]]

Blocking given comment #12 - #14.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Please request gaia v1.3 approval on this patch as blockers no longer have automatic uplift approval.

Comment 21

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8379166 [details] [review]
Pointer to Github PR https://github.com/mozilla-b2g/gaia/pull/16492

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

[Approval Request Comment]
See comment #13 please
[Bug caused by] (feature/regressing bug #):
Bug 933203 DSDS support for operator variant logic.
[User impact] if declined:
Not user impact at all. It broke QC marionette test.
[Testing completed]:
Yes, unit test added.
[Risk to taking this patch] (and alternatives if risky):
None
[String changes made]:
None

Comment 22

[bhavana bajaj [:bajaj]]

(In reply to Anshul from comment #13)
> Jason, sorry I wasn't clear. This bug is breaking a lot of our marionette
> test cases on 1.3 so having this bug would mean we would be able to enable
> all those tests to catch regressions on 1.3 branch due to any unexpected
> changes to the interface.

Can you please help verify this on a 1.3 nightly build once this is landed on 1.3 branch ? Thanks !

Comment 23

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 41ddf8d12140f7310f037d65b1e8a04ba8cb7270 to:
v1.3: dd2e075277000706d4f6203d6f569f4ee9fcde8d