[Webkit-unassigned] [Bug 37674] Implement sizes attribute for link tag from HTML5

[bugzilla-daemon at webkit.org]

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


Dimitri Glazkov (Google) <dglazkov at chromium.org> changed:

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




--- Comment #22 from Dimitri Glazkov (Google) <dglazkov at chromium.org>  2011-07-22 10:40:02 PST ---
(From update of attachment 101696)
View in context: https://bugs.webkit.org/attachment.cgi?id=101696&action=review

the bots are all confused, probably need to re-run the patch. Great progress!

> LayoutTests/fast/dom/icon-size-property.html:42
> +

Let's also test setting the sizes attribute from script.

> Source/WebCore/html/DOMSettableTokenList.cpp:103
> +    return DOMTokenList::validateToken(token, ec);

Could we just override validateToken and not do the fooInternal dance?

> Source/WebCore/html/SizesList.cpp:56
> +                localEc = SYNTAX_ERR;

Are we really supposed to throw SYNTAX_ERR in these cases?

> Source/WebCore/html/SizesList.cpp:85
> +    ExceptionCode ec;
> +    DOMSettableTokenList::setValue("");
> +    for (unsigned i = 0; i < tokens.size(); ++i)
> +        add(tokens[i], ec);

This seems inefficient. It's almost like SpaceSplitString wants to have a validating function parameter. But I guess it's ok as is.

> Source/WebCore/html/SizesList.h:37
> +class SizesList : public DOMSettableTokenList {

Should we be more specific in naming, like IconSizeList?

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