[webkit-reviews] review denied: [Bug 80269] [Microdata] Implement PropertyNodeList interface. : [Attachment 133059] Updated patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Arko Saha <nghq36 at motorola.com>'s
request for review:
Bug 80269: [Microdata] Implement PropertyNodeList interface.
https://bugs.webkit.org/show_bug.cgi?id=80269

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=133059&action=review


> Source/WebCore/bindings/js/JSMicroDataItemValueCustom.cpp:49
> +    return jsString(exec, itemValue->getString());

Why does this return a string?	We have a MicroDataItemValue.idl in this file. 
I would have expected it to return a MicroDataItemValue...

> Source/WebCore/bindings/v8/custom/V8MicroDataItemValueCustom.cpp:48
> +    return v8String(itemValue->getString());

Same question here.

> Source/WebCore/dom/PropertyNodeList.cpp:48
> +	   m_propertyValue.append(element->itemValue());

Is this going to keep appending to m_propertyValue every time this function is
called?  That doesn't seem right.

> Source/WebCore/dom/PropertyNodeList.cpp:66
> +Node* PropertyNodeList::itemWithName(const AtomicString& id) const

itemWithName not itemWithID ?

> Source/WebCore/dom/PropertyNodeList.h:52
> +    PropertyValueArray getValues();

Is this going to thrash the RefCounts of the MicroDataItemValue?

> Source/WebCore/dom/PropertyNodeList.h:61
> +    PropertyNodeList(Vector<RefPtr<Node> >& nodes)

Please add the explicit keyword.