Bug 411742

Summary: Audio doesn't play when Enhanced Media Controls is enabled on specific websites
Product: [Plasma] plasma-browser-integration Reporter: Frederick Zhang <frederick888>
Component: FirefoxAssignee: Kai Uwe Broulik <kde>
Status: RESOLVED FIXED    
Severity: normal CC: adia, tasos, xp-1000
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:

Description Frederick Zhang 2019-09-09 07:18:51 UTC
STEPS TO REPRODUCE
1. Enable 'Enhanced Media Controls' in Firefox add-on preferences
2. Open https://dictionary.cambridge.org/dictionary/english/example
3. Click the pronunciation button

OBSERVED RESULT
No sound.

EXPECTED RESULT
The pronunciation audio plays normally.

SOFTWARE/OS VERSIONS
Operating System: Arch Linux 
KDE Plasma Version: 5.16.5
KDE Frameworks Version: 5.61.0
Qt Version: 5.13.0
Kernel Version: 5.2.13-arch1-1-ARCH
OS Type: 64-bit

ADDITIONAL INFORMATION
I'm using Latte Dock instead of the default task manager.
Comment 1 Frederick Zhang 2019-09-16 06:30:13 UTC
Just noticed that Cambridge Dictionary has rolled out a new UI and this issue is no longer reproducible on their website.

So I created a simple fiddle for this: https://jsfiddle.net/h78m6r0q/
Comment 2 Alexandros Diamantidis 2019-10-22 15:03:03 UTC
I have the same problem with the pronunciations at https://forvo.com/
Comment 3 Kai Uwe Broulik 2019-11-07 10:26:35 UTC
*** Bug 413817 has been marked as a duplicate of this bug. ***
Comment 4 Kai Uwe Broulik 2019-11-10 17:53:59 UTC
*** Bug 412418 has been marked as a duplicate of this bug. ***
Comment 5 Kai Uwe Broulik 2019-11-10 19:02:34 UTC
I would really appreciate some feedback and testing on this patch: https://phabricator.kde.org/D24870
I tried all of your links (and some others) which seem to work well now, thanks a lot for your feedback and help!
Comment 6 Tasos Sahanidis 2019-11-11 21:49:11 UTC
I applied that patch on top of the latest source from git and then loaded the addon, however the player on vgmrips.net is still not functioning in the same way as https://bugs.kde.org/show_bug.cgi?id=412418
Comment 7 Kai Uwe Broulik 2019-11-11 22:09:59 UTC
This works here with the patch I linked. Just patching the repository and installing it is not sufficient, the *browser* extension needs to be patched.

Please see the guide [1], paragraph "For using the extension from source code". 

[1] https://community.kde.org/Plasma/Browser_Integration#How_to_install
Comment 8 Kai Uwe Broulik 2019-11-12 07:55:03 UTC
Sorry, I accidentally uploaded an earlier version of the patch which didn't work.. I just updated it and now it should be working and also does so for that vgmrips.net you mentioned.
Comment 9 Frederick Zhang 2019-11-12 09:07:20 UTC
I'm afraid that the patch fixed https://forvo.com but not https://vgmrips.net and my jsfiddle.
Comment 10 Kai Uwe Broulik 2019-11-12 09:09:41 UTC
Anything unusual on dev console? All of the sites I mentioned in "Test Plan" of the patch do work here. Make sure to have the updated version of the patch from earlier this morning. I'm on Chrome 78
Comment 11 Frederick Zhang 2019-11-12 14:44:21 UTC
I'm using Firefox 71.0b8 (64-bit). Nothing unusual observed from web console, browser console or extension debugging console.

I ran
> curl -L https://phabricator.kde.org/D24870\?download\=true | git apply
...to get apply the patch so I think it should be the latest one?

Here's my git diff just in case:

diff --git a/extension/content-script.js b/extension/content-script.js
index 68dd9912..a8aa7faa 100644
--- a/extension/content-script.js
+++ b/extension/content-script.js
@@ -780,17 +780,47 @@ function loadMediaSessionsShim() {
         // HACK We cannot really pass variables from the page's scope to our content-script's scope
         // so we just blatantly insert the <audio> tag in the DOM and pick it up through our regular
         // mechanism. Let's see how this goes :D
 
+        // HACK When removing a media object from DOM it is paused, so what we do here is once the
+        // player loaded some data we add it (doesn't work earlier since it cannot pause when
+        // there's nothing loaded to pause) to the DOM and before we remove it, we note down that
+        // we will now get a paused event because of that. When we get it, we just play() the player
+        // so it continues playing :-)
+        const addPlayerToDomEvadingAutoPlayBlocking = `
+            player.registerInDom = () => {
+                player.pausedBecauseOfDomRemoval = true;
+                player.removeEventListener("play", player.registerInDom);
+
+                (document.head || document.documentElement).appendChild(player);
+                player.parentNode.removeChild(player);
+            };
+
+            player.replayAfterRemoval = () => {
+                if (player.pausedBecauseOfDomRemoval === true) {
+                    delete player.pausedBecauseOfDomRemoval;
+                    player.removeEventListener("pause", player.replyAfterRemoval);
+
+                    player.play();
+                }
+            };
+
+            player.addEventListener("play", player.registerInDom);
+            player.addEventListener("pause", player.replayAfterRemoval);
+        `;
+
         executeScript(`function() {
                 var oldCreateElement = Document.prototype.createElement;
                 Document.prototype.createElement = function() {
                     var createdTag = oldCreateElement.apply(this, arguments);
 
                     var tagName = arguments[0];
 
                     if (typeof tagName === "string") {
-                        if (tagName.toLowerCase() === "audio" || tagName.toLowerCase() === "video") {
+                        if (tagName.toLowerCase() === "audio") {
+                            const player = createdTag;
+                            ${addPlayerToDomEvadingAutoPlayBlocking}
+                        } else if (tagName.toLowerCase() === "video") {
                             (document.head || document.documentElement).appendChild(createdTag);
                             createdTag.parentNode.removeChild(createdTag);
                         }
                     }
@@ -802,13 +832,9 @@ function loadMediaSessionsShim() {
 
         // We also briefly add items created as new Audio() to the DOM so we can control it
         // similar to the document.createElement hack above since we cannot share variables
         // between the actual website and the background script despite them sharing the same DOM
-        // HACK When removing a media object from DOM it is paused, so what we do here is once the
-        // player loaded some data we add it (doesn't work earlier since it cannot pause when
-        // there's nothing loaded to pause) to the DOM and before we remove it, we note down that
-        // we will now get a paused event because of that. When we get it, we just play() the player
-        // so it continues playing :-)
+
         if (IS_FIREFOX) {
             // Firefox enforces Content-Security-Policy also for scripts injected by the content-script
             // This causes our executeScript calls to fail for pages like Nextcloud
             // It also doesn't seem to have the aggressive autoplay prevention Chrome has,
@@ -825,34 +851,12 @@ function loadMediaSessionsShim() {
             }, window, {defineAs: "Audio"});
         } else {
             executeScript(`function() {
                 var oldAudio = window.Audio;
-                window.Audio = function () {
-                    var createdAudio = new (Function.prototype.bind.apply(oldAudio, arguments));
-
-                    createdAudio.registerInDom = function() {
-                        (document.head || document.documentElement).appendChild(createdAudio);
-                        createdAudio.pausedBecauseOfDomRemoval = true;
-                        createdAudio.parentNode.removeChild(createdAudio);
-
-                        createdAudio.removeEventListener("loadeddata", createdAudio.registerInDom);
-                    };
-
-                    createdAudio.replayAfterRemoval = function() {
-                        if (createdAudio.pausedBecauseOfDomRemoval) {
-                            if (createdAudio.paused) {
-                                createdAudio.play();
-                            }
-                            delete createdAudio.pausedBecauseOfDomRemoval;
-
-                            createdAudio.removeEventListener("pause", createdAudio.replayAfterRemoval);
-                        }
-                    };
-
-                    createdAudio.addEventListener("loadeddata", createdAudio.registerInDom);
-                    createdAudio.addEventListener("pause", createdAudio.replayAfterRemoval);
-
-                    return createdAudio;
+                window.Audio = function (...args) {
+                    const player = new oldAudio(...args);
+                    ${addPlayerToDomEvadingAutoPlayBlocking}
+                    return player;
                 };
             }`);
         }
     }
Comment 12 Kai Uwe Broulik 2019-11-12 14:54:15 UTC
Ah, Firefox. Looks like Firefox now also has this autoplay prevention... okay, will fix. Thanks.
Comment 13 Kai Uwe Broulik 2019-11-12 15:02:11 UTC
Alright, please try again with the latest patch. Works fine here on Firefox 70
Comment 14 Frederick Zhang 2019-11-12 15:15:43 UTC
Yup, it worked with the new patch! Thanks!
Comment 15 Kai Uwe Broulik 2019-11-14 18:33:47 UTC
Git commit c1d7ea7346074e1912d56b9aed8d5ec47957e3a2 by Kai Uwe Broulik.
Committed on 14/11/2019 at 18:33.
Pushed by broulik into branch 'master'.

Fixup new Audio() constructor and apply autoplay prevention evasion to document.createElement

The arguments apparently weren't properly forwarded to the constructor,
breaking e.g. new Audio("foo") calls. This uses the much nicer spread syntax introduced in ES6.
Also, document.createElement("audio") is affected in the same way as new Audio()
The evasion is now only done when the player actually starts playing,
not immediately when it is created, since we only care about playing players.

Differential Revision: https://phabricator.kde.org/D24870

M  +40   -39   extension/content-script.js

https://commits.kde.org/plasma-browser-integration/c1d7ea7346074e1912d56b9aed8d5ec47957e3a2
Comment 16 Tasos Sahanidis 2019-11-18 02:38:30 UTC
(In reply to Kai Uwe Broulik from comment #15)
> Git commit c1d7ea7346074e1912d56b9aed8d5ec47957e3a2 by Kai Uwe Broulik.
> Committed on 14/11/2019 at 18:33.
> Pushed by broulik into branch 'master'.
> 
> Fixup new Audio() constructor and apply autoplay prevention evasion to
> document.createElement
> 
> The arguments apparently weren't properly forwarded to the constructor,
> breaking e.g. new Audio("foo") calls. This uses the much nicer spread syntax
> introduced in ES6.
> Also, document.createElement("audio") is affected in the same way as new
> Audio()
> The evasion is now only done when the player actually starts playing,
> not immediately when it is created, since we only care about playing players.
> 
> Differential Revision: https://phabricator.kde.org/D24870
> 
> M  +40   -39   extension/content-script.js
> 
> https://commits.kde.org/plasma-browser-integration/
> c1d7ea7346074e1912d56b9aed8d5ec47957e3a2

The patch seems to cause a regression with the Wikipedia audio player.

Clicking play does make it start playing, however it acts as if it hasn't started playing yet, thus showing the play icon instead of the pause icon while playing. It's impossible to pause the playback, seek, mute, and in general no controls work.

Clicking play more than once makes it play the same sound multiple times simultaneously.

Example URL: https://en.wikipedia.org/wiki/File:Thunder.ogg
Comment 17 Kai Uwe Broulik 2019-11-18 08:38:57 UTC
Git commit 47a2790188bf0a6d05fdbc94b6378f26514313ac by Kai Uwe Broulik.
Committed on 18/11/2019 at 08:38.
Pushed by broulik into branch 'master'.

Don't add and remove player element if it is already in the page

In case the player has genuinely been added to the website as visible player control.

Differential Revision: https://phabricator.kde.org/D25362

M  +8    -2    extension/content-script.js

https://commits.kde.org/plasma-browser-integration/47a2790188bf0a6d05fdbc94b6378f26514313ac