Repeated slashes should be ignored in file scheme #234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
watilde
force-pushed
the
feature/check-file-slashes
branch
from
b587151
to
ed2a94a
Compare
|
Are you planning on writing tests as well? There's some indentation issues with that patch, but it looks reasonable overall. Did you verify it with Node.js's implementation? (I think you mean to write "ignored" rather than "arranged".) |
watilde
added a commit
to watilde/web-platform-tests
that referenced
this pull request
Feb 8, 2017
Related links + The URL Standard change: whatwg/url#234 + The original bug report: whatwg/url#232
watilde
changed the title
watilde
added a commit
to watilde/web-platform-tests
that referenced
this pull request
Feb 8, 2017
Related links + The URL Standard change: whatwg/url#234 + The original bug report: whatwg/url#232
|
@annevk Yeah I've verified on the branch I posted in the issue report, and it worked. I just created PR for WPT now, then I will fix the indentation issues and update the commit message with the PR link. |
domenic
reviewed
Feb 8, 2017
url.bs
Outdated
| </ol> | ||
| <li> | ||
| <p>Otherwise, set <var>state</var> | ||
| to <a>authority state</a>, and decrease <var>pointer</var> by one.</p> |
There was a problem hiding this comment.
Running the jsdom/whatwg-url tests, this pointer decrease needs to happen even if the scheme is file.
There was a problem hiding this comment.
Ahh that's right! I will update it.
Repeated slashes should be ignored in not only the http scheme, but also the file scheme as well. Tests: web-platform-tests/wpt#4762 Fixes: whatwg#232
watilde
force-pushed
the
feature/check-file-slashes
branch
from
ed2a94a
to
f320178
Compare
watilde
added a commit
to watilde/web-platform-tests
that referenced
this pull request
Feb 8, 2017
Related links + The URL Standard change: whatwg/url#234 + The original bug report: whatwg/url#232
domenic
reviewed
Feb 8, 2017
url.bs
Outdated
| </li> | ||
| <li> | ||
| <p>Set <var>state</var> to <a>file state</a>, | ||
| and decrease <var>pointer</var> by one.</p> |
There was a problem hiding this comment.
You can factor out the pointer decrease. Let me push a commit to show how.
watilde
added a commit
to watilde/web-platform-tests
that referenced
this pull request
Feb 8, 2017
Related links + The URL Standard change: whatwg/url#234 + The original bug report: whatwg/url#232
watilde
added a commit
to watilde/web-platform-tests
that referenced
this pull request
Feb 8, 2017
Related links + The URL Standard change: whatwg/url#234 + The original bug report: whatwg/url#232
watilde
added a commit
to watilde/web-platform-tests
that referenced
this pull request
Feb 8, 2017
Related links + The URL Standard change: whatwg/url#234 + The original bug report: whatwg/url#232
annevk
added
needs tests
annevk
added a commit
that referenced
this pull request
Mar 22, 2017
Tests: web-platform-tests/wpt#5195. Closes #234 by superseding it. Fixes #232.
annevk
added a commit
that referenced
this pull request
Mar 28, 2017
Also copy file URL hosts correctly for path-absolute input. Tests: web-platform-tests/wpt#5195. Closes #234 by superseding it. Fixes #232.
annevk
added a commit
that referenced
this pull request
Mar 29, 2017
Also copy file URL hosts correctly for path-absolute input. Tests: web-platform-tests/wpt#5195. Closes #234 by superseding it. Fixes #232.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #232.
Preview | Diff