Bug 159967 – [iPhone] Playing a video on tudou.com plays only sound, no video

• Component: New Bugs
• Status: RESOLVED FIXED

[Myles C. Maxfield]

[iPhone] Playing a video on tudou.com plays only sound, no video

[Myles C. Maxfield]

Created attachment 284091 [details]
WIP

[WebKit Commit Bot]

Attachment 284091 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 284091 [details]
WIP

Attachment 284091 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1713223

New failing tests:
media/video-playsinline.html

[Build Bot]

Created attachment 284104 [details]
Archive of layout-test-results from ews102 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 284091 [details]
WIP

Attachment 284091 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1713240

New failing tests:
media/video-playsinline.html

[Build Bot]

Created attachment 284105 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Myles C. Maxfield]

Created attachment 284110 [details]
WIP

[WebKit Commit Bot]

Attachment 284110 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:9:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 284110 [details]
WIP

Attachment 284110 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1714417

New failing tests:
media/video-playsinline.html

[Build Bot]

Created attachment 284114 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Jon Lee]

rdar://problem/26964090

[Myles C. Maxfield]

Created attachment 284121 [details]
WIP

[Jon Lee]

Comment on attachment 284121 [details]
WIP

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

> Source/WebCore/html/MediaElementSession.cpp:458
> +    bool requiresPlaysInline = settings->fullscreenMediaPlaybackRequiresPlaysInlineAttributeAbsence();

The phrase construction is awkward. I understand how there could be confusion with two "requires"--how about allowsInlineMediaPlaybackWithPlaysInlineAttribute and allowsInlineMediaPlaybackWithWebKitPlaysInlineAttribute

[Jon Lee]

Comment on attachment 284121 [details]
WIP

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

>> Source/WebCore/html/MediaElementSession.cpp:458
>> +    bool requiresPlaysInline = settings->fullscreenMediaPlaybackRequiresPlaysInlineAttributeAbsence();
> 
> The phrase construction is awkward. I understand how there could be confusion with two "requires"--how about allowsInlineMediaPlaybackWithPlaysInlineAttribute and allowsInlineMediaPlaybackWithWebKitPlaysInlineAttribute

By two "requires" I meant keeping the "inlineMediaPlaybackRequires" construct.

[Myles C. Maxfield]

Created attachment 284138 [details]
Patch

[Myles C. Maxfield]

Created attachment 284142 [details]
Patch

[Jeremy Jones]

The new property name is much better.
*Absence version of the property name still appears in many places. Use the new name everywhere.

[Myles C. Maxfield]

Created attachment 284144 [details]
Patch

[Jeremy Jones]

Comment on attachment 284144 [details]
Patch

LGTM

[Jon Lee]

Comment on attachment 284144 [details]
Patch

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

> Source/WebKit2/UIProcess/API/C/WKPreferencesRefPrivate.h:198
> +// Defaults to false.

Not "true on iOS, false elsewhere."?

> Source/WebKit2/UIProcess/API/C/WKPreferencesRefPrivate.h:202
> +// Defaults to false.

Ditto

[Myles C. Maxfield]

Committed r203471: <http://trac.webkit.org/changeset/203471>

[Myles C. Maxfield]

Mitz pointed out that, because this touches WK2, it needs to be reviewed by an owner.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 160003

[Myles C. Maxfield]

Created attachment 284183 [details]
Needs WK2 Review

[Anders Carlsson]

Comment on attachment 284183 [details]
Needs WK2 Review

WK2 part looks fine.

[Myles C. Maxfield]

Comment on attachment 284183 [details]
Needs WK2 Review

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

> Source/WebKit2/UIProcess/API/C/WKPreferencesRefPrivate.h:198
> +// Defaults to false.

I need to re-apply Jon's comment "true on iOS, false elsewhere."

> Source/WebKit2/UIProcess/API/C/WKPreferencesRefPrivate.h:202
> +// Defaults to false.

Ditto.

[Myles C. Maxfield]

Created attachment 284246 [details]
Patch for committing

[Myles C. Maxfield]

Created attachment 284247 [details]
Patch for committing

[WebKit Commit Bot]

Comment on attachment 284247 [details]
Patch for committing

Clearing flags on attachment: 284247

Committed r203520: <http://trac.webkit.org/changeset/203520>

[Myles C. Maxfield]

Committed r203540: <http://trac.webkit.org/changeset/203540>