Bug 173030 - Netflix seeking quirk should also apply to Now Playing, and should always use the livestream UI
Summary: Netflix seeking quirk should also apply to Now Playing, and should always use...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2017-06-06 13:05 PDT by Beth Dakin
Modified: 2017-06-06 14:47 PDT (History)
7 users (show)

See Also:

Patch (23.62 KB, patch)
2017-06-06 13:25 PDT, Beth Dakin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2017-06-06 13:05:22 PDT
Netflix seeking quirk should also apply to Now Playing, and should always use the livestream UI

Comment 1 Beth Dakin 2017-06-06 13:25:47 PDT
Created attachment 312114 [details]
Comment 2 Build Bot 2017-06-06 13:28:29 PDT
Attachment 312114 [details] did not pass style-queue:

ERROR: Source/WebCore/html/HTMLMediaElement.cpp:7100:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/html/HTMLMediaElement.cpp:7104:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 2 in 7 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Darin Adler 2017-06-06 14:06:45 PDT
Comment on attachment 312114 [details]

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

> Source/WebCore/html/HTMLMediaElement.cpp:7091
> +static bool needsSeekingSupportQuirk(Page& page)

It’s awkward to use this since the caller needs to check Page for null. I think it would be cleaner to just pass an Element or Document to this function.

Also, we could write an implementation that uses Document::settings to check the "needs quirks" flag, and then Document::topDocument to get the hose, and then we would not need to use Page at all.

In fact, there would be no null checks needed either!

    auto& document = element.document().topDocument();
    if (!document.settings().needsSiteSpecificQuirks())
        return false;
    String host = document.url().host();
    return equalLettersIgnoringASCIICase(host, "netflix.com") || host.endsWithIgnoringASCIICase(".netflix.com");

> Source/WebCore/html/HTMLMediaElement.cpp:7110
> +    if (Page* page = document().page()) {
> +        if (needsSeekingSupportQuirk(*page))
> +            return false;
> +    }
>      return !isLiveStream();

Then this code could be:

    return !needsSeekingSupportQuirk(*this) && !isLiveStream();

> Source/WebCore/platform/mac/WebPlaybackControlsManager.mm:123
> +    return !isnan(_contentDuration) && !isinf(_contentDuration);

A nice way to write this is:

    return std::isfinite(_contentDuration);
Comment 4 Beth Dakin 2017-06-06 14:47:06 PDT
Thanks Darin! I addressed all of your comments.
Comment 5 Beth Dakin 2017-06-06 14:47:12 PDT