[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