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
[Gecko Bug 1489308] part 5. Align the work we do on document.open with the spec. #15641
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
The main behavior changes are: 1) We no longer create a new Window when doing document.open(). We use the same Window but remove all the event listeners on it and on the existing DOM tree before removing the document's existing kids. 2) We no longer create a new session history entry. The existing one always gets replaced instead. 3) We now support document.open on documents that are not in a Window. The reasons for the various test changes are as follows: The change to browser_modifiedclick_inherit_principal.js is because we no longer set the docshell to a wyciwyg URL when document.open() happens and the test was depending on that to terminate. browser_wyciwyg_urlbarCopying.js is being removed because it's trying to test wyciwyg URIs, which no longer exist. The changes in docshell/test/navigation are because document.open() no longer affects session history. One of the tests was testing the interactions there and is being removed; another is being repurposed to just test that document.open() does not affect history.length. The change to test_x-frame-options.html is because document.open() now removes event listeners on the window, which it didn't use to do (and in the specific case in this test reused the existing inner too, so the listener was still around in practice). The new behavior matches other browsers. The removal of test_bug172261.html is because document.open() no longer affects session history, so you can't go back across it or forward to the "opened" state, so the situation that test is trying to test no longer exists. The changes to test_bug255820.html are because reloading a document after document.open() will now just load the URL of the document that was the entry document for the open() call, not reload the written content. So there's not much point testing reload behavior, and in this test it was just reloading the toplevel test file inside the frames. The change to test_bug346659.html is because now we no longer create a new Window on document.open(). The change to test_bug1232829.html is because document.open() (implicit in this test) no longer adds history entries, so the back() was just leaving the test page instead of going back across the document.open(). The test is a crashtest in practice, so might still be testing something useful about how document.open() interacts with animations. The change to test_bug715739.html is because the URL of the document after document.open() is now the URL of the entry document, not a wyciwyg URL, so reload() has different behavior than it used to. The change to test_bug329869.html is because now when we go back we're reloading the original document we had, not doing a wyciwyg load, and the security info now doesn't include the untrusted script. The changes to the wpt expectations are removing a bunch of expected failures now that we pass those tests and disabling some tests that are fundamentally racy and hence fail randomly. The latter all have github issues filed for the test problem. The change to testing/web-platform/tests/common/object-association.js is fixing tests that were not matching the spec (and were failing in other browsers). The change to parser-uses-registry-of-owner-document.html is fixing tests that were not matching the spec (and were failing in other browsers). The change to document-write.tentative.html is because the test was buggy: it was using the same iframe element for all its tests and racing loads from some tests against API calls from other tests, etc. It's a wonder it ever managed to pass, independent of these patches (and in fact it doesn't pass according to wpt.fyi data, even in Firefox). The changes in html/browsers/history/the-history-interface are because document.open() no longer adds history entries. The test was failing in all other browsers for the same reason. The changes in html/browsers/history/the-location-interface are because reloading a document.open()-created thing now loads the URL of the page that was the entry document for the open() call. The test was failing in all other browsers. The change to reload_document_open_write.html is because we now reload the url of the document that entered the script that called open() when we reload, not the written content. Other browsers were failing this test too; Gecko with the old document.open implementation was the only one that passed. The change to http-refresh.py is to fix a test bug: it was not returning a Content-Type header, so we were putting up helper app dialogs, etc. The change to test_ext_contentscript.js is because we no create a new global for document.open() calls. Kris Maglione OKed this part. Differential Revision: https://phabricator.services.mozilla.com/D17323 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1489308 gecko-commit: 6504b5468b32786ec2f492e0dc6cd2ed3f1cf55d gecko-integration-branch: central gecko-reviewers: mccr8, smaug
wpt-pr-bot
approved these changes
Mar 3, 2019
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already reviewed downstream.
This is unstable in Firefox in the CI, but merging anyway. The failure is Unstable results
@bzbarsky is that expected? |
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.
The main behavior changes are:
We no longer create a new Window when doing document.open(). We use the
same Window but remove all the event listeners on it and on the existing DOM
tree before removing the document's existing kids.
We no longer create a new session history entry. The existing one always
gets replaced instead.
We now support document.open on documents that are not in a Window.
The reasons for the various test changes are as follows:
The change to browser_modifiedclick_inherit_principal.js is because we no
longer set the docshell to a wyciwyg URL when document.open() happens and the
test was depending on that to terminate.
browser_wyciwyg_urlbarCopying.js is being removed because it's trying to test
wyciwyg URIs, which no longer exist.
The changes in docshell/test/navigation are because document.open() no longer
affects session history. One of the tests was testing the interactions there
and is being removed; another is being repurposed to just test that
document.open() does not affect history.length.
The change to test_x-frame-options.html is because document.open() now removes
event listeners on the window, which it didn't use to do (and in the specific
case in this test reused the existing inner too, so the listener was still
around in practice). The new behavior matches other browsers.
The removal of test_bug172261.html is because document.open() no longer affects
session history, so you can't go back across it or forward to the "opened"
state, so the situation that test is trying to test no longer exists.
The changes to test_bug255820.html are because reloading a document after
document.open() will now just load the URL of the document that was the entry
document for the open() call, not reload the written content. So there's not
much point testing reload behavior, and in this test it was just reloading the
toplevel test file inside the frames.
The change to test_bug346659.html is because now we no longer create a new
Window on document.open().
The change to test_bug1232829.html is because document.open() (implicit in this
test) no longer adds history entries, so the back() was just leaving the test
page instead of going back across the document.open(). The test is a
crashtest in practice, so might still be testing something useful about how
document.open() interacts with animations.
The change to test_bug715739.html is because the URL of the document after
document.open() is now the URL of the entry document, not a wyciwyg URL, so
reload() has different behavior than it used to.
The change to test_bug329869.html is because now when we go back we're
reloading the original document we had, not doing a wyciwyg load, and the
security info now doesn't include the untrusted script.
The changes to the wpt expectations are removing a bunch of expected failures
now that we pass those tests and disabling some tests that are fundamentally
racy and hence fail randomly. The latter all have github issues filed for the
test problem.
The change to testing/web-platform/tests/common/object-association.js is fixing
tests that were not matching the spec (and were failing in other browsers).
The change to parser-uses-registry-of-owner-document.html is fixing tests that
were not matching the spec (and were failing in other browsers).
The change to document-write.tentative.html is because the test was buggy: it
was using the same iframe element for all its tests and racing loads from some
tests against API calls from other tests, etc. It's a wonder it ever managed
to pass, independent of these patches (and in fact it doesn't pass according to
wpt.fyi data, even in Firefox).
The changes in html/browsers/history/the-history-interface are because
document.open() no longer adds history entries. The test was failing in all
other browsers for the same reason.
The changes in html/browsers/history/the-location-interface are because
reloading a document.open()-created thing now loads the URL of the page that
was the entry document for the open() call. The test was failing in all other
browsers.
The change to reload_document_open_write.html is because we now reload the url
of the document that entered the script that called open() when we reload, not
the written content. Other browsers were failing this test too; Gecko with
the old document.open implementation was the only one that passed.
The change to http-refresh.py is to fix a test bug: it was not returning a
Content-Type header, so we were putting up helper app dialogs, etc.
The change to test_ext_contentscript.js is because we no create a new global
for document.open() calls. Kris Maglione OKed this part.
Differential Revision: https://phabricator.services.mozilla.com/D17323
bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1489308
gecko-commit: 6504b5468b32786ec2f492e0dc6cd2ed3f1cf55d
gecko-integration-branch: central
gecko-reviewers: mccr8, smaug