Closed Bug 903055 Opened 6 years ago Closed 6 years ago

Test geolocation permission

Categories

(Firefox Graveyard :: Webapp Runtime, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 26

People

(Reporter: marco, Assigned: marco)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch test_geolocation_webapprt (obsolete) — Splinter Review
Test that a prompt appears iff the geolocation permission is specified in the manifest.
Attachment #787678 - Flags: review?(myk)
Attachment #787678 - Attachment is patch: true
Comment on attachment 787678 [details] [diff] [review]
test_geolocation_webapprt

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

Three lines add tabs that should be spaces:

/Users/myk/test_geolocation_webapprt:152: space before tab in indent.
    	document.getElementById("msg").textContent = "Failure.";
/Users/myk/test_geolocation_webapprt:191: space before tab in indent.
    	document.getElementById("msg").textContent = "Failure.";
/Users/myk/test_geolocation_webapprt:211: space before tab in indent.
  	"geolocation": { "description": "Desc" }
warning: 3 lines add whitespace errors.

::: webapprt/test/chrome/browser_geolocation-prompt-noperm.js
@@ +1,1 @@
> +let scope = {};

It doesn't look like this definition (nor the identical one in browser_geolocation_prompt-perm.js) is used anywhere.

@@ +32,5 @@
> +    mutObserver = new MutationObserver(function(mutations) {
> +      if (msg.textContent == "Failure.") {
> +        ok(true, "The permission shouldn't be granted.");
> +      } else {
> +        ok(false, "The position shouldn't be granted.");

Nit: these messages should be the same, since they represent the same test.  Also, in general, it's a bit more readable for test messages to state what *is* rather than what *should be*, f.e. "the app is not privileged"; alternately, just drop the verb entirely, as in "principal app ID correct."
Attachment #787678 - Flags: review?(myk) → review-
I also see a test failure when running these tests:

TEST-START | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js
TEST-INFO | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | Console message: OpenGL LayerManager Initialized Succesfully.
Version: 2.1 NVIDIA-8.12.47 310.40.00.05f01
Vendor: NVIDIA Corporation
Renderer: NVIDIA GeForce GT 650M OpenGL Engine
FBO Texture Target: TEXTURE_2D
TEST-PASS | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | Geolocation permission: unknown.
2013-08-12 15:55:01.589 webapprt-stub[58416:8607] invalid context
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | Prompt shown.
Stack trace:
    JS frame :: chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js :: onLoadWindow :: line 15
    native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0

TEST-PASS | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | The permission shouldn't be granted.
INFO TEST-END | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | finished in 1109ms
Did you test with the patches from bug 892837 applied?
Assignee: nobody → mcastelluccio
Attachment #787678 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(In reply to Marco Castelluccio [:marco] from comment #3)
> Did you test with the patches from bug 892837 applied?

Yes, and I still see the failure with the latest versions of those patches:

TEST-START | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js
TEST-PASS | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | Geolocation permission: unknown.
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js | Prompt shown.
Stack trace:
    JS frame :: chrome://mochitests/content/webapprtChrome/webapprt/test/chrome/browser_geolocation-prompt-noperm.js :: onLoadWindow :: line 14
    native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0
Priority: -- → P2
Depends on: 905397
I've tested on Win, Linux and Mac and can't reproduce the test failure.
Could you retest now? Maybe there was something different in our patch queues.
Flags: needinfo?(myk)
Comment on attachment 789682 [details] [diff] [review]
test_geolocation_webapprt

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

(In reply to Marco Castelluccio [:marco] from comment #6)
> Could you retest now? Maybe there was something different in our patch
> queues.

I no longer see the failure!
Attachment #789682 - Flags: review+
Flags: needinfo?(myk)
(In reply to Myk Melez [:myk] [@mykmelez] from comment #7)
> I no longer see the failure!

However, it'd be worth doing a try run to make extra sure that this isn't breaking on any platform.
Keywords: checkin-needed
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #9)
> Went ahead and fixed the commit message for you.

Sorry!
https://hg.mozilla.org/mozilla-central/rev/cd5a8bda1196
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.