Closed Bug 1653390 Opened 3 months ago Closed 2 months ago

Inaudible media should not be affected by media key

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla80
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- wontfix
firefox79 --- wontfix
firefox80 --- verified

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 1 open bug, Regression, )

Details

(Keywords: regression)

Attachments

(7 files, 3 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

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

Duplicate of this bug: 1654026
Attachment #9165083 - Attachment description: Bug 1653390 - part8 : add test cases for enabling muted media into fullscreen/PIP mode. → Bug 1653390 - part10 : add test cases for enabling muted media into fullscreen/PIP mode.
Attachment #9164635 - Attachment description: Bug 1653390 - part7 : add test for a page mixing audible and inaudible media. → Bug 1653390 - part9 : add test for a page mixing audible and inaudible media.
Attachment #9164634 - Attachment description: Bug 1653390 - part6 : start listener when media enters PIP mode. → Bug 1653390 - part8 : start listener when media enters PIP mode.
Attachment #9164633 - Attachment description: Bug 1653390 - part5 : start listener when media enters fullscreen. → Bug 1653390 - part7 : start listener when media enters fullscreen.
Attachment #9164630 - Attachment description: Bug 1653390 - part2 : add audible check back in media element. → Bug 1653390 - part6 : add audible check back in media element.
Attachment #9164632 - Attachment description: Bug 1653390 - part4 : call 'Stop()' directly. → Bug 1653390 - part5 : remove 'StopListeningMediaControlKeyIfNeeded()'.
Attachment #9164631 - Attachment description: Bug 1653390 - part3 : always hold a MediaControlKeyListener until media dies. → Bug 1653390 - part4 : always hold a MediaControlKeyListener until media dies.

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).

Blocks: 1654959

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.

Attachment #9165083 - Attachment is obsolete: true

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.

Attachment #9164634 - Attachment is obsolete: true

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.

Attachment #9164633 - Attachment is obsolete: true
Attachment #9164635 - Attachment description: Bug 1653390 - part9 : add test for a page mixing audible and inaudible media. → Bug 1653390 - part7 : add test for a page mixing audible and inaudible media.
Pushed by alwu@mozilla.com:
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
Regressions: 1655175
Flags: qe-verify+

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

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.