Summary: | Modify Netflix controlsManager quirk to prevent only scrubbing | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||
Component: | Media | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aestes, bdakin, buildbot, eric.carlson, jer.noble, thorton, wenson_hsieh | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Beth Dakin
2017-06-02 16:37:19 PDT
Created attachment 311887 [details]
Patch
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.
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] != '.'; Does this mean that Now Playing scrubber controls will now be enabled again for Netflix? (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? (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. (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! (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. |