| Summary: | [iOS] Disable autoplay of silent videos in low power mode | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||||||||||||||||||||||
| Component: | Media | Assignee: | Chris Dumez <cdumez> | ||||||||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | barraclough, buildbot, commit-queue, eric.carlson, graouts, jeremyj-wk, jer.noble, jond, jonlee, rniwa, webkit-bug-importer | ||||||||||||||||||||||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=216887 | ||||||||||||||||||||||||||||||||||||
| Bug Depends on: | 168837 | ||||||||||||||||||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||
|
Description
Chris Dumez
2017-02-28 12:54:21 PST
Created attachment 302979 [details]
WIP Patch
Created attachment 302989 [details]
WIP Patch
Created attachment 303068 [details]
WIP Patch
Created attachment 303071 [details]
WIP Patch
Attachment 303071 [details] did not pass style-queue:
ERROR: LayoutTests/platform/ios-simulator/TestExpectations:2734: Path does not exist. [test/expectations] [5]
ERROR: LayoutTests/platform/ios-simulator/TestExpectations:2735: Path does not exist. [test/expectations] [5]
Total errors found: 2 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 303071 [details] WIP Patch Attachment 303071 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3216665 New failing tests: media/video-autoplay-lowPowerMode.html media/video-autoplay-lowPowerMode-with-controls.html Created attachment 303082 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 303084 [details]
WIP Patch
Created attachment 303085 [details]
WIP Patch
Comment on attachment 303085 [details] WIP Patch Attachment 303085 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3217203 New failing tests: http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html media/modern-media-controls/media-controller/media-controller-compact.html media/modern-media-controls/volume-support/volume-support-drag.html media/modern-media-controls/scrubber-support/scrubber-support-click.html media/modern-media-controls/fullscreen-support/fullscreen-support-click.html media/modern-media-controls/playback-support/playback-support-button-click.html media/modern-media-controls/start-support/start-support-autoplay.html media/modern-media-controls/playback-support/playback-support-autoplay.html media/modern-media-controls/status-support/status-support-playing.html media/modern-media-controls/scrubber-support/scrubber-support-drag.html media/modern-media-controls/volume-support/volume-support-click.html http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html Created attachment 303096 [details]
Archive of layout-test-results from ews100 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 303085 [details] WIP Patch Attachment 303085 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3217253 New failing tests: http/tests/media/modern-media-controls/skip-back-support/skip-back-support-live-broadcast.html media/modern-media-controls/media-controller/media-controller-compact.html media/modern-media-controls/volume-support/volume-support-drag.html media/modern-media-controls/scrubber-support/scrubber-support-click.html media/modern-media-controls/fullscreen-support/fullscreen-support-click.html media/modern-media-controls/playback-support/playback-support-button-click.html media/modern-media-controls/volume-support/volume-support-click.html media/modern-media-controls/playback-support/playback-support-autoplay.html media/modern-media-controls/status-support/status-support-playing.html media/modern-media-controls/scrubber-support/scrubber-support-drag.html media/modern-media-controls/start-support/start-support-autoplay.html http/tests/media/modern-media-controls/status-support/status-support-live-broadcast.html Created attachment 303098 [details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 303101 [details]
WIP Patch
Comment on attachment 303101 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=303101&action=review > Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl:51 > + readonly attribute boolean shouldForceControlsDisplay; Weird: CONSOLE MESSAGE: line 66: TypeError: null is not an object (evaluating 'this.mediaController.host.shouldForceControlsDisplay') in some layout tests. Created attachment 303105 [details]
WIP Patch
(In reply to comment #16) > Comment on attachment 303101 [details] > WIP Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=303101&action=review > > > Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl:51 > > + readonly attribute boolean shouldForceControlsDisplay; > > Weird: > CONSOLE MESSAGE: line 66: TypeError: null is not an object (evaluating > 'this.mediaController.host.shouldForceControlsDisplay') in some layout tests. It seems this.mediaController.host can be null. Other places using it have null checks so I'll do the same. (In reply to comment #18) > (In reply to comment #16) > > Comment on attachment 303101 [details] > > WIP Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=303101&action=review > > > > > Source/WebCore/Modules/mediacontrols/MediaControlsHost.idl:51 > > > + readonly attribute boolean shouldForceControlsDisplay; > > > > Weird: > > CONSOLE MESSAGE: line 66: TypeError: null is not an object (evaluating > > 'this.mediaController.host.shouldForceControlsDisplay') in some layout tests. > > It seems this.mediaController.host can be null. Other places using it have > null checks so I'll do the same. That's right, you need to expect the MediaControlsHost may be null, some tests do not use media elements at all and pass in a null host to the MediaController constructor. Created attachment 303112 [details]
WIP Patch
Created attachment 303207 [details]
Patch
Comment on attachment 303207 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=303207&action=review JS part looks good. > Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67 > + const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay)); Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)` Comment on attachment 303207 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=303207&action=review >> Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67 >> + const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay)); > > Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)` FYI, I tried but it is not work: FAIL mediaController.controls.controlsBar.visible should be false (of type boolean). Was null (of type object). Code looked like: const shouldShowControls = (!!media.controls || (host && host.shouldForceControlsDisplay)); (In reply to comment #23) > Comment on attachment 303207 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=303207&action=review > > >> Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67 > >> + const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay)); > > > > Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)` > > FYI, I tried but it is not work: > FAIL mediaController.controls.controlsBar.visible should be false (of type > boolean). Was null (of type object). > > Code looked like: > const shouldShowControls = (!!media.controls || (host && > host.shouldForceControlsDisplay)); *did* not work. (In reply to comment #24) > (In reply to comment #23) > > Comment on attachment 303207 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=303207&action=review > > > > >> Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js:67 > > >> + const shouldShowControls = !!(media.controls || (host && host.shouldForceControlsDisplay)); > > > > > > Since the second part of the statement is guaranteed to return a boolean, I think you can simply do `!!media.controls || (host && host.shouldForceControlsDisplay)` > > > > FYI, I tried but it is not work: > > FAIL mediaController.controls.controlsBar.visible should be false (of type > > boolean). Was null (of type object). > > > > Code looked like: > > const shouldShowControls = (!!media.controls || (host && > > host.shouldForceControlsDisplay)); > > *did* not work. It impacted tests like those: media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle.html media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-off-audio.html Comment on attachment 303207 [details] Patch Attachment 303207 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3223188 New failing tests: media/modern-media-controls/start-support/start-support-lowPowerMode.html Created attachment 303217 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 303207 [details] Patch Attachment 303207 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3223235 New failing tests: media/modern-media-controls/start-support/start-support-lowPowerMode.html Created attachment 303221 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 303223 [details]
Patch
Comment on attachment 303223 [details] Patch Clearing flags on attachment: 303223 Committed r213460: <http://trac.webkit.org/changeset/213460> All reviewed patches have been landed. Closing bug. |