[webkit-reviews] review granted: [Bug 69839] [Microdata] Add itemprop, itemref, itemvalue attributes : [Attachment 111854] Updated patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Oct 21 14:59:32 PDT 2011


Ryosuke Niwa <rniwa at webkit.org> has granted Arko Saha <nghq36 at motorola.com>'s
request for review:
Bug 69839: [Microdata] Add itemprop, itemref, itemvalue attributes
https://bugs.webkit.org/show_bug.cgi?id=69839

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

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


r=me. I'm very impressed by your through testing and how clean your patch is!
Keep up your great work.

I can fix those myself and land it for you if you'd like.

> Source/WebCore/html/HTMLElement.idl:76
> +		   attribute [Conditional=MICRODATA, Custom] custom itemValue

Please fix indentation here (missing one space).

> LayoutTests/fast/dom/MicroData/002-expected.txt:5
> +document.getItems() without aurgument
> +TEST SUCCEEDED

Why the change? PASS looked better.

>
LayoutTests/fast/dom/MicroData/itemprop-for-an-element-must-be-correct-expected
.txt:15
> +PASS element.itemProp[2] == 'FOO' is true

Maybe try something like fOo?

>
LayoutTests/fast/dom/MicroData/itemprop-for-an-element-must-be-correct.html:28
> +debug("<br>itemProp.length should be 0 if element has not tokens.");

Nit: has no tokens or doesn't have any tokens?

> LayoutTests/fast/dom/MicroData/itemvalue-reflects-the-src-attr.html:1
> +<html>

Nit: can we add <!DOCTYPE html>?


More information about the webkit-reviews mailing list