Skip to content
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

Merged
merged 4 commits into from
Sep 9, 2019
Merged

Add SecureContext and API behavior of browsers #34

merged 4 commits into from
Sep 9, 2019

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented Aug 22, 2019

Closes #25

The following tasks have been completed:

Implementation commitment:


Preview | Diff

@anssiko
Copy link
Member

anssiko commented Aug 22, 2019

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

@marcoscaceres
Copy link
Member Author

Probably want a nod from @annevk too :)

@annevk
Copy link
Member

annevk commented Aug 22, 2019

This doesn't end up invoking the failure callback.

@marcoscaceres
Copy link
Member Author

Will add. Totally forgot to check that 🙃

@reillyeon
Copy link
Member

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

@marcoscaceres
Copy link
Member Author

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

@marcoscaceres
Copy link
Member Author

marcoscaceres commented Aug 28, 2019

Updated spec text and added tests web-platform-tests/wpt#18715

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

cc @cdumez

@marcoscaceres marcoscaceres mentioned this pull request Aug 28, 2019
@foolip
Copy link
Member

foolip commented Aug 28, 2019

idlharness.js tests will be updated by web-platform-tests/wpt#18717 after the spec change is merged.

@marcoscaceres
Copy link
Member Author

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

@marcoscaceres
Copy link
Member Author

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

@cdumez
Copy link

cdumez commented Aug 28, 2019

@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

index.html Outdated Show resolved Hide resolved
@marcoscaceres
Copy link
Member Author

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

@marcoscaceres
Copy link
Member Author

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

@marcoscaceres
Copy link
Member Author

@anssiko could you please review the tests?

@anssiko
Copy link
Member

anssiko commented Sep 6, 2019

@Honry PTAL

@marcoscaceres
Copy link
Member Author

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

@anssiko
Copy link
Member

anssiko commented Sep 9, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrict interfaces to secure contexts?
6 participants