903055 - Test geolocation permission

Status: RESOLVED FIXED

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

Description

[Marco Castelluccio [:marco]]

Attachment: test_geolocation_webapprt

Test that a prompt appears iff the geolocation permission is specified in the manifest.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

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."

Comment 2

[Myk Melez [:myk] [@mykmelez]]

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

Comment 3

[Marco Castelluccio [:marco]]

Attachment: test_geolocation_webapprt

Did you test with the patches from bug 892837 applied?

Comment 4

[Myk Melez [:myk] [@mykmelez]]

(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

Comment 5

[Marco Castelluccio [:marco]]

I've tested on Win, Linux and Mac and can't reproduce the test failure.

Comment 6

[Marco Castelluccio [:marco]]

Could you retest now? Maybe there was something different in our patch queues.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

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!

Comment 8

[Myk Melez [:myk] [@mykmelez]]

(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.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Went ahead and fixed the commit message for you.

https://hg.mozilla.org/integration/mozilla-inbound/rev/cd5a8bda1196

Comment 10

[Marco Castelluccio [:marco]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #9)
> Went ahead and fixed the commit message for you.

Sorry!

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cd5a8bda1196