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

[bugzilla-daemon at webkit.org]

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


Kenichi Ishibashi <bashi at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71149|review?                     |review-
               Flag|                            |




--- Comment #11 from Kenichi Ishibashi <bashi at google.com>  2010-10-19 18:16:43 PST ---
(From update of attachment 71149)
View in context: https://bugs.webkit.org/attachment.cgi?id=71149&action=review

Hi Erik,

Thank you for your helpful comments! I'll post revised patch later.

>> 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.

Thank you for correction. I should be more careful.. I've modified the patch to reuse existing code.

>> WebCore/html/DOMSettableTokenList.cpp:74
>> +        m_tokens.set(builder.toString(), true);
> 
> Can we add an add to SpaceSplitString?

Added.

>> 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?

I reused existing code so the code no longer exists.

>> WebCore/html/DOMSettableTokenList.cpp:92
>> +    m_tokens.set(m_value, true);
> 
> Can we add a remove to SpaceSplitString?

Added.

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