| Summary: | Audio doesn't play when Enhanced Media Controls is enabled on specific websites | ||
|---|---|---|---|
| Product: | [Plasma] plasma-browser-integration | Reporter: | Frederick Zhang <frederick888> |
| Component: | Firefox | Assignee: | 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: | https://commits.kde.org/plasma-browser-integration/c1d7ea7346074e1912d56b9aed8d5ec47957e3a2 | Version Fixed/Implemented In: | |
| Sentry Crash Report: | |||
|
Description
Frederick Zhang
2019-09-09 07:18:51 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/ I have the same problem with the pronunciations at https://forvo.com/ *** Bug 413817 has been marked as a duplicate of this bug. *** *** Bug 412418 has been marked as a duplicate of this bug. *** 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! 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 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 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. I'm afraid that the patch fixed https://forvo.com but not https://vgmrips.net and my jsfiddle. 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 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;
};
}`);
}
}
Ah, Firefox. Looks like Firefox now also has this autoplay prevention... okay, will fix. Thanks. Alright, please try again with the latest patch. Works fine here on Firefox 70 Yup, it worked with the new patch! Thanks! 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
(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 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 |