[webkit-reviews] review granted: [Bug 71510] [Microdata] LayoutTests/fast/dom/MicroData/itemid-attribute-test.html assertion failure in Element::getURLAttribute() : [Attachment 113792] Updated patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Arko Saha <nghq36 at motorola.com>'s
request for review:
Bug 71510: [Microdata]
LayoutTests/fast/dom/MicroData/itemid-attribute-test.html assertion failure in
Element::getURLAttribute()
https://bugs.webkit.org/show_bug.cgi?id=71510

Attachment 113792: Updated patch
https://bugs.webkit.org/attachment.cgi?id=113792&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=113792&action=review


> Source/WebCore/html/HTMLElement.cpp:1004
> +bool HTMLElement::isURLAttribute(Attribute* attribute) const
> +{
> +#if ENABLE(MICRODATA)
> +    return attribute->name() == itemidAttr;
> +#else
> +    return false;
> +#endif
> +}

This will fail to compile on platforms where we have MICRODATA off and we have
unused argument warnings on.

> Source/WebCore/html/HTMLElement.h:79
> +    virtual bool isURLAttribute(Attribute*) const;

This should be protected, not public.

> Source/WebCore/html/HTMLInputElement.cpp:1295
> +    return (attr->name() == srcAttr || attr->name() == formactionAttr ||
HTMLTextFormControlElement::isURLAttribute(attr));

Should remove the unneeded parentheses here.

> Source/WebCore/html/HTMLObjectElement.cpp:387
> +    return (attr->name() == dataAttr || (attr->name() == usemapAttr &&
attr->value().string()[0] != '#') ||
HTMLPlugInImageElement::isURLAttribute(attr));

Should remove the unneeded parentheses here.