w3c / geolocation

W3C Geolocation API
https://www.w3.org/TR/geolocation/
81 stars 56 forks source link

Deal with not fully active documents #78

Closed marcoscaceres closed 3 years ago

marcoscaceres commented 3 years ago

See guide - example using Geolocation, which is not covered in our spec I think.

Cc @rakina for review.

rakina commented 3 years ago

Thanks! I think this is already somewhat implicitly specified by step 4 of "request position", which waits for the document to become visible, and bfcached/non-fully active documents should not be visible. But I think it is better to make it more explicit (wait for the document to become fully active also).

also cc @fergal @domenic

marcoscaceres commented 3 years ago

Ok, so, it might be worth checking if the document is still active as part of acquire position.

I'll verify what browsers do there... if they silently ignore the request or error.

Tom333Trinity commented 3 years ago

I think the big yellow box might also have something to do with it: -

Not Ready For Implementation This spec is not yet ready for implementation. It exists in this repository to record the ideas and promote discussion.

Also, good luck getting potential contributors to understand this definition of active versus fully-active. Regardless, and IMHO, if your document is top level then do not concern yourself (especially with the client.claim stuff) and as @rakina noted this is pretty much the behaviour that happens now with FG geolocation.

marcoscaceres commented 3 years ago

@Tom333Trinity, first and last warning. Don't start being disruptive or we will ban you again (sigh).

marcoscaceres commented 3 years ago

Did a little digging... both Chromium and WebKit simply do the following check:

if (!frame())
   return 0;

But I couldn't find any other checks for when .watchPosition() is actually running.

It's hard to test if the callbacks are invoked, as even .watchPosition() only fires once.... I'll see if I can try on my phone and move around a bit.

Gecko unfortunately throws. I can probably fix that in Gecko easily enough though.

marcoscaceres commented 3 years ago

@rakina ok, below is a simple test case... I'll covert it to a WPT, but would like your feedback before I do.

The neat thing in all browsers is that removal and reattachment does indeed kill the geolocation service for the frame.

  async function runTest() {
    const iframe = document.createElement("iframe");
    iframe.src = "empty.html";
    document.body.appendChild(iframe);
    console.log("loading iframe");
    await new Promise((r) => {
      iframe.onload = r;
    });

    // steal geolocation
    const geo = win.navigator.geolocation;

    // queue up some position watchers...
    console.log(geo.watchPosition(console.log, console.error));
    console.log(geo.watchPosition(console.log, console.error));
    console.log(geo.watchPosition(console.log, console.error));
    geo.getCurrentPosition(console.log, console.error)
    geo.getCurrentPosition(console.log, console.error)
    geo.getCurrentPosition(console.log, console.error)

    // make doc no longer fully active
    iframe.remove();

    // try to use geolocation
    try {
      console.log("watchPosition", geo.watchPosition(console.log, console.error));
    } catch (err) {
      console.log(err);
    }

    try {
      console.log(
        "getCurrentPosition",
        geo.getCurrentPosition(console.log, console.error)
      );
    } catch (err) {
      console.log(err);
    }
    // re-attach
    console.log("reattach");
    document.body.appendChild(iframe);

    // And we are back! But dead... 
    iframe.contentWindow.navigator.geolocation.getCurrentPosition(
      console.log,
      console.error
    );
    iframe.contentWindow.navigator.geolocation.watchPosition(
      console.log,
      console.error
    );

    console.log("done");
  }

  runTest();
marcoscaceres commented 3 years ago

Filed Gecko bug https://bugzilla.mozilla.org/show_bug.cgi?id=1715482

marcoscaceres commented 3 years ago

Sent Gecko patch: https://phabricator.services.mozilla.com/D117273

marcoscaceres commented 3 years ago

Test file is here: https://marcoscaceres.github.io/playground/geo_test.html

rakina commented 3 years ago

Thanks a lot @marcoscaceres! I think the test looks good, neat that you can test it with just iframes. CC-ing @hiroshige-g who is working on BFCache WPTs so that he can make a similar test but with BFCache involved (he's also currently working on a guide to write BFCache tests) and @fergald who is involved in BFCache WPT stuff as well.