Bug 1232357 - Delay hiding the sound indicator icon by a few seconds after audio has stopped. r?gijs
MozReview-Commit-ID: BNaVgrX6jsv
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -431,16 +431,18 @@ pref("browser.tabs.drawInTitlebar", true
// When tabs opened by links in other tabs via a combination of
// browser.link.open_newwindow being set to 3 and target="_blank" etc are
// closed:
// true return to the tab that opened this tab (its owner)
// false return to the adjacent tab (old default)
pref("browser.tabs.selectOwnerOnClose", true);
pref("browser.tabs.showAudioPlayingIcon", true);
+// This should match Chromium's audio indicator delay.
+pref("browser.tabs.delayHidingAudioPlayingIconMS", 3000);
pref("browser.tabs.dontfocusfordialogs", true);
pref("browser.ctrlTab.previews", false);
// By default, do not export HTML at shutdown.
// If true, at shutdown the bookmarks in your menu and toolbar will
// be exported as HTML to the bookmarks.html file.
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -760,16 +760,18 @@
((aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) &&
aLocation.spec != "about:blank") ||
(isSameDocument && this.mBrowser.inLoadURI)) {
this.mBrowser.userTypedValue = null;
}
// If the browser was playing audio, we should remove the playing state.
if (this.mTab.hasAttribute("soundplaying") && !isSameDocument) {
+ clearTimeout(this.mTab._soundPlayingAttrRemovalTimer);
+ this.mTab._soundPlayingAttrRemovalTimer = 0;
this.mTab.removeAttribute("soundplaying");
this.mTabBrowser._tabAttrModified(this.mTab, ["soundplaying"]);
}
// If the browser was previously muted, we should restore the muted state.
if (this.mTab.hasAttribute("muted")) {
this.mTab.linkedBrowser.mute();
}
@@ -2711,16 +2713,24 @@
if (otherBrowser.hasAttribute("usercontextid")) {
ourBrowser.setAttribute("usercontextid", otherBrowser.getAttribute("usercontextid"));
}
// That's gBrowser for the other window, not the tab's browser!
var remoteBrowser = aOtherTab.ownerDocument.defaultView.gBrowser;
var isPending = aOtherTab.hasAttribute("pending");
+ // Expedite the removal of the icon if it was already scheduled.
+ if (aOtherTab._soundPlayingAttrRemovalTimer) {
+ clearTimeout(aOtherTab._soundPlayingAttrRemovalTimer);
+ aOtherTab._soundPlayingAttrRemovalTimer = 0;
+ aOtherTab.removeAttribute("soundplaying");
+ remoteBrowser._tabAttrModified(aOtherTab, ["soundplaying"]);
+ }
+
// First, start teardown of the other browser. Make sure to not
// fire the beforeunload event in the process. Close the other
// window if this was its last tab.
if (!remoteBrowser._beginRemoveTab(aOtherTab, aOurTab, true))
return;
let modifiedAttrs = [];
if (aOtherTab.hasAttribute("muted")) {
@@ -4863,16 +4873,17 @@
},
// Also support adding event listeners (forward to the tab container)
addEventListener: function (a, b, c) { this.self.tabContainer.addEventListener(a, b, c); },
removeEventListener: function (a, b, c) { this.self.tabContainer.removeEventListener(a, b, c); }
});
]]>
</getter>
</property>
+ <field name="_soundPlayingAttrRemovalTimer">0</field>
</implementation>
<handlers>
<handler event="DOMWindowClose" phase="capturing">
<![CDATA[
if (!event.isTrusted)
return;
@@ -4993,16 +5004,19 @@
!event.isTrusted)
return;
var browser = event.originalTarget;
var tab = this.getTabForBrowser(browser);
if (!tab)
return;
+ clearTimeout(tab._soundPlayingAttrRemovalTimer);
+ tab._soundPlayingAttrRemovalTimer = 0;
+
if (!tab.hasAttribute("soundplaying")) {
tab.setAttribute("soundplaying", true);
this._tabAttrModified(tab, ["soundplaying"]);
}
]]>
</handler>
<handler event="DOMAudioPlaybackStopped">
<![CDATA[
@@ -5011,18 +5025,21 @@
return;
var browser = event.originalTarget;
var tab = this.getTabForBrowser(browser);
if (!tab)
return;
if (tab.hasAttribute("soundplaying")) {
- tab.removeAttribute("soundplaying");
- this._tabAttrModified(tab, ["soundplaying"]);
+ let removalDelay = Services.prefs.getIntPref("browser.tabs.delayHidingAudioPlayingIconMS");
+ tab._soundPlayingAttrRemovalTimer = setTimeout(() => {
+ tab.removeAttribute("soundplaying");
+ this._tabAttrModified(tab, ["soundplaying"]);
+ }, removalDelay);
}
]]>
</handler>
</handlers>
</binding>
<binding id="tabbrowser-tabbox"
extends="chrome://global/content/bindings/tabbox.xml#tabbox">
--- a/browser/base/content/test/general/browser_audioTabIcon.js
+++ b/browser/base/content/test/general/browser_audioTabIcon.js
@@ -1,17 +1,19 @@
const PAGE = "https://example.com/browser/browser/base/content/test/general/file_mediaPlayback.html";
+const TABATTR_REMOVAL_PREFNAME = "browser.tabs.delayHidingAudioPlayingIconMS";
+const INITIAL_TABATTR_REMOVAL_DELAY_MS = Services.prefs.getIntPref(TABATTR_REMOVAL_PREFNAME);
function* wait_for_tab_playing_event(tab, expectPlaying) {
if (tab.soundPlaying == expectPlaying) {
ok(true, "The tab should " + (expectPlaying ? "" : "not ") + "be playing");
return true;
}
return yield BrowserTestUtils.waitForEvent(tab, "TabAttrModified", false, (event) => {
- if (event.detail.changed.indexOf("soundplaying") >= 0) {
+ if (event.detail.changed.includes("soundplaying")) {
is(tab.hasAttribute("soundplaying"), expectPlaying, "The tab should " + (expectPlaying ? "" : "not ") + "be playing");
is(tab.soundPlaying, expectPlaying, "The tab should " + (expectPlaying ? "" : "not ") + "be playing");
return true;
}
return false;
});
}
@@ -20,24 +22,51 @@ function* play(tab) {
yield ContentTask.spawn(browser, {}, function* () {
let audio = content.document.querySelector("audio");
audio.play();
});
yield wait_for_tab_playing_event(tab, true);
}
-function* pause(tab) {
- let browser = tab.linkedBrowser;
- yield ContentTask.spawn(browser, {}, function* () {
- let audio = content.document.querySelector("audio");
- audio.pause();
- });
+function* pause(tab, options) {
+ ok(tab.hasAttribute("soundplaying"), "The tab should have the soundplaying attribute when pause() is called");
+
+ let extendedDelay = options && options.extendedDelay;
+ if (extendedDelay) {
+ // Use 10s to remove possibility of race condition with attr removal.
+ Services.prefs.setIntPref(TABATTR_REMOVAL_PREFNAME, 10000);
+ }
- yield wait_for_tab_playing_event(tab, false);
+ try {
+ let browser = tab.linkedBrowser;
+ let awaitDOMAudioPlaybackStopped =
+ BrowserTestUtils.waitForEvent(browser, "DOMAudioPlaybackStopped", "DOMAudioPlaybackStopped event should get fired after pause");
+ let awaitTabPausedAttrModified =
+ wait_for_tab_playing_event(tab, false);
+ yield ContentTask.spawn(browser, {}, function* () {
+ let audio = content.document.querySelector("audio");
+ audio.pause();
+ });
+
+ if (extendedDelay) {
+ ok(tab.hasAttribute("soundplaying"), "The tab should still have the soundplaying attribute immediately after pausing");
+
+ yield awaitDOMAudioPlaybackStopped;
+ ok(tab.hasAttribute("soundplaying"), "The tab should still have the soundplaying attribute immediately after DOMAudioPlaybackStopped");
+ }
+
+ yield awaitTabPausedAttrModified;
+ ok(!tab.hasAttribute("soundplaying"), "The tab should not have the soundplaying attribute after the timeout has resolved");
+ } finally {
+ // Make sure other tests don't timeout if an exception gets thrown above.
+ // Need to use setIntPref instead of clearUserPref because prefs_general.js
+ // overrides the default value to help this and other tests run faster.
+ Services.prefs.setIntPref(TABATTR_REMOVAL_PREFNAME, INITIAL_TABATTR_REMOVAL_DELAY_MS);
+ }
}
function disable_non_test_mouse(disable) {
let utils = window.QueryInterface(Ci.nsIInterfaceRequestor)
.getInterface(Ci.nsIDOMWindowUtils);
utils.disableNonTestMouseEvents(disable);
}
@@ -78,17 +107,17 @@ function* test_tooltip(icon, expectedToo
}
// The set of tabs which have ever had their mute state changed.
// Used to determine whether the tab should have a muteReason value.
let everMutedTabs = new WeakSet();
function get_wait_for_mute_promise(tab, expectMuted) {
return BrowserTestUtils.waitForEvent(tab, "TabAttrModified", false, event => {
- if (event.detail.changed.indexOf("muted") >= 0) {
+ if (event.detail.changed.includes("muted")) {
is(tab.hasAttribute("muted"), expectMuted, "The tab should " + (expectMuted ? "" : "not ") + "be muted");
is(tab.muted, expectMuted, "The tab muted property " + (expectMuted ? "" : "not ") + "be true");
if (expectMuted || everMutedTabs.has(tab)) {
everMutedTabs.add(tab);
is(tab.muteReason, null, "The tab should have a null muteReason value");
} else {
is(tab.muteReason, undefined, "The tab should have an undefined muteReason value");
@@ -205,55 +234,40 @@ function* test_playing_icon_on_tab(tab,
function* test_swapped_browser_while_playing(oldTab, newBrowser) {
ok(oldTab.hasAttribute("muted"), "Expected the correct muted attribute on the old tab");
is(oldTab.muteReason, null, "Expected the correct muteReason attribute on the old tab");
ok(oldTab.hasAttribute("soundplaying"), "Expected the correct soundplaying attribute on the old tab");
let newTab = gBrowser.getTabForBrowser(newBrowser);
let AttrChangePromise = BrowserTestUtils.waitForEvent(newTab, "TabAttrModified", false, event => {
- return event.detail.changed.indexOf("soundplaying") >= 0 &&
- event.detail.changed.indexOf("muted") >= 0;
+ return event.detail.changed.includes("soundplaying") &&
+ event.detail.changed.includes("muted");
});
gBrowser.swapBrowsersAndCloseOther(newTab, oldTab);
yield AttrChangePromise;
ok(newTab.hasAttribute("muted"), "Expected the correct muted attribute on the new tab");
is(newTab.muteReason, null, "Expected the correct muteReason property on the new tab");
ok(newTab.hasAttribute("soundplaying"), "Expected the correct soundplaying attribute on the new tab");
- let receivedSoundPlaying = 0;
- // We need to receive two TabAttrModified events with 'soundplaying'
- // because swapBrowsersAndCloseOther involves nsDocument::OnPageHide and
- // nsDocument::OnPageShow. Each methods lead to TabAttrModified event.
- yield BrowserTestUtils.waitForEvent(newTab, "TabAttrModified", false, event => {
- if (event.detail.changed.indexOf("soundplaying") >= 0) {
- return (++receivedSoundPlaying == 2);
- }
- return false;
- });
-
- ok(newTab.hasAttribute("muted"), "Expected the correct muted attribute on the new tab");
- is(newTab.muteReason, null, "Expected the correct muteReason property on the new tab");
- ok(newTab.hasAttribute("soundplaying"), "Expected the correct soundplaying attribute on the new tab");
-
let icon = document.getAnonymousElementByAttribute(newTab, "anonid",
"soundplaying-icon");
yield test_tooltip(icon, "Unmute tab", true);
}
function* test_swapped_browser_while_not_playing(oldTab, newBrowser) {
ok(oldTab.hasAttribute("muted"), "Expected the correct muted attribute on the old tab");
is(oldTab.muteReason, null, "Expected the correct muteReason property on the old tab");
ok(!oldTab.hasAttribute("soundplaying"), "Expected the correct soundplaying attribute on the old tab");
let newTab = gBrowser.getTabForBrowser(newBrowser);
let AttrChangePromise = BrowserTestUtils.waitForEvent(newTab, "TabAttrModified", false, event => {
- return event.detail.changed.indexOf("muted") >= 0;
+ return event.detail.changed.includes("muted");
});
let AudioPlaybackPromise = new Promise(resolve => {
let observer = (subject, topic, data) => {
ok(false, "Should not see an audio-playback notification");
};
Services.obs.addObserver(observer, "audiochannel-activity-normal", false);
setTimeout(() => {
@@ -354,17 +368,17 @@ function* test_click_on_pinned_tab_after
function* test_cross_process_load() {
function* test_on_browser(browser) {
let tab = gBrowser.getTabForBrowser(browser);
// Start playback and wait for it to finish.
yield play(tab);
let soundPlayingStoppedPromise = BrowserTestUtils.waitForEvent(tab, "TabAttrModified", false,
- event => event.detail.changed.indexOf("soundplaying") >= 0
+ event => event.detail.changed.includes("soundplaying")
);
// Go to a different process.
browser.loadURI("about:");
yield BrowserTestUtils.browserLoaded(browser);
yield soundPlayingStoppedPromise;
@@ -442,16 +456,34 @@ function* test_on_browser(browser) {
gBrowser,
url: "data:text/html,test"
}, () => test_on_browser(browser));
} else {
yield test_browser_swapping(tab, browser);
}
}
+function* test_delayed_tabattr_removal() {
+ function* test_on_browser(browser) {
+ let tab = gBrowser.getTabForBrowser(browser);
+ yield play(tab);
+
+ // Extend the delay to guarantee the soundplaying attribute
+ // is not removed from the tab when audio is stopped. Without
+ // the extended delay the attribute could be removed in the
+ // same tick and the test wouldn't catch that this broke.
+ yield pause(tab, {extendedDelay: true});
+ }
+
+ yield BrowserTestUtils.withNewTab({
+ gBrowser,
+ url: PAGE
+ }, test_on_browser);
+}
+
add_task(function*() {
yield new Promise((resolve) => {
SpecialPowers.pushPrefEnv({"set": [
["browser.tabs.showAudioPlayingIcon", true],
]}, resolve);
});
});
@@ -463,8 +495,10 @@ add_task(function* test_page() {
}, test_on_browser);
});
add_task(test_click_on_pinned_tab_after_mute);
add_task(test_cross_process_load);
add_task(test_mute_keybinding);
+
+add_task(test_delayed_tabattr_removal);
--- a/testing/profiles/prefs_general.js
+++ b/testing/profiles/prefs_general.js
@@ -317,16 +317,18 @@ user_pref("media.autoplay.enabled", true
#if defined(XP_WIN)
user_pref("media.decoder.heuristic.dormant.timeout", 0);
#endif
// Don't use auto-enabled e10s
user_pref("browser.tabs.remote.autostart.1", false);
user_pref("browser.tabs.remote.autostart.2", false);
+// Don't show a delay when hiding the audio indicator during tests
+user_pref("browser.tabs.delayHidingAudioPlayingIconMS", 0);
// Don't forceably kill content processes after a timeout
user_pref("dom.ipc.tabs.shutdownTimeoutSecs", 0);
// Don't block add-ons for e10s
user_pref("extensions.e10sBlocksEnabling", false);
// Avoid performing Reader Mode intros during tests.
user_pref("browser.reader.detectedFirstArticle", true);