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

Make the DOMTokenList creation steps use "get an attribute value" #673

Merged
merged 1 commit into from
Aug 8, 2018

Conversation

TRowbotham
Copy link
Contributor

@TRowbotham TRowbotham commented Aug 1, 2018

This makes it so that if the DOMTokenList's associated element has an attribute that is the associated attribute's local name, at creation time, it will use the "get an attribute value" steps, which will always return a string. Previously, the this used the "get an attribute by namespace and local name" steps, which would return an Attr node if the associated element had an attribute that is the associated attribute's local name. Fixes #666.


Preview | Diff

This makes it so that if the `DOMTokenList`'s associated element has an attribute that is the associated attribute's local name, at creation time, it will use the "get an attribute value" steps, which will always return a string. Previously, the this used the "get an attribute by namespace and local name" steps, whuch would return an `Attr` node if the associated element had an attribute that is the associated attribute's local name. Fixes whatwg#666.
@TRowbotham
Copy link
Contributor Author

Sorry, I was reshuffling to not base my pulls off my fork's master branch, so I had to recreate this pull request.

Just a few thoughts.

Current behavior

  • Uses "get an attribute by namespace and local name", which returns an Attr node or null
  • This only worked if the element did not have an attribute that was the associated attribute's local name and therefore, "get an attribute by namespace and local name" would return null.
  • If the value is null, the "attribute change steps" would use "Step 1" and empty the token set.
  • If the value is an Attr node, the "attribute change steps" will use "Step 2" and will try to run the "ordered set parsing" steps on the Attr node. (I guess the best case scenario here is that it gets serialized to "[object Attr]" and the "ordered set parsing" steps return a new token set containing "[object" and "Attr]")

New behavior

  • Uses "get an attribute value", which always returns a string.
  • The "attribute change steps" will now always use "Step 2", which runs the string through the "ordered set parsing" steps. (This now always replaces the token set with a new token set instead of just emptying the existing token set when an attribute with the associated local name is not present, since we always pass a string unlike the current behavior, which passes null when the attribute is not present.)

@annevk annevk merged commit 28cdb47 into whatwg:master Aug 8, 2018
@annevk
Copy link
Member

annevk commented Aug 8, 2018

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants