Last Comment Bug 751403 - Telephony tests: JavascriptException: TypeError: navigator is undefined
: Telephony tests: JavascriptException: TypeError: navigator is undefined
Status: RESOLVED FIXED
:
Product: Testing
Classification: Components
Component: Marionette (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla15
Assigned To: Jonathan Griffin (:jgriffin)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-02 16:07 PDT by Jonathan Griffin (:jgriffin)
Modified: 2012-05-03 13:46 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
telephony tests patch (4.25 KB, patch)
2012-05-02 16:11 PDT, Jonathan Griffin (:jgriffin)
malini: review+
Details | Diff | Splinter Review

Description User image Jonathan Griffin (:jgriffin) 2012-05-02 16:07:36 PDT
I got the webapi tests running again, see http://brasstacks.mozilla.com/autolog/?tree=b2g&source=autolog.  Unfortunately, something broke the telephony tests between now and the last time they ran, 24 days ago.
Comment 1 User image Jonathan Griffin (:jgriffin) 2012-05-02 16:11:48 PDT
Created attachment 620508 [details] [diff] [review]
telephony tests patch

The fix to all these tests is to replace navigator with window.navigator.  I don't know why this necessary now...I tried looking through commits but I couldn't identify any obvious candidates that would have caused this.

We could add the navigator property directly to the sandbox instead; I don't really have an opinion on which is better, do you?
Comment 2 User image Malini Das [:mdas] - Away, not checking bugmail 2012-05-03 09:20:48 PDT
Comment on attachment 620508 [details] [diff] [review]
telephony tests patch

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

Looks good!

I would prefer not to include navigator in the sandbox if we don't have to. It makes the test unclear, and it makes the Marionette server responsible for things it shouldn't care about.
Comment 3 User image Jonathan Griffin (:jgriffin) 2012-05-03 11:43:29 PDT
http://hg.mozilla.org/mozilla-central/rev/a0356446a56a
Comment 4 User image :Gavin Sharp [email: gavin@gavinsharp.com] 2012-05-03 12:42:43 PDT
(In reply to Jonathan Griffin (:jgriffin) from comment #1)
> The fix to all these tests is to replace navigator with window.navigator.  I
> don't know why this necessary now...I tried looking through commits but I
> couldn't identify any obvious candidates that would have caused this.

This seems like something worth investigating, lest it end up being caused by a more serious bug that causes more problems later. It shouldn't be necessary to prefix navigator with "window.".
Comment 5 User image Jonathan Griffin (:jgriffin) 2012-05-03 13:35:43 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #4)
> 
> This seems like something worth investigating, lest it end up being caused
> by a more serious bug that causes more problems later. It shouldn't be
> necessary to prefix navigator with "window.".

Gavin's right; this is a regression in gecko and not something to do with Marionette.  I'll file a separate bug.
Comment 6 User image Jonathan Griffin (:jgriffin) 2012-05-03 13:46:23 PDT
Filed as bug 751699

Note You need to log in before you can comment on or make changes to this bug.