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

Fix customized built-in element is handling and reactions #1369

Merged
merged 1 commit into from
Jun 6, 2016
Merged

Conversation

domenic
Copy link
Member

@domenic domenic commented Jun 1, 2016

This uses the new "is value" concept introduced to DOM in
whatwg/dom#262, which more correctly implements
the intention that changing the is attribute after element creation does
not impact the processing model, and thus fixes #1298. This allows us to
then fix #1360: custom element callback reactions for customized
built-in elements were not correctly looking up their definition.

Do not merge until whatwg/dom#262 is merged. @dominiccooney and an editor to review.

This uses the new "is value" concept introduced to DOM in
whatwg/dom#262, which more correctly implements
the intention that changing the is attribute after element creation does
not impact the processing model, and thus fixes #1298. This allows us to
then fix #1360: custom element callback reactions for customized
built-in elements were not correctly looking up their definition.
@domenic domenic added do not merge yet Pull request must not be merged per rationale in comment topic: custom elements Relates to custom elements (as defined in DOM and HTML) labels Jun 1, 2016
@dominiccooney
Copy link

LGTM

FWIW in code we have a pair of local-name, name which we call a descriptor and use as a key for matching elements to definitions. "name", if set, is always a valid custom element name, and it lets you represent all kinds of elements:

  1. name is not set ⇒ not a potential custom element
  2. local-name == name ⇒ autonomous
  3. local-name != name ⇒ customized built-in; local-name must not be a valid custom element name

This is isomorphic to the representation you have here, but it might be useful—you no longer talk about matching the name string, which is tedious/tricky with the existence of customized build-in elements, but instead on associating this tuple with an element and then using that to match definitions (both parts must match.)

@domenic domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Jun 3, 2016
@annevk
Copy link
Member

annevk commented Jun 6, 2016

I like that alternate representation, but this change is okay too.

@domenic domenic merged commit 4e632a8 into master Jun 6, 2016
@domenic domenic deleted the save-is branch June 6, 2016 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: custom elements Relates to custom elements (as defined in DOM and HTML)
3 participants