Fix document URL setting for javascript: URLs #4698
Merged
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
annevk
reviewed
Jun 28, 2019
This concept was deleted in 6440cca along with "overridden overload", but it was discovered in whatwg#3989 that correct handling for javascript: URLs still depend on this concept. The spec around this area has changed quite a bit since then due to intermediate cleanups such as e13762f, so this is not a simple revert. Also, fix ungrammatical "append URL URL list" that was accidentally introduced in e13762f. Addresses some concern raised in whatwg#3989.
This reverts commit e91668d.
A series of changes starting with 6440cca which removed the override URL concept as well as intermediate cleanups such as e13762f eroded the correctness of javascript: document URLs. Now it seems to be the case that fixing this typo restores the correct handling for such URLs. Addresses some concern raised in whatwg#3989.
|
I've changed the PR to not reintroduce override URL as @annevk discovered once we fix the URL list appending the handling seems correct. @bzbarsky could you please double-check if this is good? |
TimothyGu
changed the title
bzbarsky
approved these changes
Jul 5, 2019
6 tasks
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.
A series of changes starting with 6440cca which removed the override URL concept as well as intermediate cleanups such as e13762f eroded the correctness of javascript: document URLs. Now it seems to be the case that fixing this typo restores the correct handling for such URLs.
Addresses some concern raised in #3989.
Original outdated description
This concept was deleted in 6440cca along with "overridden overload", but it was discovered in #3989 that correct handling for
javascript:URLs still depend on this concept. The spec around this area has changed quite a bit since then due to intermediate cleanups such as e13762f, so this is not a simple revert.Also, fix ungrammatical "append URL URL list" that was accidentally introduced in e13762f.
Addresses some concern raised in #3989.
It would be nice if we can have some tests for this, though this is arguably just fixing a spec bug.
/cc @bzbarsky
/browsing-the-web.html ( diff )