[webkit-reviews] review granted: [Bug 68610] Microdata: Basic implementation of document.getItems() method : [Attachment 110685] Updated patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Arko Saha <nghq36 at motorola.com>'s
request for review:
Bug 68610: Microdata: Basic implementation of document.getItems() method
https://bugs.webkit.org/show_bug.cgi?id=68610

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110685&action=review


I know there's some discussion about whether we should prefix this API but that
can be added later if we strongly feel about. r=me provided you change
UndefinedMicroDataItemType to some unique url under webkit.org.

> Source/WebCore/dom/Document.cpp:5212
> +    // Since documet.getItem() is allowed for microdata, typeNames will be
null string.
> +    // In this case we need to create an unique string identifier to map
such request in the cache.
> +    String localTypeNames = typeNames.isNull() ?
String("UndefinedMicroDataItemType") : typeNames;

I don't think UndefinedMicroDataItemType is unique enough. Authors can
certainly define UndefinedMicroDataItemType as a type, right?  If anything, I'd
suggest to use http://webkit.org/microdata/undefined since type can be an url.

> Source/WebCore/dom/Node.cpp:2289
> +    // We need to invalidate the microDataItemListCache when itemtype
attribute changed

This comment repeats what the code says. Please remove.