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

document.open(): remove fragment before propagating entry document's URL #3970

Merged
merged 1 commit into from
Aug 29, 2018

Conversation

TimothyGu
Copy link
Member

@TimothyGu TimothyGu commented Aug 28, 2018

This is another part of the effort to overhaul document.open() as outlined in #3818.

Tests: web-platform-tests/wpt#10817

Fixes #2555.

TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Aug 28, 2018
TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Aug 28, 2018
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

As discussed offline, we better align with Safari and Chrome if we guard the URL and history update states behind "If entry is not document" as well. So, we should do that.

Testing the difference between that and the current PR will require writing a test to show that in such cases POSTs don't get converted to GET. I'm sorry :(

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

OK, as discussed further offline, my argument about delta from today's implementations was not sensible. All implementations will have to make small changes in this area, either with this PR's current version or with my suggested tweak to it. And those changes are of similar magnitude in both cases.

I still think it might be a bit nicer to have one if statement governing all this "weird URL-related stuff". But @TimothyGu is the expert, so I will trust his judgment that this is the best path.

@annevk annevk merged commit ae688ea into whatwg:master Aug 29, 2018
@annevk
Copy link
Member

annevk commented Aug 29, 2018

Indeed, as with the other document.open() PRs the intention is to file bugs and align implementations once the revised standard is "done". That still seems likely to happen.

annevk added a commit to web-platform-tests/wpt that referenced this pull request Aug 29, 2018
@TimothyGu TimothyGu deleted the document-open-fragment branch August 29, 2018 12:30
@TimothyGu TimothyGu self-assigned this Aug 29, 2018
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Sep 5, 2018
… (with fragments), a=testonly

Automatic update from web-platform-testsHTML: document.open() and document's URL (with fragments)

For whatwg/html#3970.

Co-authored-by: Timothy Gu <[email protected]>
--

wpt-commits: d2cffc6c5ec316e0269cf2809cba090153ea29ba
wpt-pr: 10817
jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Sep 5, 2018
… (with fragments), a=testonly

Automatic update from web-platform-testsHTML: document.open() and document's URL (with fragments)

For whatwg/html#3970.

Co-authored-by: Timothy Gu <[email protected]>
--

wpt-commits: d2cffc6c5ec316e0269cf2809cba090153ea29ba
wpt-pr: 10817
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
… (with fragments), a=testonly

Automatic update from web-platform-testsHTML: document.open() and document's URL (with fragments)

For whatwg/html#3970.

Co-authored-by: Timothy Gu <timothygu99gmail.com>
--

wpt-commits: d2cffc6c5ec316e0269cf2809cba090153ea29ba
wpt-pr: 10817

UltraBlame original commit: 58e1e3b29ba385cc708b828728ec973bb878efac
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
… (with fragments), a=testonly

Automatic update from web-platform-testsHTML: document.open() and document's URL (with fragments)

For whatwg/html#3970.

Co-authored-by: Timothy Gu <timothygu99gmail.com>
--

wpt-commits: d2cffc6c5ec316e0269cf2809cba090153ea29ba
wpt-pr: 10817

UltraBlame original commit: 58e1e3b29ba385cc708b828728ec973bb878efac
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
… (with fragments), a=testonly

Automatic update from web-platform-testsHTML: document.open() and document's URL (with fragments)

For whatwg/html#3970.

Co-authored-by: Timothy Gu <timothygu99gmail.com>
--

wpt-commits: d2cffc6c5ec316e0269cf2809cba090153ea29ba
wpt-pr: 10817

UltraBlame original commit: 58e1e3b29ba385cc708b828728ec973bb878efac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants