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.

Sign up for GitHub

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

Add "precustomized" custom element state #894

Merged
merged 5 commits into from
Sep 28, 2020
Merged

Add "precustomized" custom element state #894

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9318a88
Add "precustomized" state
mfreed7 Sep 16, 2020
f63d8cf
Address review comments
mfreed7 Sep 25, 2020
cf4f0db
Custom element definition should include "precustomized"
mfreed7 Sep 25, 2020
ee36e05
Remove precustomized from "custom" definition
mfreed7 Sep 25, 2020
7431705
formatting
annevk Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions dom.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5875,19 +5875,19 @@ dictionary ShadowRootInit {
initialized.

<p>An <a for=/>element</a>'s <a for=Element>custom element state</a> is one of
"<code>undefined</code>", "<code>failed</code>", "<code>uncustomized</code>", or
"<code>custom</code>". An <a for=/>element</a> whose <a for=Element>custom element state</a> is
"<code>uncustomized</code>" or "<code>custom</code>" is said to be
<dfn export id=concept-element-defined for=Element>defined</dfn>. An <a for=/>element</a> whose
<a for=Element>custom element state</a> is "<code>custom</code>" is said to be
<dfn export id=concept-element-custom for=Element>custom</dfn>.
"<code>undefined</code>", "<code>failed</code>", "<code>uncustomized</code>",
"<code>precustomized</code>", or "<code>custom</code>". An <a for=/>element</a> whose <a
for=Element>custom element state</a> is "<code>uncustomized</code>", "<code>precustomized</code>",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems wrong. Why would a custom element be considered as defined before super() call is made in the constructor? This will be incompatible with the shipping behavior of custom elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I agree. I've removed "precustomized" from the definition of "defined".

@annevk do you still think there is a need for more testing of :defined?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should have a test yes, to ensure that it does not match that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll add a test that :defined doesn't match, pre-super().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we already have a test for :defined during an upgrade, and I think it's good enough to catch problems here. Note a couple things:

  • :defined already doesn't match during an upgrade, even after super(), because the custom element state is only set to "custom" after the constructor completes.
  • Testing :defined prior to super() shouldn't be typical anyway, since you can't access this prior to super(). You could use an external reference to the element to test, of course, but that doesn't seem like it'd be common.

I'm sure I'm missing something here - please let me know what it is, and what you'd like me to test for. I can add a test for the second bullet above, but it doesn't seem terribly useful. It would catch the case where :defined matches prior to super() but stops matching it after super().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this additional testing, in case that's what you're looking for here.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, I think those asserts are helpful.

or "<code>custom</code>" is said to be <dfn export id=concept-element-defined
for=Element>defined</dfn>. An <a for=/>element</a> whose <a for=Element>custom element state</a> is
"<code>custom</code>" is said to be <dfn export id=concept-element-custom for=Element>custom</dfn>.

<p class=note>Whether or not an element is <a for=Element>defined</a> is used to determine the
behavior of the '':defined'' pseudo-class. Whether or not an element is <a for=Element>custom</a> is
used to determine the behavior of the <a href=#mutation-algorithms>mutation algorithms</a>. The
"<code>failed</code>" state is used to ensure that if a <a>custom element constructor</a> fails to
execute correctly the first time, it is not executed again by an
<a lt="upgrade an element">upgrade</a>.</p>
"<code>failed</code>" and "<code>precustomized</code>" states are used to ensure that if a <a>custom
element constructor</a> fails to execute correctly the first time, it is not executed again by an <a
lt="upgrade an element">upgrade</a>.</p>

<div class=example id=example-c5b21302>
<p>The following code illustrates elements in each of these four states:</p>
Expand Down