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

Test that window.customElements is per global, not per-document #5681

Merged
merged 4 commits into from
May 2, 2017

Conversation

domenic
Copy link
Member

@domenic domenic commented Apr 24, 2017

See whatwg/html#2578.

/cc @rniwa @dominiccooney


This change is Reviewable

@wpt-pr-bot
Copy link
Collaborator

@w3c-bots
Copy link

w3c-bots commented Apr 24, 2017

View the complete job log.

Lint

Passed

@w3c-bots
Copy link

w3c-bots commented Apr 24, 2017

View the complete job log.

Firefox (nightly channel)

Testing web-platform-tests at revision d547f34
Using browser at version BuildID 20170501100238; SourceStamp 076a7a66096f9e8d102548397254be32eb26bc3d
Starting 10 test iterations
All results were stable

All results

1 test ran
/custom-elements/custom-element-registry/per-global.html
Subtest Results Messages
TIMEOUT
Discarding the browsing context must not change window.customElements FAIL assert_equals: expected object "[object CustomElementRegistry]" but got object "[object CustomElementRegistry]"
Navigating from the initial about:blank must not replace window.customElements PASS
document.open() must replace window.customElements PASS

@w3c-bots
Copy link

w3c-bots commented Apr 24, 2017

View the complete job log.

Chrome (unstable channel)

Testing web-platform-tests at revision d547f34
Using browser at version 60.0.3080.5 dev
Starting 10 test iterations
All results were stable

All results

1 test ran
/custom-elements/custom-element-registry/per-global.html
Subtest Results Messages
OK
Discarding the browsing context must not change window.customElements PASS
Navigating from the initial about:blank must not replace window.customElements FAIL assert_equals: expected object "[object CustomElementRegistry]" but got object "[object CustomElementRegistry]"
document.open() must replace window.customElements FAIL assert_not_equals: got disallowed value object "[object CustomElementRegistry]"

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

This is great. Love the abstraction. I'm curious though whether Chrome and Safari can implement this or have some kind of architectural objection to it given that nothing seems tied to the lifetime of the Window object there now.


const after = frame[propertyName];
assert_equals(after, before);
t.done();
Copy link
Member

Choose a reason for hiding this comment

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

Use step_func_done?

Copy link
Member Author

Choose a reason for hiding this comment

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

Cannot be used here because we don't want to be done if the location is about:blank. I'll add a comment.


const after = frame[propertyName];
assert_not_equals(after, before);
t.done();
Copy link
Member

Choose a reason for hiding this comment

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

Same comment here.


iframe.src = "/common/blank.html";
document.body.appendChild(iframe);
}, `document.open() must replace window.${propertyName}`);
Copy link
Member

Choose a reason for hiding this comment

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

This is disputed: whatwg/html#1698. Not sure if we should test it. We should maybe call this out as an open issue in the HTML Standard.

Copy link
Member Author

Choose a reason for hiding this comment

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

My take is that even if we change the spec there we'll still want to reset all the per-Window objects. I'll comment there.

<link rel="author" title="Domenic Denicola" href="mailto:d@domenic.me">
<script src="/common/object-association.js"></script>

<iframe id="blank-iframe"></iframe>
Copy link
Member

Choose a reason for hiding this comment

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

It seems you can remove this. Meaning this could in fact be a .window.js though that would remove some metadata so up to you.

@domenic
Copy link
Member Author

domenic commented May 2, 2017

Let's merge this and file bugs, and if people object we can reopen discussion. It seems like the right way to get the conversation started.

@domenic domenic merged commit 01f7065 into master May 2, 2017
@domenic domenic deleted the ce-per-window branch May 2, 2017 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants