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

URL/Encoding: change query state parsing #10915

Merged
merged 1 commit into from
May 23, 2018
Merged

Conversation

annevk
Copy link
Member

@annevk annevk commented May 9, 2018

See whatwg/encoding#139 for rationale and whatwg/url#386 for the change to the URL Standard.

(I found all these resources in part due to @rakuco's work on trying to align Chrome with the earlier iteration of the specification.)

@annevk
Copy link
Member Author

annevk commented May 9, 2018

Note: I looked into removing normalizeStr completely, but that broke too many tests. So I only removed it where it was a no-op or caused only one browser (Firefox) to start failing tests.

Perhaps further cleanup here can be done as part of #10636.

@annevk
Copy link
Member Author

annevk commented May 9, 2018

@annevk
Copy link
Member Author

annevk commented May 9, 2018

This needs to change #10891 as well. I haven't corrected it there to keep the state of the test suite consistent.

@annevk annevk force-pushed the annevk/query-state-encoding branch from 39db988 to 361b63f Compare May 9, 2018 14:39
@annevk
Copy link
Member Author

annevk commented May 9, 2018

That's now done. This as well the URL Standard change are ready for review.

@annevk
Copy link
Member Author

annevk commented May 9, 2018

Note to self: once this is reviewed and landed, do some work on #4934 again.

Copy link
Member

@inexorabletash inexorabletash left a comment

Choose a reason for hiding this comment

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

lgtm

@annevk
Copy link
Member Author

annevk commented May 10, 2018

(I merged the commits together so that "rebase and merge" can be used by an owner. This needs an override due to build timeouts. However, please do not merge this before I remove that label.)

@annevk
Copy link
Member Author

annevk commented May 10, 2018

Also, thanks @inexorabletash for the speedy review!

annevk added a commit to whatwg/url that referenced this pull request May 22, 2018
If the input to the URL parser contains code points outside the non-UTF-8 encoding's value space and the URL parser was invoked using a non-UTF-8 encoding, then those code points end up as &#...;.

The problem is that &, #, and ; are also URL separators, but the previous algorithm would only encode #. This ensures that & and ; are also encoded, as some browsers already do, but only if they came about as the result of the encode operation.

Tests: web-platform-tests/wpt#10915.
@annevk annevk force-pushed the annevk/query-state-encoding branch from bd7ea53 to d5b8565 Compare May 22, 2018 09:32
@annevk
Copy link
Member Author

annevk commented May 22, 2018

I rebased this to address a merge conflict in url/README.md.

annevk added a commit to whatwg/url that referenced this pull request May 23, 2018
If the input to the URL parser contains code points outside the non-UTF-8 encoding's value space and the URL parser was invoked using a non-UTF-8 encoding, then those code points end up as &#...;.

The problem is that &, #, and ; are also URL separators, but the previous algorithm would only encode #. This ensures that & and ; are also encoded, as some browsers already do, but only if they came about as the result of the encode operation.

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

Fixes whatwg/encoding#139.
annevk added a commit to whatwg/url that referenced this pull request May 23, 2018
If the input to the URL parser contains code points outside the non-UTF-8 encoding's value space and the URL parser was invoked using a non-UTF-8 encoding, then those code points end up as &#...;.

The problem is that &, #, and ; are also URL separators, but the previous algorithm would only encode #. This ensures that & and ; are also encoded, as some browsers already do, but only if they came about as the result of the encode operation.

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

Fixes whatwg/encoding#139.
See whatwg/encoding#139 for rationale and whatwg/url#386 for the change to the URL Standard.

(I found all these resources in part due to @rakuco's work on trying to align Chrome with the earlier iteration of the specification.)
@jgraham jgraham merged commit e399a2c into master May 23, 2018
@annevk annevk deleted the annevk/query-state-encoding branch May 23, 2018 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants