Add SecureContext and API behavior of browsers

w3c / geolocation

W3C Geolocation API

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

81 stars 56 forks source link

Add SecureContext and API behavior of browsers #34

Closed marcoscaceres closed 5 years ago

marcoscaceres commented 5 years ago

Closes #25

The following tasks have been completed:

[x] Modified Web platform tests

Implementation commitment:

[x] WebKit (https://bugs.webkit.org/show_bug.cgi?id=201221)
[X] Chromium - all tests pass
[X] Gecko - all tests pass

Preview | Diff

anssiko commented 5 years ago

I think this PR would benefit from @foolip's review.

marcoscaceres commented 5 years ago

Probably want a nod from @annevk too :)

annevk commented 5 years ago

This doesn't end up invoking the failure callback.

marcoscaceres commented 5 years ago

Will add. Totally forgot to check that 🙃

reillyeon commented 5 years ago

For reference, in Blink the failure callback is called with a PositionError where code is PERMISSION_DENIED.

marcoscaceres commented 5 years ago

I’ll add some tests to see what we get across browsers.

marcoscaceres commented 5 years ago

Updated spec text and added tests https://github.com/web-platform-tests/wpt/pull/18715

Chrome and Firefox are aligned
Safari returns error code 2 instead of 1.

cc @cdumez

foolip commented 5 years ago

idlharness.js tests will be updated by https://github.com/web-platform-tests/wpt/pull/18717 after the spec change is merged.

marcoscaceres commented 5 years ago

Forgot to push before 🤪 should be good to review now.

marcoscaceres commented 5 years ago

Eventually, we need to go through and rewrite all these algorithms properly.

cdumez commented 5 years ago

@marcoscaceres Is WebKit returning POSITION_UNAVAILABLE instead of PERMISSION_DENIED the only thing failing in Safari? I am happy to fix this: https://bugs.webkit.org/show_bug.cgi?id=201221

marcoscaceres commented 5 years ago

@cdumez, awesome! thanks for fixing it so quickly!

marcoscaceres commented 5 years ago

@reillyeon could you review the tests also? Then we should be good to land.

marcoscaceres commented 5 years ago

@anssiko could you please review the tests?

anssiko commented 5 years ago

@Honry PTAL

marcoscaceres commented 5 years ago

Ok, tests merged. Please merge this when possible. @anssiko, could you grant me write acceptance?

anssiko commented 5 years ago

Thanks @marcoscaceres for championing this fix and others for updating the implementations, we're ready to land now. @marcoscaceres you have been granted write.