1495364 - FeaturePolicy: geolocation

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Andrea Marchesini [:baku]]

Supporting for 'geolocation' feature policy.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: A_feature_geolocation.patch

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9013219 [details] [diff] [review]
A_feature_geolocation.patch

https://bugzilla.mozilla.org/show_bug.cgi?id=1495362#c3 applies here too.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9013219 [details] [diff] [review]
A_feature_geolocation.patch

https://bugzilla.mozilla.org/show_bug.cgi?id=1495362#c6 applies here too.

 nsCOMPtr<nsIDocument> doc = win->GetDoc();
should use GetExtantDoc()

Comment 4

[Andrea Marchesini [:baku]]

Attachment: A_feature_geolocation.patch

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 9014731 [details] [diff] [review]
A_feature_geolocation.patch

>+function test_currentPosition() {
>+  navigator.geolocation.getCurrentPosition(() => {
>+    tests.push(1);
>+    test_watchPosition();
>+  }, () => {
>+    tests.push(0);
>+    test_watchPosition();
>+  })
>+}
>+
>+function test_watchPosition() {
>+  navigator.geolocation.watchPosition(() => {
>+    tests.push(1);
>+    send_results();
>+  }, () => {
>+    tests.push(0);
>+    send_results();
>+  });
>+}
What are these magical 0 and 1 values in the tests array? Couldn't you just push "denied" or "allowed"


>+
>+var tests = [
>+  [ "geolocation 'none'", "denied"],
>+  [ "geolocation", "allowed"],
>+  [ "geolocation 'src'", "allowed"],
>+  [ "geolocation 'self'", "allowed"],
>+  [ "geolocation *", "allowed"],
>+  [ "geolocation http://random.net", "denied"],
>+];
Please add a test without any featurepolicy (in which case allow attribute isn't set) and ensure its behavior doesn't change with or without the patch.


>+
>+function nextTest() {
>+  if (tests.length == 0) {
>+    SimpleTest.finish();
>+    return;
>+  }
>+
>+  let test = tests.shift();
>+
>+  var iframe = document.createElement("iframe");
>+  iframe.setAttribute("allow", test[0]);
>+
>+  window.continueTest = event => {
>+    delete window.continueTest;
event isn't an event, just a message. So, don't call the argument event. Perhaps msg

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd12d054f76c
FeaturePolicy: geolocation, r=smaug

Comment 7

[Stefan Hindli [:stefan_hindli]]

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

Comment 8

[Florian Scholz (Open Web Docs)]

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy/geolocation