RESOLVED FIXED 173030
Netflix seeking quirk should also apply to Now Playing, and should always use the livestream UI
https://bugs.webkit.org/show_bug.cgi?id=173030
Summary Netflix seeking quirk should also apply to Now Playing, and should always use...
Beth Dakin
Reported 2017-06-06 13:05:22 PDT
Netflix seeking quirk should also apply to Now Playing, and should always use the livestream UI rdar://problem/32228660
Attachments
Patch (23.62 KB, patch)
2017-06-06 13:25 PDT, Beth Dakin
darin: review+
Beth Dakin
Comment 1 2017-06-06 13:25:47 PDT
Build Bot
Comment 2 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.
Darin Adler
Comment 3 2017-06-06 14:06:45 PDT
Comment on attachment 312114 [details] Patch 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);
Beth Dakin
Comment 4 2017-06-06 14:47:06 PDT
Thanks Darin! I addressed all of your comments.
Beth Dakin
Comment 5 2017-06-06 14:47:12 PDT
Note You need to log in before you can comment on or make changes to this bug.