Bug 165104 – Blacklist Netflix for TouchBar support

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Beth Dakin]

Blacklist Netflix for TouchBar support

rdar://problem/29404778

[Beth Dakin]

Created attachment 295516 [details]
Patch

[Darin Adler]

Comment on attachment 295516 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295516&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:7142
> +static bool needsPlaybackControlsManagerQuirk(bool needsQuirks, const URL& url)

I think I would have suggested having this take a Page& argument rather than having the caller dig out the boolean and the URL.

> Source/WebCore/html/HTMLMediaElement.cpp:7148
> +    if (!needsQuirks)
> +        return false;
> +
> +    String host = url.host();
> +    return equalLettersIgnoringASCIICase(host, "www.netflix.com");

I think this would read better without the local variable. Maybe even just a one-liner:

    return needsQuirks && equalLettersIgnoringASCIICase(url.host(), "www.netflix.com");

[Tim Horton]

Comment on attachment 295516 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295516&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:7148
> +    return equalLettersIgnoringASCIICase(host, "www.netflix.com");

This could be a (host isEqualIgnoringCase "netflix.com" || host hasPrefixIgnoringCase ".netflix.com") if you don't want to depend on Netflix's redirect.

[WebKit Commit Bot]

Comment on attachment 295516 [details]
Patch

Clearing flags on attachment: 295516

Committed r209013: <http://trac.webkit.org/changeset/209013>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Beth Dakin]

Re-opening since the blacklisting failed to cover Now Playing.

[Beth Dakin]

Created attachment 295548 [details]
Patch

[Beth Dakin]

Thanks Tim https://trac.webkit.org/changeset/209045

[Darin Adler]

Hi Beth. What we ended up landing didn’t match Tim’s suggestion, and will return false if Netflix ever uses "netflix.com"; might want to return and clean that up either by removing the "www.netflix.com" check (which is redundant) or changing it to "netflix.com" (taking Tim’s suggestion more literally).

[Beth Dakin]

(In reply to comment #9)
> Hi Beth. What we ended up landing didn’t match Tim’s suggestion, and will
> return false if Netflix ever uses "netflix.com"; might want to return and
> clean that up either by removing the "www.netflix.com" check (which is
> redundant) or changing it to "netflix.com" (taking Tim’s suggestion more
> literally).

Oops!

[Beth Dakin]

Committed follow-up: https://trac.webkit.org/changeset/209206