Bug 158815 - [iOS] Media controls are too cramped with small video
Summary: [iOS] Media controls are too cramped with small video
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Other
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on: 159169 159302
Blocks:
  Show dependency treegraph
 
Reported: 2016-06-15 15:30 PDT by Antoine Quint
Modified: 2016-06-30 12:11 PDT (History)
6 users (show)

See Also:


Attachments
Testcase (614 bytes, text/html)
2016-06-15 15:30 PDT, Antoine Quint
no flags Details
Screenshot (101.17 KB, image/png)
2016-06-15 15:30 PDT, Antoine Quint
no flags Details
Patch (10.33 KB, patch)
2016-06-21 08:11 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (1.11 MB, application/zip)
2016-06-21 09:00 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (1.14 MB, application/zip)
2016-06-21 09:02 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews113 for mac-yosemite (1.71 MB, application/zip)
2016-06-21 09:14 PDT, Build Bot
no flags Details
Patch (14.43 KB, patch)
2016-06-24 08:12 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-yosemite (899.84 KB, application/zip)
2016-06-24 08:57 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (902.39 KB, application/zip)
2016-06-24 09:00 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews116 for mac-yosemite (1.53 MB, application/zip)
2016-06-24 09:08 PDT, Build Bot
no flags Details
Patch (14.45 KB, patch)
2016-06-24 09:22 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (18.34 KB, patch)
2016-06-27 05:40 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (18.69 KB, patch)
2016-06-30 03:35 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (18.69 KB, patch)
2016-06-30 03:37 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (18.68 KB, patch)
2016-06-30 07:39 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (20.14 KB, patch)
2016-06-30 09:41 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2016-06-15 15:30:35 PDT
Created attachment 281393 [details]
Testcase

Opening up the attached test file in landscape on a 4" iPhone shows cramped controls.
Comment 1 Antoine Quint 2016-06-15 15:30:52 PDT
Created attachment 281394 [details]
Screenshot
Comment 2 Radar WebKit Bug Importer 2016-06-15 15:33:34 PDT
<rdar://problem/26824238>
Comment 3 Antoine Quint 2016-06-21 08:11:39 PDT
Created attachment 281747 [details]
Patch
Comment 4 Antoine Quint 2016-06-21 08:13:12 PDT
No tests yet, I'm having timeouts on all tests with the test runner (see rdar://problem/26918438). Feedback welcome on the source changes so far though. I will update the patch with tests when I have a working test runner.
Comment 5 Build Bot 2016-06-21 09:00:03 PDT
Comment on attachment 281747 [details]
Patch

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

New failing tests:
media/media-document-audio-repaint.html
media/controls-strict.html
media/video-volume-slider.html
media/controls-styling.html
fast/hidpi/video-controls-in-hidpi.html
media/video-display-toggle.html
media/media-controls-drag-timeline-set-controls-property.html
media/controls-drag-timebar.html
media/video-controls-rendering.html
accessibility/media-element.html
media/controls-without-preload.html
media/media-controls-clone.html
fast/layers/video-layer.html
media/video-empty-source.html
media/track/track-cue-rendering-horizontal.html
http/tests/media/hls/video-controls-live-stream.html
media/controls-after-reload.html
Comment 6 Build Bot 2016-06-21 09:00:06 PDT
Created attachment 281749 [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
Comment 7 Build Bot 2016-06-21 09:02:09 PDT
Comment on attachment 281747 [details]
Patch

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

New failing tests:
media/media-document-audio-repaint.html
media/controls-strict.html
media/controls-drag-timebar.html
media/video-volume-slider.html
media/video-display-toggle.html
media/media-controls-drag-timeline-set-controls-property.html
fast/hidpi/video-controls-in-hidpi.html
media/video-controls-rendering.html
accessibility/media-element.html
media/controls-without-preload.html
media/media-controls-clone.html
fast/layers/video-layer.html
media/video-empty-source.html
media/track/track-cue-rendering-horizontal.html
http/tests/media/hls/video-controls-live-stream.html
http/tests/contentextensions/text-track-blocked.html
Comment 8 Build Bot 2016-06-21 09:02:12 PDT
Created attachment 281750 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 9 Build Bot 2016-06-21 09:14:45 PDT
Comment on attachment 281747 [details]
Patch

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

New failing tests:
media/media-document-audio-repaint.html
media/controls-strict.html
media/video-volume-slider.html
media/controls-styling.html
fast/hidpi/video-controls-in-hidpi.html
media/video-display-toggle.html
media/media-controls-drag-timeline-set-controls-property.html
media/controls-drag-timebar.html
media/video-controls-rendering.html
accessibility/media-element.html
media/controls-without-preload.html
media/media-controls-clone.html
fast/layers/video-layer.html
media/video-empty-source.html
media/track/track-cue-rendering-horizontal.html
http/tests/media/hls/video-controls-live-stream.html
media/controls-after-reload.html
Comment 10 Build Bot 2016-06-21 09:14:48 PDT
Created attachment 281751 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 11 Dean Jackson 2016-06-23 11:18:23 PDT
Comment on attachment 281747 [details]
Patch

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

> Source/WebCore/Modules/mediacontrols/mediaControlsApple.js:1318
>          if (duration >= 60*60*10) {
> -            this.controls.currentTime.classList.add(this.ClassNames.sixDigitTime);
> -            this.controls.remainingTime.classList.add(this.ClassNames.sixDigitTime);
> +            addTimeClass(this.ClassNames.sixDigitTime);
>          } else if (duration >= 60*60) {
> -            this.controls.currentTime.classList.add(this.ClassNames.fiveDigitTime);
> -            this.controls.remainingTime.classList.add(this.ClassNames.fiveDigitTime);
> +            addTimeClass(this.ClassNames.fiveDigitTime);
>          } else if (duration >= 60*10) {
> -            this.controls.currentTime.classList.add(this.ClassNames.fourDigitTime);
> -            this.controls.remainingTime.classList.add(this.ClassNames.fourDigitTime);
> +            addTimeClass(this.ClassNames.fourDigitTime);
>          } else {
> -            this.controls.currentTime.classList.add(this.ClassNames.threeDigitTime);
> -            this.controls.remainingTime.classList.add(this.ClassNames.threeDigitTime);
> +            addTimeClass(this.ClassNames.threeDigitTime);
>          }

These all become single line conditionals.
Comment 12 Antoine Quint 2016-06-24 07:27:17 PDT
So some of the failures (maybe all) come from the fact that the scrubber won't display now for the default width of an <audio> or <video> element (300 CSS pixels). That is not ideal in terms of user experience!
Comment 13 Antoine Quint 2016-06-24 08:12:15 PDT
Created attachment 281976 [details]
Patch
Comment 14 Build Bot 2016-06-24 08:56:58 PDT
Comment on attachment 281976 [details]
Patch

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

New failing tests:
media/track/track-cue-rendering-horizontal.html
Comment 15 Build Bot 2016-06-24 08:57:00 PDT
Created attachment 281978 [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
Comment 16 Build Bot 2016-06-24 09:00:04 PDT
Comment on attachment 281976 [details]
Patch

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

New failing tests:
media/track/track-cue-rendering-horizontal.html
http/tests/contentextensions/text-track-blocked.html
Comment 17 Build Bot 2016-06-24 09:00:06 PDT
Created attachment 281979 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5
Comment 18 Build Bot 2016-06-24 09:08:42 PDT
Comment on attachment 281976 [details]
Patch

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

New failing tests:
media/track/track-cue-rendering-horizontal.html
Comment 19 Build Bot 2016-06-24 09:08:45 PDT
Created attachment 281982 [details]
Archive of layout-test-results from ews116 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 20 Antoine Quint 2016-06-24 09:22:24 PDT
Created attachment 281984 [details]
Patch
Comment 21 Antoine Quint 2016-06-27 05:40:49 PDT
Created attachment 282124 [details]
Patch
Comment 22 WebKit Commit Bot 2016-06-27 12:07:04 PDT
Comment on attachment 282124 [details]
Patch

Clearing flags on attachment: 282124

Committed r202505: <http://trac.webkit.org/changeset/202505>
Comment 23 WebKit Commit Bot 2016-06-27 12:07:09 PDT
All reviewed patches have been landed.  Closing bug.
Comment 24 Ryan Haddad 2016-06-27 14:32:00 PDT
The test added with change is very flaky:

media/video-controls-drop-and-restore-timeline.html
<https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fvideo-controls-drop-and-restore-timeline.html>

It also appears to have caused media/controls-drag-timebar.html to time out on El Capitan WK2
<https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fcontrols-drag-timebar.html>
Comment 25 WebKit Commit Bot 2016-06-27 14:41:47 PDT
Re-opened since this is blocked by bug 159169
Comment 26 Antoine Quint 2016-06-30 03:35:59 PDT
Created attachment 282427 [details]
Patch for landing
Comment 27 Antoine Quint 2016-06-30 03:37:09 PDT
Created attachment 282428 [details]
Patch
Comment 28 Eric Carlson 2016-06-30 07:18:48 PDT
Comment on attachment 282428 [details]
Patch

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

> LayoutTests/media/video-controls-drop-and-restore-timeline.html:17
> +                return child.classList.contains('dropped');

Minor nit: You use double quotes almost everywhere else, you may as well be consistent.

> LayoutTests/media/video-controls-drop-and-restore-timeline.html:27
> +        testExpected("video.controls", null, '!=');

Ditto.

> LayoutTests/media/video-controls-drop-and-restore-timeline.html:37
> +            testExpected("timelineContainer = mediaControlsElement(shadowRoot.firstChild, '-webkit-media-controls-timeline-container')", null, "!=");

Minor nit: you could make the first string a template literal.
Comment 29 Antoine Quint 2016-06-30 07:37:55 PDT
Thanks Eric, these will be all addressed when landing.
Comment 30 Antoine Quint 2016-06-30 07:39:54 PDT
Created attachment 282441 [details]
Patch for landing
Comment 31 WebKit Commit Bot 2016-06-30 08:09:59 PDT
Comment on attachment 282441 [details]
Patch for landing

Clearing flags on attachment: 282441

Committed r202679: <http://trac.webkit.org/changeset/202679>
Comment 32 WebKit Commit Bot 2016-06-30 08:10:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 33 WebKit Commit Bot 2016-06-30 09:18:12 PDT
Re-opened since this is blocked by bug 159302
Comment 34 Alexey Proskuryakov 2016-06-30 09:18:42 PDT
This is still causing timeouts on media/controls-drag-timebar.html. Antoine could reproduce locally, rolling out.
Comment 35 Antoine Quint 2016-06-30 09:41:48 PDT
Created attachment 282447 [details]
Patch
Comment 36 WebKit Commit Bot 2016-06-30 12:11:01 PDT
Comment on attachment 282447 [details]
Patch

Clearing flags on attachment: 282447

Committed r202694: <http://trac.webkit.org/changeset/202694>
Comment 37 WebKit Commit Bot 2016-06-30 12:11:08 PDT
All reviewed patches have been landed.  Closing bug.