Callback with error if doc is not fully active

w3c / geolocation

W3C Geolocation API

https://www.w3.org/TR/geolocation/

81 stars 56 forks source link

Callback with error if doc is not fully active #97

Closed marcoscaceres closed 3 years ago

marcoscaceres commented 3 years ago

Closes #96

The following tasks have been completed:

[x] Modified Web platform tests

Implementation commitment:

[x] WebKit (https://bugs.webkit.org/show_bug.cgi?id=228319)
[ ] Chromium (https://bugs.chromium.org/p/chromium/issues/detail?id=1233329)
[x] Gecko https://phabricator.services.mozilla.com/D117273

Preview | Diff

marcoscaceres commented 3 years ago

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

reillyeon commented 3 years ago

I've filed an issue against Chromium for this and updated the summary. Any chance the test you've added in the Gecko patch could be added to WPT instead?

marcoscaceres commented 3 years ago

Thanks @reillyeon! And I again have to give a huge shout out to @rakina and co. for producing that TAG guide and for her guidance - and to @saschanaz for a super detailed review over on the Gecko side, who caught the potential issue when used with promises.

I'll see if I can create a WPT test. The challenge I've been facing is that the page requires permission to use the API. But I'll see if Webdriver, via WPT, lets me grant a page permission to use the API... Oh! might be in luck!

 await test_driver.set_permission({name: 'geolocation'}, 'granted');

Re non fully active in general: yeah, I've been finding really weird behavior... not sure if you are following the Permission's one, where (unsurprisingly) Chrome and Firefox are producing different results:

https://github.com/w3c/permissions/issues/162#issuecomment-886409154

These are a lot of fun to work on tho! So please ping if there are other APIs that could use a look over.

marcoscaceres commented 3 years ago

Ok, WPT is up https://github.com/web-platform-tests/wpt/pull/29799 - however, I'm unsure who to tag for review? @reillyeon can you suggest anyone (or review it?).

That reminds me... we need to find new folks to review the tests. The current set listed over at WPT don't appear to be very active in this space as they might have moved onto other things.

marcoscaceres commented 3 years ago

Filed WebKit bug https://bugs.webkit.org/show_bug.cgi?id=228319

saschanaz commented 3 years ago

Ok, WPT is up https://github.com/web-platform-tests/wpt/pull/29799 - however, I'm unsure who to tag for review? @reillyeon can you suggest anyone (or review it?).

That reminds me... we need to find new folks to review the tests. The current set listed over at WPT don't appear to be very active in this space as they might have moved onto other things.

It can be included the gecko patch so that it can be automatically upstreamed.

marcoscaceres commented 3 years ago

@saschanaz wrote:

It can be included the gecko patch so that it can be automatically upstreamed.

I was hoping to do that, but Gecko doesn't implement test_driver.set_permission() yet 😢. I could still bundle it in, if you think it's still worth while.

saschanaz commented 3 years ago

Oops!

@jgraham Are we planning to implement test_driver.set_permission() in the foreseeable future?

marcoscaceres commented 3 years ago

Good news! WebKit patch inbound for this too! https://bugs.webkit.org/show_bug.cgi?id=228319 🎉

marcoscaceres commented 3 years ago

@reillyeon, could you by chance point me to how fully active checks are done in Chromium? With a little guidance, maybe I can slap together a Chromium patch? ☺️

reillyeon commented 3 years ago

I left notes on how to do this on https://bugs.chromium.org/p/chromium/issues/detail?id=1233329.