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

HTML: set location.protocol to broken schemes #4412

Merged
merged 4 commits into from
Jan 5, 2017

Conversation

annevk
Copy link
Member

@annevk annevk commented Jan 3, 2017

See whatwg/url#61 for context.

@wpt-pr-bot
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot added the html label Jan 3, 2017
@annevk
Copy link
Member Author

annevk commented Jan 3, 2017

Note: this requires some changes to the URL Standard that the HTML Standard has forever anticipated. PR coming up for that.

annevk added a commit to whatwg/url that referenced this pull request Jan 3, 2017
This is necessary to define the behavior of the Location object’s
protocol attribute, which throws on syntax errors rather than silently
ignoring them as other protocol setters do.

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

This is the cleanup work mentioned in
whatwg/html@f0a7365
345689 that is happening way late.
annevk added a commit to whatwg/url that referenced this pull request Jan 3, 2017
This is necessary to define the behavior of the Location object’s
protocol attribute, which throws on syntax errors rather than silently
ignoring them as other protocol setters do.

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

This is the cleanup work mentioned in
whatwg/html@f0a7365
that is happening way late.
@annevk
Copy link
Member Author

annevk commented Jan 3, 2017

It might be worth testing these broken schemes inside a data URL <iframe> as well and maybe an <iframe srcdoc>. So please don't land straight away.

@annevk
Copy link
Member Author

annevk commented Jan 3, 2017

whatwg/url#194 is the change to the URL Standard by the way. This is ready to land now.

@wpt-stability-bot
Copy link

wpt-stability-bot commented Jan 3, 2017

Firefox

Testing revision 1765203
Starting 10 test iterations
All results were stable

All results

/html/browsers/history/the-location-interface/location-protocol-setter.html
Subtest Results
OK
X%0A (percent-encoded) is not a scheme FAIL
%20X (percent-encoded) is not a scheme FAIL
%C3%BFx (percent-encoded) is not a scheme FAIL
X%20 (percent-encoded) is not a scheme FAIL
%C3%A2%E2%82%AC%C2%A0x (percent-encoded) is not a scheme FAIL
x%01 (percent-encoded) is not a scheme FAIL
X%00 (percent-encoded) is not a scheme FAIL
%C2%80x (percent-encoded) is not a scheme FAIL
%20 (percent-encoded) is not a scheme FAIL
x%C3%A2%E2%82%AC%C2%A0 (percent-encoded) is not a scheme FAIL
!X (percent-encoded) is not a scheme FAIL
a+-.%0A (percent-encoded) is not a scheme FAIL
:x (percent-encoded) is not a scheme FAIL
X! (percent-encoded) is not a scheme FAIL
%C3%BF (percent-encoded) is not a scheme FAIL
x%0A (percent-encoded) is not a scheme FAIL
x! (percent-encoded) is not a scheme FAIL
%7F (percent-encoded) is not a scheme FAIL
%C3%BFX (percent-encoded) is not a scheme FAIL
%C2%80 (percent-encoded) is not a scheme FAIL
x%C2%80 (percent-encoded) is not a scheme FAIL
X%C3%A2%E2%82%AC%C2%A0 (percent-encoded) is not a scheme FAIL
%C3%A2%E2%82%AC%C2%A0X (percent-encoded) is not a scheme FAIL
X%01 (percent-encoded) is not a scheme FAIL
x%C3%BF (percent-encoded) is not a scheme FAIL
%0Ax (percent-encoded) is not a scheme FAIL
!x (percent-encoded) is not a scheme FAIL
%01x (percent-encoded) is not a scheme FAIL
: (percent-encoded) is not a scheme FAIL
%7FX (percent-encoded) is not a scheme FAIL
! (percent-encoded) is not a scheme FAIL
%20x (percent-encoded) is not a scheme FAIL
a%0A (percent-encoded) is not a scheme FAIL
%0AX (percent-encoded) is not a scheme FAIL
X%7F (percent-encoded) is not a scheme FAIL
%00x (percent-encoded) is not a scheme FAIL
x%00 (percent-encoded) is not a scheme FAIL
Equivalent tests for data URL and srcdoc <iframe>s FAIL
%00 (percent-encoded) is not a scheme FAIL
%0A (percent-encoded) is not a scheme FAIL
%01X (percent-encoded) is not a scheme FAIL
x%20 (percent-encoded) is not a scheme FAIL
%C3%A2%E2%82%AC%C2%A0 (percent-encoded) is not a scheme FAIL
%7Fx (percent-encoded) is not a scheme FAIL
X%C2%80 (percent-encoded) is not a scheme FAIL
%C2%80X (percent-encoded) is not a scheme FAIL
X%C3%BF (percent-encoded) is not a scheme FAIL
%01 (percent-encoded) is not a scheme FAIL
x%7F (percent-encoded) is not a scheme FAIL
%00X (percent-encoded) is not a scheme FAIL
:X (percent-encoded) is not a scheme FAIL

@wpt-stability-bot
Copy link

wpt-stability-bot commented Jan 3, 2017

Chrome

Testing revision 1765203
Starting 10 test iterations
All results were stable

All results

/html/browsers/history/the-location-interface/location-protocol-setter.html
Subtest Results
OK
X%0A (percent-encoded) is not a scheme PASS
%20X (percent-encoded) is not a scheme PASS
x%E2%80%A0 (percent-encoded) is not a scheme PASS
%C3%BFx (percent-encoded) is not a scheme PASS
X%20 (percent-encoded) is not a scheme PASS
x%01 (percent-encoded) is not a scheme PASS
X%00 (percent-encoded) is not a scheme PASS
%C2%80x (percent-encoded) is not a scheme PASS
%20 (percent-encoded) is not a scheme PASS
%E2%80%A0 (percent-encoded) is not a scheme PASS
!X (percent-encoded) is not a scheme PASS
a+-.%0A (percent-encoded) is not a scheme PASS
:x (percent-encoded) is not a scheme PASS
X! (percent-encoded) is not a scheme PASS
x%0A (percent-encoded) is not a scheme PASS
x! (percent-encoded) is not a scheme PASS
%7F (percent-encoded) is not a scheme PASS
%C3%BFX (percent-encoded) is not a scheme PASS
%C2%80 (percent-encoded) is not a scheme PASS
x%C2%80 (percent-encoded) is not a scheme PASS
X%01 (percent-encoded) is not a scheme PASS
x%C3%BF (percent-encoded) is not a scheme PASS
%0Ax (percent-encoded) is not a scheme PASS
!x (percent-encoded) is not a scheme PASS
%01x (percent-encoded) is not a scheme PASS
: (percent-encoded) is not a scheme PASS
%7FX (percent-encoded) is not a scheme PASS
! (percent-encoded) is not a scheme PASS
%20x (percent-encoded) is not a scheme PASS
a%0A (percent-encoded) is not a scheme PASS
%0AX (percent-encoded) is not a scheme PASS
%E2%80%A0X (percent-encoded) is not a scheme PASS
X%7F (percent-encoded) is not a scheme PASS
%00x (percent-encoded) is not a scheme PASS
X%E2%80%A0 (percent-encoded) is not a scheme PASS
x%00 (percent-encoded) is not a scheme PASS
Equivalent tests for data URL and srcdoc <iframe>s PASS
%00 (percent-encoded) is not a scheme PASS
%0A (percent-encoded) is not a scheme PASS
%01X (percent-encoded) is not a scheme PASS
x%20 (percent-encoded) is not a scheme PASS
%C3%BF (percent-encoded) is not a scheme PASS
%7Fx (percent-encoded) is not a scheme PASS
X%C2%80 (percent-encoded) is not a scheme PASS
%C2%80X (percent-encoded) is not a scheme PASS
X%C3%BF (percent-encoded) is not a scheme PASS
%01 (percent-encoded) is not a scheme PASS
%E2%80%A0x (percent-encoded) is not a scheme PASS
x%7F (percent-encoded) is not a scheme PASS
%00X (percent-encoded) is not a scheme PASS
:X (percent-encoded) is not a scheme PASS

@domenic
Copy link
Member

domenic commented Jan 3, 2017

These all fail in Chrome and Firefox because of the exception type, right? I think we should change it to SyntaxError DOMException in the tests and spec before merging; no need to churn browsers over this detail.

@annevk
Copy link
Member Author

annevk commented Jan 4, 2017

Well Firefox needs a change anyway since it's not "SyntaxError" there but rather some internal thing. I guess we can decide to align with Chrome, but it seems a little weird since we use TypeError normally for URL parsing issues.

@domenic
Copy link
Member

domenic commented Jan 4, 2017

Hmm. Edge also has a weird Error instead of any normal error type. Safari matches Chrome though. I guess I do not feel strongly, but I do note the following places that use "SyntaxError" DOMExceptions for URL parsing failures:

  • location.assign/location.replace
  • registerProtocolHandler/registerContentHandler
  • EventSource constructor
  • WebSocket constructor
  • window.postMessage origin parser
  • Worker/SharedWorker constructor
  • importScripts

@annevk
Copy link
Member Author

annevk commented Jan 4, 2017

Okay, I will give up, but not today, dinner time!

annevk added a commit to whatwg/html that referenced this pull request Jan 5, 2017
This matches Chrome and Safari. Edge and Firefox use an internal error
that needs to change either way.

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

Additional context: whatwg/url#61.
@annevk
Copy link
Member Author

annevk commented Jan 5, 2017

This is now ready to land.

domenic pushed a commit to whatwg/html that referenced this pull request Jan 5, 2017
This matches Blink and WebKit. Edge and Gecko use an internal error
that needs to change either way.

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

Additional context: whatwg/url#61.
@domenic domenic merged commit 6ab8b20 into master Jan 5, 2017
@domenic domenic deleted the annevk/location-protocol-broken branch January 5, 2017 14:41
annevk added a commit to whatwg/url that referenced this pull request Jan 5, 2017
This is necessary to define the behavior of the Location object’s
protocol attribute, which throws on syntax errors rather than silently
ignoring them as other protocol setters do.

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

This is the cleanup work mentioned in
whatwg/html@f0a7365
that is happening way late.
annevk added a commit that referenced this pull request Jan 5, 2017
alice pushed a commit to alice/html that referenced this pull request Jan 8, 2019
This matches Blink and WebKit. Edge and Gecko use an internal error
that needs to change either way.

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

Additional context: whatwg/url#61.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants