Bug 172881 – Modify Netflix controlsManager quirk to prevent only scrubbing

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Beth Dakin]

Modify Netflix controlsManager quirk to prevent only scrubbing

rdar://problem/32228660

[Beth Dakin]

Created attachment 311887 [details]
Patch

[Build Bot]

Attachment 311887 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:518:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:522:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:526:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:529:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:529:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:530:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/mac/WebPlaybackControlsManager.mm:59:  Should not have spaces around = in property synthesis.  [whitespace/property] [4]
Total errors found: 7 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andy Estes]

Comment on attachment 311887 [details]
Patch

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

> Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:533
> +    String host = document->url().host();
> +    return !(equalLettersIgnoringASCIICase(host, "netflix.com") || host.endsWithIgnoringASCIICase(".netflix.com"));

Not new to this patch, but the double string comparison makes me a little sad. In the past I've done something like this:

    String netflixDomain = ASCIILiteral("netflix.com");
    if (!host.endsWithIgnoringASCIICase(netflixDomain))
        return true;

    unsigned suffixOffset = host.length() - netflixDomain.length();
    return suffixOffset && host[suffixOffset - 1] != '.';

[Wenson Hsieh]

Does this mean that Now Playing scrubber controls will now be enabled again for Netflix?

[Eric Carlson]

(In reply to Andy Estes from comment #3)
> Comment on attachment 311887 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=311887&action=review
> 
> > Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:533
> > +    String host = document->url().host();
> > +    return !(equalLettersIgnoringASCIICase(host, "netflix.com") || host.endsWithIgnoringASCIICase(".netflix.com"));
> 
> Not new to this patch, but the double string comparison makes me a little
> sad. In the past I've done something like this:
> 
>     String netflixDomain = ASCIILiteral("netflix.com");
>     if (!host.endsWithIgnoringASCIICase(netflixDomain))
>         return true;
> 
>     unsigned suffixOffset = host.length() - netflixDomain.length();
>     return suffixOffset && host[suffixOffset - 1] != '.';

Isn't it possible for host.length to be smaller than netflixDomain.length?

[Andy Estes]

(In reply to Eric Carlson from comment #5)
> (In reply to Andy Estes from comment #3)
> > Comment on attachment 311887 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=311887&action=review
> > 
> > > Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:533
> > > +    String host = document->url().host();
> > > +    return !(equalLettersIgnoringASCIICase(host, "netflix.com") || host.endsWithIgnoringASCIICase(".netflix.com"));
> > 
> > Not new to this patch, but the double string comparison makes me a little
> > sad. In the past I've done something like this:
> > 
> >     String netflixDomain = ASCIILiteral("netflix.com");
> >     if (!host.endsWithIgnoringASCIICase(netflixDomain))
> >         return true;
> > 
> >     unsigned suffixOffset = host.length() - netflixDomain.length();
> >     return suffixOffset && host[suffixOffset - 1] != '.';
> 
> Isn't it possible for host.length to be smaller than netflixDomain.length?

We already know that host ends with netflixDomain at this point, so it must be at least as long as netflixDomain.

[Beth Dakin]

(In reply to Andy Estes from comment #3)
> Comment on attachment 311887 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=311887&action=review
> 
> > Source/WebCore/platform/cocoa/WebPlaybackSessionModelMediaElement.mm:533
> > +    String host = document->url().host();
> > +    return !(equalLettersIgnoringASCIICase(host, "netflix.com") || host.endsWithIgnoringASCIICase(".netflix.com"));
> 
> Not new to this patch, but the double string comparison makes me a little
> sad. In the past I've done something like this:
> 
>     String netflixDomain = ASCIILiteral("netflix.com");
>     if (!host.endsWithIgnoringASCIICase(netflixDomain))
>         return true;
> 
>     unsigned suffixOffset = host.length() - netflixDomain.length();
>     return suffixOffset && host[suffixOffset - 1] != '.';

Cool, I'll try this! Thanks Andy!

[Beth Dakin]

(In reply to Wenson Hsieh from comment #4)
> Does this mean that Now Playing scrubber controls will now be enabled again
> for Netflix?

It means that Netflix will have all of the controls, but if a user tries to use the timeline to scrub, nothing will happen.

[Beth Dakin]

https://trac.webkit.org/changeset/217791/webkit