Bug 167836 - [Modern Media Controls] Improve handling of <video> with only audio tracks
Summary: [Modern Media Controls] Improve handling of <video> with only audio tracks
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
: 167976 (view as bug list)
Depends on: 168506
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-04 06:35 PST by Antoine Quint
Modified: 2017-02-17 13:50 PST (History)
5 users (show)

See Also:


Attachments
Patch (7.22 KB, patch)
2017-02-05 13:32 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (7.22 KB, patch)
2017-02-05 13:41 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-cq-03 for mac-elcapitan (1.01 MB, application/zip)
2017-02-05 18:44 PST, WebKit Commit Bot
no flags Details
Patch for landing (12.38 KB, patch)
2017-02-06 02:53 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (14.21 KB, patch)
2017-02-06 03:22 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (901.70 KB, application/zip)
2017-02-06 04:53 PST, Build Bot
no flags Details
Patch for landing (15.49 KB, patch)
2017-02-06 05:11 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (16.09 KB, patch)
2017-02-07 02:17 PST, 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 2017-02-04 06:35:52 PST
A <video> element that doesn't contain any video tracks still turns on auto-hide for the controls and shows a fullscreen button. This shouldn't be the case.
Comment 1 Antoine Quint 2017-02-04 06:36:07 PST
<rdar://problem/30255812>
Comment 2 Antoine Quint 2017-02-05 13:32:44 PST
Created attachment 300675 [details]
Patch
Comment 3 Dean Jackson 2017-02-05 13:38:10 PST
Comment on attachment 300675 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        We now check for the availability of video tracks before considerig a <video>

Typo: considering

> Source/WebCore/ChangeLog:13
> +        to be the same as an <audio> element.

Did you check this with a media document? Which always uses video, even for audio content.
Comment 4 Antoine Quint 2017-02-05 13:40:40 PST
(In reply to comment #3)
> Comment on attachment 300675 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=300675&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        We now check for the availability of video tracks before considerig a <video>
> 
> Typo: considering
> 
> > Source/WebCore/ChangeLog:13
> > +        to be the same as an <audio> element.
> 
> Did you check this with a media document? Which always uses video, even for
> audio content.

Yes, that's how I found out about this bug :)
Comment 5 Antoine Quint 2017-02-05 13:41:41 PST
Created attachment 300676 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2017-02-05 14:30:22 PST
The commit-queue encountered the following flaky tests while processing attachment 300676 [details]:

The commit-queue is continuing to process your patch.
Comment 7 WebKit Commit Bot 2017-02-05 18:44:43 PST
Comment on attachment 300676 [details]
Patch for landing

Rejecting attachment 300676 [details] from commit-queue.

New failing tests:
media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on.html
media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle.html
fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.html
Full output: http://webkit-queues.webkit.org/results/3011053
Comment 8 WebKit Commit Bot 2017-02-05 18:44:45 PST
Created attachment 300681 [details]
Archive of layout-test-results from webkit-cq-03 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-03  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 9 Antoine Quint 2017-02-06 02:53:51 PST
Created attachment 300702 [details]
Patch for landing
Comment 10 Antoine Quint 2017-02-06 03:22:17 PST
Created attachment 300703 [details]
Patch for landing
Comment 11 Build Bot 2017-02-06 04:53:25 PST
Comment on attachment 300703 [details]
Patch for landing

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

New failing tests:
media/modern-media-controls/time-label/time-label-white-space-nowrap.html
Comment 12 Build Bot 2017-02-06 04:53:28 PST
Created attachment 300709 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 13 Antoine Quint 2017-02-06 05:11:49 PST
Created attachment 300711 [details]
Patch for landing
Comment 14 WebKit Commit Bot 2017-02-06 05:49:41 PST
Comment on attachment 300711 [details]
Patch for landing

Clearing flags on attachment: 300711

Committed r211722: <http://trac.webkit.org/changeset/211722>
Comment 15 WebKit Commit Bot 2017-02-06 05:49:45 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 Ryan Haddad 2017-02-06 09:26:04 PST
(In reply to comment #14)
> Comment on attachment 300711 [details]
> Patch for landing
> 
> Clearing flags on attachment: 300711
> 
> Committed r211722: <http://trac.webkit.org/changeset/211722>

This change caused fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.html to fail on mac-wk2:

https://build.webkit.org/results/Apple%20Sierra%20Release%20WK2%20(Tests)/r211724%20(3315)/results.html

 https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fregions%2Finline-block-inside-anonymous-overflow-with-covered-controls.html
Comment 17 Ryan Haddad 2017-02-06 09:29:37 PST
Reverted r211722 for reason:

This change introduced a LayoutTest failure on mac-wk2.

Committed r211729: <http://trac.webkit.org/changeset/211729>
Comment 18 Antoine Quint 2017-02-07 01:56:10 PST
This is an image reference mismatch issue. Investigating.
Comment 19 Antoine Quint 2017-02-07 02:17:48 PST
Created attachment 300797 [details]
Patch for landing
Comment 20 WebKit Commit Bot 2017-02-07 02:55:25 PST
Comment on attachment 300797 [details]
Patch for landing

Clearing flags on attachment: 300797

Committed r211802: <http://trac.webkit.org/changeset/211802>
Comment 21 WebKit Commit Bot 2017-02-07 02:55:30 PST
All reviewed patches have been landed.  Closing bug.
Comment 22 Ryan Haddad 2017-02-09 13:33:11 PST
Reverted r211802 for reason:

This change caused fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.html to be a flaky failure.

Committed r211986: <http://trac.webkit.org/changeset/211986>
Comment 23 Ryan Haddad 2017-02-09 13:33:52 PST
(In reply to comment #22)
> Reverted r211802 for reason:
> 
> This change caused
> fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.
> html to be a flaky failure.
> 
> Committed r211986: <http://trac.webkit.org/changeset/211986>

See https://bugs.webkit.org/show_bug.cgi?id=167976
Comment 24 Ryan Haddad 2017-02-09 13:33:55 PST
*** Bug 167976 has been marked as a duplicate of this bug. ***
Comment 25 Antoine Quint 2017-02-17 05:43:40 PST
Fixing https://bugs.webkit.org/show_bug.cgi?id=168506 will address the flakiness of fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.html without any further source changes. Will re-land once that bug is fixed.
Comment 26 Antoine Quint 2017-02-17 13:50:00 PST
Committed r212575: <http://trac.webkit.org/changeset/212575>