[webkit-reviews] review denied: [Bug 43017] removeFormat needs to be reimplemented : [Attachment 71209] reimplements RemoveFormat

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 43017: removeFormat needs to be reimplemented
https://bugs.webkit.org/show_bug.cgi?id=43017

Attachment 71209: reimplements RemoveFormat
https://bugs.webkit.org/attachment.cgi?id=71209&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=71209&action=review

Code looks fine, just some naming nits.

> WebCore/editing/ApplyStyleCommand.h:131
> +    bool (*m_isInlineElementToRemove)(const Element*);

Can we name this m_isInlineElementToRemoveFunction?  This seems to follow the
naming convention in other parts of the code.  Also, can you make a typedef? 
In create(), it's particularly hard to read.

> LayoutTests/editing/execCommand/remove-format-elements.html:70
> +	   elementName = elementList[elementList.length-1]

Nit: spaces round the minus sign

> LayoutTests/editing/execCommand/remove-format-elements.html:92
> +    document.execCommand('removeFormat', false, null);

This tests each element one at a time.	Would it be possible to add a test that
has multiple elements in it?