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

Upstream a test for CustomElementRegistry interface from WebKit #3782

Conversation

rniwa
Copy link
Contributor

@rniwa rniwa commented Sep 21, 2016

No description provided.

@wpt-pr-bot
Copy link
Collaborator

Reviewers for this pull request are: @alsemenov, @deepak-sa, @domenic, @dominiccooney, @hayatoito, @kojiishi, @rniwa, @sgrekhov, and @TakayoshiKochi.

@rniwa
Copy link
Contributor Author

rniwa commented Sep 21, 2016

@kojiishi


test(function () {
customElements.define('custom-html-element', HTMLElement);
}, 'customElements.define must not throw the constructor is HTMLElement');
Copy link
Contributor

Choose a reason for hiding this comment

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

Grammar: throw the constructor -> throw if the constructor

calls.push(name);
return target[name];
}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe trailing semi here to not rely on automatic insertion?


(function () {
var testCase = async_test('customElements.define must not throw'
+' when defining another custom element in a different global object during Get(constructor, "prototype")', {timeout: 100});
Copy link
Contributor

Choose a reason for hiding this comment

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

Style: maybe space between + and '

@domenic
Copy link
Member

domenic commented Sep 21, 2016

It's worth noting we have some tests already in https://github.com/w3c/web-platform-tests/blob/master/custom-elements/custom-element-registry/define.html. I'm fine with duplicate coverage (more tests better!), but we might want to consider the folder structure a bit more closely. E.g. maybe this should be split up into define-2.html, whenDefined.html, etc.? It's more important to land though so don't let that stop you.

@dominiccooney
Copy link
Contributor

We have evolved defined.html downstream so it might be easier to upstream them as-is and do the merging upstream.

@sideshowbarker
Copy link
Contributor

We have evolved defined.html downstream so it might be easier to upstream them as-is and do the merging upstream.

OK, will merge this to master as-is then

@sideshowbarker sideshowbarker merged commit 5a95ab0 into web-platform-tests:master Sep 26, 2016
@rniwa rniwa deleted the add-test-for-custom-element-registry branch August 2, 2022 18:00
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

5 participants