WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
147181
[iOS] Add an explicit API to allow media documents to (temporarily) play inline
https://bugs.webkit.org/show_bug.cgi?id=147181
Summary
[iOS] Add an explicit API to allow media documents to (temporarily) play inline
Jer Noble
Reported
2015-07-21 19:08:30 PDT
[iOS] Add an explicit API to allow media documents to (temporarily) play inline
Attachments
Patch
(15.72 KB, patch)
2015-07-21 19:42 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(15.79 KB, patch)
2015-07-22 16:23 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(15.71 KB, patch)
2015-07-22 16:52 PDT
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(15.78 KB, patch)
2015-07-23 12:35 PDT
,
Jer Noble
bdakin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Jer Noble
Comment 1
2015-07-21 19:42:13 PDT
Created
attachment 257240
[details]
Patch
Darin Adler
Comment 2
2015-07-22 14:13:53 PDT
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.
Jer Noble
Comment 3
2015-07-22 15:54:42 PDT
(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.
Jer Noble
Comment 4
2015-07-22 16:23:17 PDT
Created
attachment 257307
[details]
Patch
Jer Noble
Comment 5
2015-07-22 16:24:28 PDT
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.
Jer Noble
Comment 6
2015-07-22 16:52:58 PDT
Created
attachment 257308
[details]
Patch
Jer Noble
Comment 7
2015-07-23 12:35:37 PDT
Created
attachment 257363
[details]
Patch
Beth Dakin
Comment 8
2015-07-23 14:04:56 PDT
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
Jer Noble
Comment 9
2015-07-23 14:16:52 PDT
Committed
r187251
: <
http://trac.webkit.org/changeset/187251
>
Alex Christensen
Comment 10
2015-07-23 15:31:57 PDT
It looks like you had partially renamed flag to allows. Build fix committed to
http://trac.webkit.org/changeset/187263
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug