1653390 - Inaudible media should not be affected by media key

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Alastor Wu [:alwu]]

This is caused by bug1627999, the inaudible media shouldn't be controlled by media control.

STR.

1. go to https://alastor0325.github.io/htmltests/autoplay_tests/non-audible-test/autoplay_test_muted_mix_audible.html
2. start both media
3. press media key "pause"

Expect.
4. only audible media is being paused

Actual.
4. both media are being paused

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part1 : remove start check.

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part6 : add audible check back in media element.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part4 : always hold a MediaControlKeyListener until media dies.

Comment 4

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part5 : remove 'StopListeningMediaControlKeyIfNeeded()'.

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part7 : start listener when media enters fullscreen.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part8 : start listener when media enters PIP mode.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part7 : add test for a page mixing audible and inaudible media.

Comment 9

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part10 : add test cases for enabling muted media into fullscreen/PIP mode.

Comment 10

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part3 : wrap the detail of updating playback state into Start().

Comment 11

[Alastor Wu [:alwu]]

Attachment: Bug 1653390 - part2: allow calling 'UpdateMediaAudibleState()' before the listener starts.

Comment 12

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Working through review.

I don't feel strongly, but suggest moving parts 7, 8, and 10 to another bug as they're not specifically related to the audibility of an element. This could help with finding regressions and attribution. Alternatively, if you think regression is unlikely, I suggest renaming this bug to make it clear the scope is greater than just audibility. It could be named something like "Prioritize full screen and pip elements for media keys, do not use media keys on inaudible media unless it's fullscreen or pip" (bit long, but the best I could think of).

Comment 13

[Phabricator Automation]

Comment on attachment 9165083 [details]
Bug 1653390 - part10 : add test cases for enabling muted media into fullscreen/PIP mode.

Revision D84369 was moved to bug 1654959. Setting attachment 9165083 [details] to obsolete.

Comment 14

[Phabricator Automation]

Comment on attachment 9164634 [details]
Bug 1653390 - part8 : start listener when media enters PIP mode.

Revision D84119 was moved to bug 1654959. Setting attachment 9164634 [details] to obsolete.

Comment 15

[Phabricator Automation]

Comment on attachment 9164633 [details]
Bug 1653390 - part7 : start listener when media enters fullscreen.

Revision D84118 was moved to bug 1654959. Setting attachment 9164633 [details] to obsolete.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2dc8fd47bc2
part1 : remove start check. r=bryce
https://hg.mozilla.org/integration/autoland/rev/f42e3e5a600d
part2: allow calling 'UpdateMediaAudibleState()' before the listener starts. r=bryce
https://hg.mozilla.org/integration/autoland/rev/448c6633265d
part3 : wrap the detail of updating playback state into Start(). r=bryce
https://hg.mozilla.org/integration/autoland/rev/3bb773642909
part4 : always hold a MediaControlKeyListener until media dies. r=bryce
https://hg.mozilla.org/integration/autoland/rev/a8a44fb117cb
part5 : remove 'StopListeningMediaControlKeyIfNeeded()'. r=bryce
https://hg.mozilla.org/integration/autoland/rev/c4c53ba08359
part6 : add audible check back in media element. r=bryce
https://hg.mozilla.org/integration/autoland/rev/e7e5ac6a2036
part7 : add test for a page mixing audible and inaudible media. r=bryce

Comment 17

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/e2dc8fd47bc2
https://hg.mozilla.org/mozilla-central/rev/f42e3e5a600d
https://hg.mozilla.org/mozilla-central/rev/448c6633265d
https://hg.mozilla.org/mozilla-central/rev/3bb773642909
https://hg.mozilla.org/mozilla-central/rev/a8a44fb117cb
https://hg.mozilla.org/mozilla-central/rev/c4c53ba08359
https://hg.mozilla.org/mozilla-central/rev/e7e5ac6a2036

Comment 18

[Cristian Fogel [:cfogel]]

Confirmed issue with 80.0a1 (2020-07-16) on Windows 10.
Fix verified with 80.0a1 (2020-07-24) .