Summary: | <video> playlist can not advance when playing in background tab | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Carlson <eric.carlson> | ||||
Component: | Media | Assignee: | Eric Carlson <eric.carlson> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | Keywords: | InRadar | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Eric Carlson
2011-08-25 14:01:39 PDT
Created attachment 105256 [details]
Proposed patch
Comment on attachment 105256 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=105256&action=review > Source/WebCore/ChangeLog:4 > + <video> playlist can not advance when playing in background tab > + https://bugs.webkit.org/show_bug.cgi?id=66978 Change log should give the reason there is no accompanying regression test. > Source/WebCore/html/HTMLMediaElement.cpp:496 > - if (m_restrictions & RequireUserGestureForLoadRestriction && !ScriptController::processingUserGesture()) > + if (requireUserGestureForLoad() && !ScriptController::processingUserGesture()) Seems this improvement, not needed to fix the bug, could be done in a separate patch. > Source/WebCore/html/HTMLMediaElement.cpp:595 > + // Once the page has allowed an element load media, it is free to load at will. This allows a Should read: "an element *to* load media". > Source/WebCore/html/HTMLMediaElement.cpp:1501 > - if (m_restrictions & RequireUserGestureForRateChangeRestriction && !ScriptController::processingUserGesture()) > + if (requireUserGestureForRateChange() && !ScriptController::processingUserGesture()) Seems this improvement, not needed to fix the bug, could be done in a separate patch. > Source/WebCore/html/HTMLMediaElement.cpp:1549 > - if (m_restrictions & RequireUserGestureForRateChangeRestriction && !ScriptController::processingUserGesture()) > + if (requireUserGestureForRateChange() && !ScriptController::processingUserGesture()) Seems this improvement, not needed to fix the bug, could be done in a separate patch. > Source/WebCore/html/HTMLMediaElement.h:195 > bool requireUserGestureForLoad() const { return m_restrictions & RequireUserGestureForLoadRestriction; } > bool requireUserGestureForRateChange() const { return m_restrictions & RequireUserGestureForRateChangeRestriction; } > bool requireUserGestureForFullScreen() const { return m_restrictions & RequireUserGestureForFullScreenRestriction; } > + bool requirePageConsentToLoadMedia() const { return m_restrictions & RequirePageConsentToLoadMedia; } For future consideration: These function names have verbs in them and can be interpreted as calls to change behavior restrictions rather than to give the state of those restrictions. It would be best to rename them to start with the word “should” so it’s clear they are boolean getters. Since these are used only in the class it seems they could start as private. We could make them public later if needed. > Source/WebCore/html/HTMLMediaElement.h:197 > + void removeBehaviorRestriction(BehaviorRestrictions restriction) { m_restrictions &= ~restriction; } Since this is used only in the class, we could start it as private. We could make it public later if needed. |