[iOS] Add an explicit API to allow media documents to (temporarily) play inline
Created attachment 257240 [details] Patch
Comment on attachment 257240 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257240&action=review So much plumbing! > Source/WebCore/dom/Document.h:1019 > + void registerForAllowMediaDocumentInlinePlaybackChangedCallbacks(HTMLMediaElement*); > + void unregisterForAllowMediaDocumentInlinePlaybackChangedCallbacks(HTMLMediaElement*); These should take HTMLMediaElement&; > Source/WebCore/page/Page.h:464 > + WEBCORE_EXPORT bool allowMediaDocumentInlinePlayback() const { return m_allowMediaDocumentInlinePlayback; } This function name is a verb phrase, so sounds like it “allows” something. But really it just returns a boolean. That’s why so many Cocoa names of methods like this function use the word “should” or something like that to make it sound like a boolean predicate and not a verb. > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:215 > +@property (nonatomic, getter=_allowMediaDocumentInlinePlayback, setter=_setAllowMediaDocumentInlinePlayback:) BOOL _allowMediaDocumentInlinePlayback; This function name is a verb phrase, so sounds like it will “allow” something. But really it just returns a boolean. That’s why so many Cocoa names of methods like this function use the word “should” or something like that to make it sound like a boolean predicate and not a verb. Or at least “allows” by analogy with “requires” on the line above.
(In reply to comment #2) > Comment on attachment 257240 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257240&action=review > > So much plumbing! I know!! :-/ > > Source/WebCore/dom/Document.h:1019 > > + void registerForAllowMediaDocumentInlinePlaybackChangedCallbacks(HTMLMediaElement*); > > + void unregisterForAllowMediaDocumentInlinePlaybackChangedCallbacks(HTMLMediaElement*); > > These should take HTMLMediaElement&; Roger. > > Source/WebCore/page/Page.h:464 > > + WEBCORE_EXPORT bool allowMediaDocumentInlinePlayback() const { return m_allowMediaDocumentInlinePlayback; } > > This function name is a verb phrase, so sounds like it “allows” something. > But really it just returns a boolean. That’s why so many Cocoa names of > methods like this function use the word “should” or something like that to > make it sound like a boolean predicate and not a verb. Yeah; allows...? doesAllow...? shouldAllow...? > > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:215 > > +@property (nonatomic, getter=_allowMediaDocumentInlinePlayback, setter=_setAllowMediaDocumentInlinePlayback:) BOOL _allowMediaDocumentInlinePlayback; > > This function name is a verb phrase, so sounds like it will “allow” > something. But really it just returns a boolean. That’s why so many Cocoa > names of methods like this function use the word “should” or something like > that to make it sound like a boolean predicate and not a verb. Or at least > “allows” by analogy with “requires” on the line above. I'll pick a more appropriate name here and post a new patch.
Created attachment 257307 [details] Patch
I went with 'allows' rather than 'shouldAllow' or 'doesAllow', because the property is already super long without an extra non-imperative verb on the front.
Created attachment 257308 [details] Patch
Created attachment 257363 [details] Patch
Comment on attachment 257363 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257363&action=review > Source/WebCore/html/HTMLMediaElement.cpp:-3885 > - Unnecessary! > Source/WebKit2/UIProcess/WebPageProxy.cpp:3991 > +void WebPageProxy::setAllowsMediaDocumentInlinePlayback(bool flag) I would prefer a more descriptive variable name than 'flag.' > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3011 > +void WebPage::setAllowsMediaDocumentInlinePlayback(bool flag) Boo flag > Source/WebKit2/WebProcess/WebPage/WebPage.messages.in:98 > + SetAllowsMediaDocumentInlinePlayback(bool flag) Boooooooo
Committed r187251: <http://trac.webkit.org/changeset/187251>
It looks like you had partially renamed flag to allows. Build fix committed to http://trac.webkit.org/changeset/187263