[Webkit-unassigned] [Bug 47810] [HTML5] Add DOMSettableTokenList

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47810





--- Comment #10 from Erik Arvidsson <arv at chromium.org>  2010-10-19 09:51:47 PST ---
(From update of attachment 71149)
View in context: https://bugs.webkit.org/attachment.cgi?id=71149&action=review

I'm not sure we need to add add and remove to SpaceSplitString. I decided not to do it for classList since mutating the list always had to update the class attribute which triggers a lot of things (such as mutation events and style resolution). If you look at DOMTokenList and SpaceSplitString in isolation it does seem to make sense to not rebuild the SpaceSplitString for add and remove.

> WebCore/html/DOMSettableTokenList.cpp:70
> +        builder.append(m_value);

This is not right. Please check the spec.

The point is that you should not add a space if the last character is already a space.

All of this is implemented in the old DOMTokenList and covered by the layout tests. You should be able to reuse almost all of the tests for classList since DOMTokenList is a subset of SettableDOMTokenList.

> WebCore/html/DOMSettableTokenList.cpp:74
> +        m_tokens.set(builder.toString(), true);

Can we add an add to SpaceSplitString?

> WebCore/html/DOMSettableTokenList.cpp:90
> +    m_tokens.clear();

I think you could call setValue here and in addInternal to remove some duplicate code. I'm not sure if it is really worth it though?

> WebCore/html/DOMSettableTokenList.cpp:92
> +    m_tokens.set(m_value, true);

Can we add a remove to SpaceSplitString?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.