[webkit-reviews] review denied: [Bug 86409] Site-specific hack: Disclaim WebM as a supported type on Mac for YouTube. : [Attachment 141884] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 86409: Site-specific hack: Disclaim WebM as a supported type on Mac for
YouTube.
https://bugs.webkit.org/show_bug.cgi?id=86409

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=141884&action=review


Seems generally fine, but needs a little refactoring. We don’t want the same
logic repeated twice.

> Source/WebCore/html/HTMLMediaElement.cpp:629
> +    Settings* settings = document()->settings();
> +    if (settings && settings->needsSiteSpecificQuirks()) {
> +	   String host = document()->securityOrigin()->host();
> +	   if ((host.endsWith(".youtube.com", false) ||
equalIgnoringCase("youtube.com", host))
> +	       && (mimeType.startsWith("video/webm", false) ||
mimeType.startsWith("video/x-flv", false)))
> +	       return "";
> +    }

This code is repeated twice. Could you please put it into a helper function?
That way you can also avoid repeating the comment twice.

The comment does not explain why we are skipping both the video/webm MIME type
and the video/x-flv MIME type. Nor why the hack is specifically for YouTube.