[webkit-reviews] review granted: [Bug 111526] Adding id attribute test for MediaStream : [Attachment 191868] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 7 01:05:37 PST 2013


Kentaro Hara <haraken at chromium.org> has granted Li Yin <li.yin at intel.com>'s
request for review:
Bug 111526: Adding id attribute test for MediaStream
https://bugs.webkit.org/show_bug.cgi?id=111526

Attachment 191868: Patch
https://bugs.webkit.org/attachment.cgi?id=191868&action=review

------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=191868&action=review


LGTM too, with some nits.

> LayoutTests/fast/mediastream/MediaStreamConstructor.html:31
> +	   if (charCode == 0x21 ||
> +	       (charCode >= 0x23 && charCode <= 0x27) ||
> +	       (charCode >= 0x2A && charCode <= 0x2B) ||
> +	       (charCode >= 0x2D && charCode <= 0x2E) ||
> +	       (charCode >= 0x30 && charCode <= 0x39) ||
> +	       (charCode >= 0x41 && charCode <= 0x5A) ||
> +	       (charCode >= 0x5E && charCode <= 0x7E))

Nit: || should come at the head of the next line, instead of the tail of the
current line.

http://www.webkit.org/coding/coding-style.html#indentation-case-label

> LayoutTests/fast/mediastream/MediaStreamConstructor.html:36
> +	   if (id == idArray[i])

Nit: For testing purpose, let's use === instead of ==.


More information about the webkit-reviews mailing list