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

Add a validation step when a new DOMTokenList is created #107

Merged
merged 1 commit into from Nov 13, 2015

Conversation

yoavweiss
Copy link
Contributor

Closes #106

@annevk - I added a validation step to the DOMTokenList creation. Can you take a look?

@@ -8642,8 +8642,10 @@ are to:
<a lt="named attribute"><var>associated attribute's local name</var> attribute</a> or
associated <a for="/">element</a>'s
<a lt="named attribute"><var>associated attribute's local name</var> attribute</a> is
<a lt="attribute is set">set</a>, set <a>tokens</a> to the new <a for=Attr>value</a>,
<a lt="ordered set parser">parsed</a>.
<a lt="attribute is set">set</a>, let <var>temporary tokens</var> be the new <a for=Attr>value</a>,
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a "run these substeps" if we start creating variables. I think that would be a little easier to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@yoavweiss
Copy link
Contributor Author

Fixed the issues raised in comments

@@ -8606,7 +8606,7 @@ associated <a>attribute</a>'s <a for=Attr>local name</a>.
<var>token</var> are:

<ol>
<li><p>If the associated attribute's <a for=Attr>local name</a> does not define <a>supported
<li><p>If the associated <a>attribute</a>'s <a for=Attr>local name</a> does not define <a>supported
tokens</a>, return true.
Copy link
Member

Choose a reason for hiding this comment

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

Last nit, no breaking inside of a single element. (HTML does it, but try to avoid it as it makes search & replace harder than it needs to be.)

With that fixed, if you rebase/squash, I'll land this.

This fixes multiple issues introduced when adding validation to
DOMTokenList:

* When a new DOMTokenList object is created, we now validate each token.
* IDL definition of `add()` was fixed to reflect returning a boolean.
* A reference was added to "attribute" in a place where one was missing.
@yoavweiss yoavweiss force-pushed the validate_incoming_attributeset branch from 5829f19 to 63a0302 Compare November 13, 2015 09:44
@yoavweiss
Copy link
Contributor Author

Fixed and squashed

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