[webkit-reviews] review granted: [Bug 46241] Html 5 video element Useragent string is QuickTime : [Attachment 130426] Proposed patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Eric Carlson
<eric.carlson at apple.com>'s request for review:
Bug 46241: Html 5 video element Useragent string is QuickTime
https://bugs.webkit.org/show_bug.cgi?id=46241

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

------- Additional Comments from Daniel Bates <dbates at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=130426&action=review


This patch looks straightforward to me.

Notice that the code in LayoutTests/http/tests/media/resources/serve-video.php
doesn't adhere to the WebKit style guide. Although we don't seem to strictly
enforce these rules for test files and test support files, you may want to
consider fixing the style so as to be consistent with the style guide either in
this patch or in a follow up patch.

> Source/WebCore/ChangeLog:3
> +	   Html 5 video element Useragent string is Quicktime

Nit: "Html 5" => "HTML5"

> LayoutTests/http/tests/media/resources/serve-video.php:12
> +	   $range = explode("-", substr($contentRange, 6)); 

I know that you extracted this code from file
LayoutTests/http/tests/media/resources/video-referer-check-referer.php.

The number 6 for the length parameter of substr() seems mysterious, especially
for people who may not be familiar with the syntax for Byte Ranges (maybe we
don't have to worry about this audience?) . For your consideration, I suggest
either substituting the expression strlen("bytes=") or adding an inline comment
that explains that 6 is the length of the bytes-unit string "bytes=".