Bug 1478348 - Dont show doorhanger on about:newtab. r=johannh
authorDale Harvey <dale@arandomurl.com>
Fri, 24 Aug 2018 16:43:22 +0100
changeset 436033 d163fc054fdc396fba639f133bf7d9f6f5c35c18
parent 436032 e8ef34cb3e9d1b8737e2ab5d4033d35ae68d6d9d
child 436034 690dfc98faffc7257bb2534057dfd1dddb9cac94
push id34625
push userdvarga@mozilla.com
push dateThu, 13 Sep 2018 02:31:40 +0000
treeherdermozilla-central@51e9e9660b3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh
bugs1478348
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1478348 - Dont show doorhanger on about:newtab. r=johannh MozReview-Commit-ID: EmHV3OS6Kra
browser/base/content/browser.js
browser/base/content/tabbrowser.js
browser/base/content/test/permissions/browser.ini
browser/base/content/test/permissions/browser_autoplay_doorhanger.js
browser/base/content/urlbarBindings.xml
toolkit/mozapps/extensions/test/xpinstall/browser_doorhanger_installs.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -2651,18 +2651,21 @@ function BrowserPageInfo(documentURL, in
                     "chrome,toolbar,dialog=no,resizable", args);
 }
 
 /**
  * Sets the URI to display in the location bar.
  *
  * @param aURI [optional]
  *        nsIURI to set. If this is unspecified, the current URI will be used.
+ * @param updatePopupNotifications [optional]
+ *        Passed though to SetPageProxyState, indicates whether the
+ *        PopupNotifications need updated.
  */
-function URLBarSetURI(aURI) {
+function URLBarSetURI(aURI, updatePopupNotifications) {
   var value = gBrowser.userTypedValue;
   var valid = false;
 
   // Explicitly check for nulled out value. We don't want to reset the URL
   // bar if the user has deleted the URL and we'd just put the same URL
   // back. See bug 304198.
   if (value === null) {
     let uri = aURI || gBrowser.currentURI;
@@ -2697,17 +2700,17 @@ function URLBarSetURI(aURI) {
   gURLBar.valueIsTyped = !valid;
   gURLBar.removeAttribute("usertyping");
   if (isDifferentValidValue) {
     // The selection is enforced only for new values, to avoid overriding the
     // cursor position when the user switches windows while typing.
     gURLBar.selectionStart = gURLBar.selectionEnd = 0;
   }
 
-  SetPageProxyState(valid ? "valid" : "invalid");
+  SetPageProxyState(valid ? "valid" : "invalid", updatePopupNotifications);
 }
 
 function losslessDecodeURI(aURI) {
   let scheme = aURI.scheme;
   if (scheme == "moz-action")
     throw new Error("losslessDecodeURI should never get a moz-action URI");
 
   var value = aURI.displaySpec;
@@ -2794,32 +2797,37 @@ function UpdateUrlbarSearchSplitterState
       splitter.className = "chromeclass-toolbar-additional";
     }
     urlbar.parentNode.insertBefore(splitter, ibefore);
   } else if (splitter)
     splitter.remove();
 }
 
 function UpdatePageProxyState() {
-  if (gURLBar && gURLBar.value != gLastValidURLStr)
-    SetPageProxyState("invalid");
+  if (gURLBar && gURLBar.value != gLastValidURLStr) {
+    SetPageProxyState("invalid", true);
+  }
 }
 
 /**
  * Updates the user interface to indicate whether the URI in the location bar is
  * different than the loaded page, because it's being edited or because a search
  * result is currently selected and is displayed in the location bar.
  *
  * @param aState
  *        The string "valid" indicates that the security indicators and other
  *        related user interface elments should be shown because the URI in the
  *        location bar matches the loaded page. The string "invalid" indicates
  *        that the URI in the location bar is different than the loaded page.
+ * @param updatePopupNotifications
+ *        Boolean that indicates whether we should update the PopupNotifications
+ *        visibility due to this change, otherwise avoid doing so as it is being
+ *        handled somewhere else.
  */
-function SetPageProxyState(aState) {
+function SetPageProxyState(aState, updatePopupNotifications) {
   if (!gURLBar)
     return;
 
   let oldPageProxyState = gURLBar.getAttribute("pageproxystate");
   // The "browser_urlbar_stop_pending.js" test uses a MutationObserver to do
   // some verifications at this point, and it breaks if we don't write the
   // attribute, even if it hasn't changed (bug 1338115).
   gURLBar.setAttribute("pageproxystate", aState);
@@ -2830,17 +2838,17 @@ function SetPageProxyState(aState) {
     gLastValidURLStr = gURLBar.value;
     gURLBar.addEventListener("input", UpdatePageProxyState);
   } else if (aState == "invalid") {
     gURLBar.removeEventListener("input", UpdatePageProxyState);
   }
 
   // After we've ensured that we've applied the listeners and updated the value
   // of gLastValidURLStr, return early if the actual state hasn't changed.
-  if (oldPageProxyState == aState) {
+  if (oldPageProxyState == aState || !updatePopupNotifications) {
     return;
   }
 
   UpdatePopupNotificationsVisibility();
 }
 
 function UpdatePopupNotificationsVisibility() {
   // Only need to do something if the PopupNotifications object for this window
@@ -4751,17 +4759,17 @@ var XULBrowserWindow = {
         this.busyUI = false;
 
         this.stopCommand.setAttribute("disabled", "true");
         CombinedStopReload.switchToReload(aRequest, aWebProgress);
       }
     }
   },
 
-  onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags) {
+  onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags, aIsSimulated) {
     var location = aLocationURI ? aLocationURI.spec : "";
 
     let pageTooltip = document.getElementById("aHTMLTooltip");
     let tooltipNode = pageTooltip.triggerNode;
     if (tooltipNode) {
       // Optimise for the common case
       if (aWebProgress.isTopLevel) {
         pageTooltip.hidePopup();
@@ -4803,17 +4811,21 @@ var XULBrowserWindow = {
       if ((location == "about:blank" && checkEmptyPageOrigin()) ||
           location == "") { // Second condition is for new tabs, otherwise
                              // reload function is enabled until tab is refreshed.
         this.reloadCommand.setAttribute("disabled", "true");
       } else {
         this.reloadCommand.removeAttribute("disabled");
       }
 
-      URLBarSetURI(aLocationURI);
+      // We want to update the popup visibility if we received this notification
+      // via simulated locationchange events such as switching between tabs, however
+      // if this is a document navigation then PopupNotifications will be updated
+      // via TabsProgressListener.onLocationChange and we do not want it called twice
+      URLBarSetURI(aLocationURI, aIsSimulated);
 
       BookmarkingUI.onLocationChange();
 
       gIdentityHandler.onLocationChange();
 
       BrowserPageActions.onLocationChange();
 
       SafeBrowsingNotificationBox.onLocationChange(aLocationURI);
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -929,17 +929,17 @@ window._gBrowser = {
     if ((oldBrowser.blockedPopups && !newBrowser.blockedPopups) ||
         (!oldBrowser.blockedPopups && newBrowser.blockedPopups)) {
       newBrowser.updateBlockedPopups();
     }
 
     // Update the URL bar.
     let webProgress = newBrowser.webProgress;
     this._callProgressListeners(null, "onLocationChange",
-                                [webProgress, null, newBrowser.currentURI, 0],
+                                [webProgress, null, newBrowser.currentURI, 0, true],
                                 true, false);
 
     let securityUI = newBrowser.securityUI;
     if (securityUI) {
       // Include the true final argument to indicate that this event is
       // simulated (instead of being observed by the webProgressListener).
       this._callProgressListeners(null, "onSecurityChange",
                                   [webProgress, null, securityUI.state, true],
--- a/browser/base/content/test/permissions/browser.ini
+++ b/browser/base/content/test/permissions/browser.ini
@@ -9,11 +9,15 @@ support-files=
 [browser_temporary_permissions.js]
 support-files =
   temporary_permissions_subframe.html
   ../webrtc/get_user_media.html
 [browser_autoplay_blocked.js]
 support-files =
   browser_autoplay_blocked.html
   ../general/audio.ogg
+[browser_autoplay_doorhanger.js]
+support-files =
+  browser_autoplay_blocked.html
+  ../general/audio.ogg
 [browser_temporary_permissions_expiry.js]
 [browser_temporary_permissions_navigation.js]
 [browser_temporary_permissions_tabs.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/permissions/browser_autoplay_doorhanger.js
@@ -0,0 +1,33 @@
+/*
+ * Test that autoplay doorhanger is hidden when user clicks back to new tab page
+ */
+
+const AUTOPLAY_PAGE  = getRootDirectory(gTestPath).replace("chrome://mochitests/content", "https://example.com") + "browser_autoplay_blocked.html";
+
+add_task(async function testHiddenAfterBack() {
+
+  Services.prefs.setIntPref("media.autoplay.default", Ci.nsIAutoplay.PROMPT);
+
+  await BrowserTestUtils.withNewTab("about:home", async function(browser) {
+
+    is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
+
+    let shown = BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popupshown");
+    let loaded = BrowserTestUtils.browserLoaded(browser);
+    BrowserTestUtils.loadURI(browser, AUTOPLAY_PAGE);
+    await loaded;
+    await shown;
+
+    is(PopupNotifications.panel.state, "open", "Doorhanger is shown");
+
+    let hidden = BrowserTestUtils.waitForEvent(PopupNotifications.panel, "popuphidden");
+    let backPromise = BrowserTestUtils.browserStopped(browser);
+    EventUtils.synthesizeMouseAtCenter(document.getElementById("back-button"), {});
+    await backPromise;
+    await hidden;
+
+    is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
+  });
+
+  Services.prefs.clearUserPref("media.autoplay.default");
+});
--- a/browser/base/content/urlbarBindings.xml
+++ b/browser/base/content/urlbarBindings.xml
@@ -811,17 +811,17 @@ file, You can obtain one at http://mozil
         <body><![CDATA[
           var isScrolling = this.popupOpen;
 
           gBrowser.userTypedValue = null;
 
           // don't revert to last valid url unless page is NOT loading
           // and user is NOT key-scrolling through autocomplete list
           if (!XULBrowserWindow.isBusy && !isScrolling) {
-            URLBarSetURI();
+            URLBarSetURI(null, true);
 
             // If the value isn't empty and the urlbar has focus, select the value.
             if (this.value && this.hasAttribute("focused"))
               this.select();
           }
 
           // tell widget to revert to last typed text only if the user
           // was scrolling when they hit escape
@@ -1269,17 +1269,17 @@ file, You can obtain one at http://mozil
           if (droppedItem) {
             let triggeringPrincipal = browserDragAndDrop.getTriggeringPrincipal(aEvent);
             this.value = droppedItem instanceof URL ? droppedItem.href : droppedItem;
             SetPageProxyState("invalid");
             this.focus();
             this.handleCommand(null, undefined, undefined, triggeringPrincipal);
             // Force not showing the dropped URI immediately.
             gBrowser.userTypedValue = null;
-            URLBarSetURI();
+            URLBarSetURI(null, true);
           }
         ]]></body>
       </method>
 
       <method name="makeURIReadable">
         <parameter name="aURI"/>
         <body>
           <![CDATA[
--- a/toolkit/mozapps/extensions/test/xpinstall/browser_doorhanger_installs.js
+++ b/toolkit/mozapps/extensions/test/xpinstall/browser_doorhanger_installs.js
@@ -572,17 +572,17 @@ async function test_tabNavigate() {
   await waitForTick();
 
   let installs = await AddonManager.getAllInstalls();
   is(installs.length, 0, "Should be no pending install");
 
   Services.perms.remove(makeURI("http://example.com/"), "install");
   await loadPromise;
 
-  await removeTabAndWaitForNotificationClose();
+  await BrowserTestUtils.removeTab(gBrowser.selectedTab);
 },
 
 async function test_urlBar() {
   let progressPromise = waitForProgressNotification();
   let dialogPromise = waitForInstallDialog();
 
   gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser, "about:blank");
   await BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);