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.

Sign up for GitHub

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

Jump to bottom

protocol setter should not strip tabs and newlines #620

Closed
achristensen07 opened this issue Jul 8, 2021 · 1 comment
Closed

protocol setter should not strip tabs and newlines #620

achristensen07 opened this issue Jul 8, 2021 · 1 comment

Comments

@achristensen07
Copy link
Collaborator

The tests that verify that tabs and newlines are stripped in url-setters-stripping.any.html are all failed by all browsers. In order to make them pass I would have to break html/browsers/history/the-location-interface/location-protocol-setter.html which is passed by all browsers, and the location specification at https://html.spec.whatwg.org/multipage/history.html#dom-location-protocol just refers to the URL specification. I think at least the URL specification should change, and probably HTML that refers to it. I propose adding a step that checks to see if there are any tabs or newlines in the value, then returning if there are and doing nothing. That would match all browsers.
@TimothyGu
Copy link
Member

This is a duplicate of #609. We were wondering what you think, but I guess that's now clear ;)

webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Jul 8, 2021
@webkit-commit-queue
Fix some whitespace handling issues in URL setters
05c2dcb 
https://bugs.webkit.org/show_bug.cgi?id=227806

Patch by Alex Christensen <achristensen@webkit.org> on 2021-07-08
Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

* web-platform-tests/url/a-element-expected.txt:
* web-platform-tests/url/a-element-xhtml-expected.txt:
* web-platform-tests/url/url-setters-stripping.any-expected.txt:
* web-platform-tests/url/url-setters-stripping.any.worker-expected.txt:

Source/WebCore:

Covered by newly passing wpt tests.

* dom/Element.cpp:
(WebCore::Element::getURLAttribute const):
* html/HTMLAnchorElement.cpp:
(WebCore::HTMLAnchorElement::href const):
Don't remove whitespace before giving to completeURL, which will do that for us if it's a valid URL.
If it's not a valid URL, we want the original string, not the trimmed string.
* html/URLDecomposition.cpp:
(WebCore::parsePort):
Parse ports more like the URLParser, which ignores tabs and newlines.

Source/WTF:

Setters should ignore tabs and newlines like the main parser does.
The protocol setter is problematic, which I reported in whatwg/url#620

* wtf/URL.cpp:
(WTF::URL::setFragmentIdentifier):
* wtf/URLParser.cpp:
(WTF::URLParser::isSpecialScheme):
(WTF::URLParser::parse):
* wtf/URLParser.h:
The URL.hash setter should allow trailing C0 and control characters, which we would otherwise trim.
Rather than introduce a new parameter, use a sentinel value for when we need to do this.

LayoutTests:

Update some old tests that failed in Chrome and Firefox to pass in all browsers after this change.

* fast/dom/DOMURL/set-href-attribute-port-expected.txt:
* fast/dom/DOMURL/set-href-attribute-port.html:
* fast/dom/HTMLAnchorElement/set-href-attribute-port-expected.txt:
* fast/dom/HTMLAnchorElement/set-href-attribute-port.html:

Canonical link: https://commits.webkit.org/239531@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@279760 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bertogg pushed a commit to Igalia/webkit that referenced this issue Jul 9, 2021
Fix some whitespace handling issues in URL setters
2fcd1ad 
https://bugs.webkit.org/show_bug.cgi?id=227806

Patch by Alex Christensen <achristensen@webkit.org> on 2021-07-08
Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

* web-platform-tests/url/a-element-expected.txt:
* web-platform-tests/url/a-element-xhtml-expected.txt:
* web-platform-tests/url/url-setters-stripping.any-expected.txt:
* web-platform-tests/url/url-setters-stripping.any.worker-expected.txt:

Source/WebCore:

Covered by newly passing wpt tests.

* dom/Element.cpp:
(WebCore::Element::getURLAttribute const):
* html/HTMLAnchorElement.cpp:
(WebCore::HTMLAnchorElement::href const):
Don't remove whitespace before giving to completeURL, which will do that for us if it's a valid URL.
If it's not a valid URL, we want the original string, not the trimmed string.
* html/URLDecomposition.cpp:
(WebCore::parsePort):
Parse ports more like the URLParser, which ignores tabs and newlines.

Source/WTF:

Setters should ignore tabs and newlines like the main parser does.
The protocol setter is problematic, which I reported in whatwg/url#620

* wtf/URL.cpp:
(WTF::URL::setFragmentIdentifier):
* wtf/URLParser.cpp:
(WTF::URLParser::isSpecialScheme):
(WTF::URLParser::parse):
* wtf/URLParser.h:
The URL.hash setter should allow trailing C0 and control characters, which we would otherwise trim.
Rather than introduce a new parameter, use a sentinel value for when we need to do this.

LayoutTests:

Update some old tests that failed in Chrome and Firefox to pass in all browsers after this change.

* fast/dom/DOMURL/set-href-attribute-port-expected.txt:
* fast/dom/DOMURL/set-href-attribute-port.html:
* fast/dom/HTMLAnchorElement/set-href-attribute-port-expected.txt:
* fast/dom/HTMLAnchorElement/set-href-attribute-port.html:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@279760 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@achristensen07 @TimothyGu