Closed Bug 995844 Opened 10 years ago Closed 10 years ago

test_dsds_mobile_data_connection.js retrieve mozMobileConnection from a wrong frame

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(tracking-b2g:backlog)

RESOLVED FIXED
1.4 S6 (25apr)
tracking-b2g backlog

People

(Reporter: vicamo, Assigned: vicamo)

References

Details

(Whiteboard: [p=1])

Attachments

(1 file, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #995107 +++

|let connections = window.navigator.mozMobileConnections;|
http://mxr.mozilla.org/mozilla-central/source/dom/mobileconnection/tests/marionette/test_dsds_mobile_data_connection.js#9

However, when "head.js" is included, we open an iframe and run mobileconnection tests in it instead of current window.  Bug 995107 introduced a new global variable "workingFrame" for this.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → vyang
(In reply to Jessica Jong [:jjong] [:jessica] from comment #2)
> Thank you Vicamo.
> 
> Do we need to indicate the working frame to use for cases like [1][2][3],
> where we get mozMobileConnection for a specific service id?
> 
> [1]
> http://mxr.mozilla.org/mozilla-central/source/dom/mobileconnection/tests/
> marionette/head.js#285
> [2]
> http://mxr.mozilla.org/mozilla-central/source/dom/mobileconnection/tests/
> marionette/head.js#410
> [3]
> http://mxr.mozilla.org/mozilla-central/source/dom/mobileconnection/tests/
> marionette/head.js#470

Thank you for catching these.  Sure, I should correct them all at once. :)
Attached patch patch : v2 (obsolete) — Splinter Review
1) use 'workingFrame' whenever necessary,
2) use getNumOfRadioInterfaces instead.

Mnw only: https://tbpl.mozilla.org/?tree=Try&rev=b7fac81d99d6
Attachment #8405950 - Attachment is obsolete: true
Attachment #8405998 - Flags: review?(echen)
blocking-b2g: --- → backlog
Comment on attachment 8405998 [details] [diff] [review]
patch : v2

Review of attachment 8405998 [details] [diff] [review]:
-----------------------------------------------------------------

This patch looks good to me, but I have one question.
Do we need to retrieve mozSettings from 'workingFrame' [1][2] as well?

Thank you.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/tests/marionette/head.js#96
[2] http://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/tests/marionette/head.js#123
Attachment #8405998 - Flags: review?(echen)
(In reply to Edgar Chen [:edgar][:echen] from comment #5)
> Review of attachment 8405998 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This patch looks good to me, but I have one question.
> Do we need to retrieve mozSettings from 'workingFrame' [1][2] as well?
> 
> Thank you.

The first time when Jessica raised this question, I thought mozSettings is unrelated to mozMobileConnection.  However, now I suddenly realize that mozSettings related permissions are also only installed in that workingFrame.  So, yes, I should have corrected accesses to mozSettings as well.  Thank you :)
Attached patch patch : v3Splinter Review
Address previous comment.
Attachment #8405998 - Attachment is obsolete: true
Attachment #8407527 - Flags: review?(echen)
Comment on attachment 8407527 [details] [diff] [review]
patch : v3

Review of attachment 8407527 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you :)
Attachment #8407527 - Flags: review?(echen) → review+
https://hg.mozilla.org/mozilla-central/rev/36e61fd7ed89
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.