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

Adjust registerProtocolHandler() handling #5524

Merged
merged 3 commits into from
Aug 4, 2020

Conversation

annevk
Copy link
Member

@annevk annevk commented May 9, 2020

This makes things a bit more explicit and aligns what was "escaped version" with implementations.

Corresponding URL Standard PR: whatwg/url#513

Tests: web-platform-tests/wpt#23504

Closes #3377.

(See WHATWG Working Mode: Changes for more details.)


/form-control-infrastructure.html ( diff )
/infrastructure.html ( diff )
/system-state.html ( diff )

source Outdated
@@ -2503,7 +2503,8 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<li><dfn data-x="concept-url-equals" data-x-href="https://url.spec.whatwg.org/#concept-url-equals">URL equals</dfn></li>
<li><dfn data-x="serialize an integer" data-x-href="https://url.spec.whatwg.org/#serialize-an-integer">serialize an integer</dfn></li>
<li><dfn data-x-href="https://url.spec.whatwg.org/#default-encode-set">Default encode set</dfn></li>
<li><dfn data-x-href="https://url.spec.whatwg.org/#utf-8-percent-encode">UTF-8 percent encode</dfn></li>
<li><dfn data-x-href="https://url.spec.whatwg.org/#external-string-encode-set">External string encode set</dfn></li>
<li><dfn data-x-href="https://url.spec.whatwg.org/#string-utf-8-percent-encode">string UTF-8 percent encode</dfn></li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both are unavailable yet, probably pending PRs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, see whatwg/url#503. With that out of the way it would be relatively straightforward to add an external string encode set to URL as described in the last bullet point of #3377 (comment).

@domenic
Copy link
Member

domenic commented May 11, 2020

Chrome I would guess from the issue?

How does Chrome do on the tests in web-platform-tests/wpt#23504?

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding "do not merge yet" until whatwg/url#503 gets merged.

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@domenic domenic added the do not merge yet Pull request must not be merged per rationale in comment label May 11, 2020
@annevk
Copy link
Member Author

annevk commented May 12, 2020

How does Chrome do on the tests in web-platform-tests/wpt#23504?

I'm not sure, I was hoping someone else could check. I keep running into certificate errors for the service worker.

This makes things a bit more explicit and roughly aligns what was "escaped version" with implementations.

Corresponding URL Standard PR: whatwg/url#513.

Tests: web-platform-tests/wpt#23504.

Closes #3377.
@annevk annevk force-pushed the annevk/registerprotocolhandler-handler-handling branch from 403f78e to 97d67cb Compare May 12, 2020 10:37
@annevk annevk requested a review from domenic May 12, 2020 13:20
source Show resolved Hide resolved
source Show resolved Hide resolved
@domenic
Copy link
Member

domenic commented May 13, 2020

It's not clear to me that the "Leaking credentials" part is obsolete? There's nothing in the processing model that prevents substituting in credentialed URLs in the %s, is there? Or similarly, passing in intranet sites, or sites behind HTTP auth (e.g. the URLs of W3C member-only pages).

@annevk
Copy link
Member Author

annevk commented May 13, 2020

I restored both for now. They are actually useful to have as we evaluate new schemes to add to the safelist. And also for people minting web+ schemes.

@annevk annevk requested a review from domenic May 13, 2020 14:25
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but unclear whether we want to wait on tests/clear implementer interest. (I'll get back to the tests PR in a bit.)

@annevk
Copy link
Member Author

annevk commented May 13, 2020

#3377 (comment)

annevk added a commit to whatwg/url that referenced this pull request Jun 24, 2020
@fred-wang
Copy link
Contributor

See #3377 (comment) and #3377 (comment) for Chromium interest and tests.

@domenic
Copy link
Member

domenic commented Jul 29, 2020

I think we're solid on implementation interest and tests now! I'll wait for @annevk to get back from OOO so we can merge whatwg/url#513 and this PR together.

annevk added a commit to web-platform-tests/wpt that referenced this pull request Aug 4, 2020
annevk added a commit to whatwg/url that referenced this pull request Aug 4, 2020
annevk added a commit to web-platform-tests/wpt that referenced this pull request Aug 4, 2020
@annevk annevk removed the do not merge yet Pull request must not be merged per rationale in comment label Aug 4, 2020
@annevk annevk merged commit 230af41 into master Aug 4, 2020
@annevk annevk deleted the annevk/registerprotocolhandler-handler-handling branch August 4, 2020 09:25
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 8, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 15, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Aug 16, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504

UltraBlame original commit: 0fc4ea2bb5e112c615dc7796455c526ded9342d8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Aug 16, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504

UltraBlame original commit: 57aa12b4158d1813056006747e1590b43850b0c8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Aug 16, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504

UltraBlame original commit: 0fc4ea2bb5e112c615dc7796455c526ded9342d8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Aug 16, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504

UltraBlame original commit: 57aa12b4158d1813056006747e1590b43850b0c8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Aug 16, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504

UltraBlame original commit: 0fc4ea2bb5e112c615dc7796455c526ded9342d8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Aug 16, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504

UltraBlame original commit: 57aa12b4158d1813056006747e1590b43850b0c8
sidvishnoi pushed a commit to sidvishnoi/gecko-webmonetization that referenced this pull request Sep 23, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504
ambroff pushed a commit to ambroff/gecko that referenced this pull request Nov 4, 2020
…for percent encoding, a=testonly

Automatic update from web-platform-tests
registerProtocolHandler(): manual tests for percent encoding

For whatwg/html#5524.
--

wpt-commits: 084b7c4b34e54882ace03e3247ab60268e18b15d
wpt-pr: 23504
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

navigator.registerProtocolHandler specification divergent from both implementations
4 participants