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() simplifications: realm creation, unloading, tasks removal #3918

Merged
merged 4 commits into from Aug 16, 2018

Conversation

3 participants
@TimothyGu
Copy link
Member

commented Aug 14, 2018

Based on the work by Anne van Kesteren in #3651, but without the parts concerning the session history. Changes to that area will come later (see #3818).

Summary of implementation changes:

  • Chrome and Safari are already fully aligned with this change.
  • Edge and Firefox both create a new JavaScript realm. They should not do that after this PR.
  • Edge and Firefox removes some tasks but not others (see #3665). They should now remove no tasks after this PR.
  • Firefox fires beforeunload event in document.open() (but not pagehide or unload). It should no longer fire it after this PR.

Tests: web-platform-tests/wpt#10773
Tests: web-platform-tests/wpt#10778
Tests: web-platform-tests/wpt#10815
Tests: web-platform-tests/wpt#10818

Fixes #1698.
Fixes #3286.
Fixes #3306.
Closes #3665.


/cc @bzbarsky @daniec @foolip @travisleithead


/browsers.html ( diff )
/browsing-the-web.html ( diff )
/dynamic-markup-insertion.html ( diff )
/history.html ( diff )
/window-object.html ( diff )

Remove document.open() realm creation
Based on the work by Anne van Kesteren in #3651, but without the parts
concerning the session history. Changes to that area will come later
(see #3818).

Tests: web-platform-tests/wpt#10773
Tests: web-platform-tests/wpt#10778
Tests: web-platform-tests/wpt#10815
Tests: web-platform-tests/wpt#10818

Fixes #1698.
Fixes #3286.
Fixes #3306.
Closes #3665.

Co-authored-by: Anne van Kesteren <annevk@annevk.nl>
@domenic
Copy link
Member

left a comment

We should update the commit message to reflect that this removes a few separate pieces:

  • Realm creation
  • Task removal
  • Unloading

As discussed in person, there's also some other stuff to remove that references this infrastructure.

I think we'll also want a follow up bug (one may already exist?) on specifically simplifying some stuff now that there's no longer a potential 1:many Document:Window relationship, but I'm forgetting the details of how that worked right now... Will try to recall.

@@ -76876,14 +76876,11 @@ dictionary <dfn>DragEventInit</dfn> : <span>MouseEventInit</span> {

<p class="note">In general, there is a 1-to-1 mapping from the <code>Window</code> object to the
<code>Document</code> object, as long as the <code>Document</code> object has a <span
data-x="concept-document-bc">browsing context</span>. There are two exceptions. First, a
data-x="concept-document-bc">browsing context</span>. There is one exceptions. A

This comment has been minimized.

Copy link
@domenic
@domenic

This comment has been minimized.

Copy link
Member

commented Aug 14, 2018

I think we'll also want a follow up bug (one may already exist?) on specifically simplifying some stuff now that there's no longer a potential 1:many Document:Window relationship, but I'm forgetting the details of how that worked right now... Will try to recall.

It looks like the main thing is there's a lot of "document's Window object", which is an underdefined term before this PR. After this PR it can simply become "document's relevant global object".

There's also a lot of "document's Window object's environment settings object" which should just become "document's relevant settings object".

My method for finding these is to use Ctrl+F with the string 's Window on the page.

@annevk

annevk approved these changes Aug 15, 2018

Copy link
Member

left a comment

Looks good editorially. I think I also sufficiently convinced that @bzbarsky that we should do this, but more tests (and revisions) for the entire document.open() algorithm will be needed for Firefox to take the jump.

TimothyGu added some commits Aug 15, 2018

@domenic
Copy link
Member

left a comment

LGTM, let's merge!

Another test to update is https://github.com/web-platform-tests/wpt/blob/34b1111aaf51b0143edcc5e3b454cff680edcd6f/common/object-association.js#L44-L65. We may want to actually flip the condition instead of deleting it.

@annevk

This comment has been minimized.

Copy link
Member

commented Aug 16, 2018

(Really enjoying these additional cleanup commits. Lots of simplifications in complex algorithms.)

@TimothyGu TimothyGu referenced this pull request Aug 16, 2018

Closed

Tracking issue for document.open() overhaul #3818

19 of 19 tasks complete

@TimothyGu TimothyGu changed the title Remove document.open() realm creation Remove document.open() realm creation, unloading, and tasks removal Aug 16, 2018

@TimothyGu TimothyGu changed the title Remove document.open() realm creation, unloading, and tasks removal document.open() simplifications: realm creation, unloading, tasks removal Aug 16, 2018

@TimothyGu

This comment has been minimized.

Copy link
Member Author

commented Aug 16, 2018

I've updated the pull request title and I believe this should be ready to merge! I will continue working on the WPT PRs to ensure they are merged.

On the bugs side, I plan on filing a mega-bug for both Edge and Firefox containing this change and some future PRs. As such I don't plan to file a bug for this PR specifically. As states before this PR does not require bugs on Chrome or Safari.

@domenic domenic merged commit 6f769b8 into whatwg:master Aug 16, 2018

2 checks passed

Participation TimothyGu participates on behalf of Google LLC
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Aug 17, 2018

@TimothyGu TimothyGu deleted the TimothyGu:document-open-realms branch Aug 17, 2018

TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Aug 17, 2018

TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Aug 17, 2018

HTML: document.open() and global variables (#10815)
For whatwg/html#3918.

Several tests were removed as they no longer apply after the change in HTML. The expectations were reversed and moved to the new no-new-global.window.js file.

Co-authored-by: Timothy Gu <timothygu99@gmail.com>

TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Aug 20, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 23, 2018

Bug 1458554 [wpt PR 10773] - HTML: document.open() and the unload eve…
…nt, a=testonly

Automatic update from web-platform-testsHTML: document.open() and the unload events (#10773)

For whatwg/html#3918.
--

wpt-commits: 23c21d2e0e6a8035ddfab91e33c54103d859ffca
wpt-pr: 10773

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 23, 2018

Bug 1458569 [wpt PR 10778] - HTML: document.open() and the beforeunlo…
…ad event, a=testonly

Automatic update from web-platform-testsHTML: document.open() and the beforeunload event (#10778)

For whatwg/html#3918.
--

wpt-commits: eafe4bdb65ced7352e953ec35c06c3afa3d8125b
wpt-pr: 10778

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 23, 2018

Bug 1458813 [wpt PR 10815] - HTML: document.open() and global variabl…
…es, a=testonly

Automatic update from web-platform-testsHTML: document.open() and global variables (#10815)

For whatwg/html#3918.

Several tests were removed as they no longer apply after the change in HTML. The expectations were reversed and moved to the new no-new-global.window.js file.

Co-authored-by: Timothy Gu <timothygu99@gmail.com>
--

wpt-commits: 70c15a6e508f7460e1584bc8b122ac2cc8f9d6ea
wpt-pr: 10815

jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Aug 23, 2018

Bug 1458554 [wpt PR 10773] - HTML: document.open() and the unload eve…
…nt, a=testonly

Automatic update from web-platform-testsHTML: document.open() and the unload events (#10773)

For whatwg/html#3918.
--

wpt-commits: 23c21d2e0e6a8035ddfab91e33c54103d859ffca
wpt-pr: 10773

jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Aug 23, 2018

Bug 1458569 [wpt PR 10778] - HTML: document.open() and the beforeunlo…
…ad event, a=testonly

Automatic update from web-platform-testsHTML: document.open() and the beforeunload event (#10778)

For whatwg/html#3918.
--

wpt-commits: eafe4bdb65ced7352e953ec35c06c3afa3d8125b
wpt-pr: 10778

jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Aug 23, 2018

Bug 1458813 [wpt PR 10815] - HTML: document.open() and global variabl…
…es, a=testonly

Automatic update from web-platform-testsHTML: document.open() and global variables (#10815)

For whatwg/html#3918.

Several tests were removed as they no longer apply after the change in HTML. The expectations were reversed and moved to the new no-new-global.window.js file.

Co-authored-by: Timothy Gu <timothygu99@gmail.com>
--

wpt-commits: 70c15a6e508f7460e1584bc8b122ac2cc8f9d6ea
wpt-pr: 10815

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Aug 27, 2018

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Aug 27, 2018

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Aug 27, 2018

HTML: document.open() and global variables (#10815)
For whatwg/html#3918.

Several tests were removed as they no longer apply after the change in HTML. The expectations were reversed and moved to the new no-new-global.window.js file.

Co-authored-by: Timothy Gu <timothygu99@gmail.com>

zcorpan added a commit to web-platform-tests/wpt that referenced this pull request Aug 27, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 29, 2018

Bug 1458849 [wpt PR 10818] - HTML: document.open() and tasks, a=testonly
Automatic update from web-platform-testsHTML: document.open() and tasks (#10818)

For whatwg/html#3918.
--

wpt-commits: 87329a1500b1aea8f9cf28b40afb08491e25a5c7
wpt-pr: 10818

jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Aug 29, 2018

Bug 1458849 [wpt PR 10818] - HTML: document.open() and tasks, a=testonly
Automatic update from web-platform-testsHTML: document.open() and tasks (#10818)

For whatwg/html#3918.
--

wpt-commits: 87329a1500b1aea8f9cf28b40afb08491e25a5c7
wpt-pr: 10818

TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Sep 6, 2018

TimothyGu added a commit to web-platform-tests/wpt that referenced this pull request Sep 6, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Sep 12, 2018

Bug 1489299 [wpt PR 12882] - HTML: Change document.open() test expect…
…ation, a=testonly

Automatic update from web-platform-testsHTML: Change document.open() test expectation (#12882)

For whatwg/html#3918.
--

wpt-commits: b70ac6309528aacacfaec186478d0b30b018b8f9
wpt-pr: 12882

jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Sep 12, 2018

Bug 1489299 [wpt PR 12882] - HTML: Change document.open() test expect…
…ation, a=testonly

Automatic update from web-platform-testsHTML: Change document.open() test expectation (#12882)

For whatwg/html#3918.
--

wpt-commits: b70ac6309528aacacfaec186478d0b30b018b8f9
wpt-pr: 12882
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.