-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SecureContext and API behavior of browsers #34
Conversation
I think this PR would benefit from @foolip's review. |
Probably want a nod from @annevk too :) |
This doesn't end up invoking the failure callback. |
Will add. Totally forgot to check that 🙃 |
For reference, in Blink the failure callback is called with a PositionError where code is PERMISSION_DENIED. |
I’ll add some tests to see what we get across browsers. |
Updated spec text and added tests web-platform-tests/wpt#18715
cc @cdumez |
idlharness.js tests will be updated by web-platform-tests/wpt#18717 after the spec change is merged. |
Forgot to push before 🤪 should be good to review now. |
Eventually, we need to go through and rewrite all these algorithms properly. |
@marcoscaceres Is WebKit returning POSITION_UNAVAILABLE instead of PERMISSION_DENIED the only thing failing in Safari? I am happy to fix this: |
@cdumez, awesome! thanks for fixing it so quickly! |
@reillyeon could you review the tests also? Then we should be good to land. |
@anssiko could you please review the tests? |
@Honry PTAL |
Ok, tests merged. Please merge this when possible. @anssiko, could you grant me write acceptance? |
Thanks @marcoscaceres for championing this fix and others for updating the implementations, we're ready to land now. @marcoscaceres you have been granted write. |
Closes #25
The following tasks have been completed:
Implementation commitment:
Preview | Diff