Bug 1381020 - Account for UITour panel targets having been invalidated due to DOM shuffle and XBL teardown. r=MattN
authorMike Conley <mconley@mozilla.com>
Wed, 19 Jul 2017 14:33:08 -0700
changeset 369897 1134bcd9eb8850a9b49cd7d5255768580a23e292
parent 369896 6d52fa99545926627d4e2767af66b001373ca3b8
child 369898 532785204f7bcfbcb992e01816c6566716ba9638
push id32208
push userarchaeopteryx@coole-files.de
push dateFri, 21 Jul 2017 09:12:51 +0000
treeherdermozilla-central@0faada5c2f30 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1381020
milestone56.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 1381020 - Account for UITour panel targets having been invalidated due to DOM shuffle and XBL teardown. r=MattN MozReview-Commit-ID: CAcJdCmo8ol
browser/components/uitour/UITour.jsm
browser/components/uitour/test/browser_UITour2.js
--- a/browser/components/uitour/UITour.jsm
+++ b/browser/components/uitour/UITour.jsm
@@ -1000,17 +1000,18 @@ this.UITour = {
     return targetElement.id.startsWith(prefix)
              && targetElement.id != "PanelUI-button";
   },
 
   /**
    * Called before opening or after closing a highlight or info panel to see if
    * we need to open or close the appMenu to see the annotation's anchor.
    */
-  _setAppMenuStateForAnnotation(aWindow, aAnnotationType, aShouldOpenForHighlight, aCallback = null) {
+  _setAppMenuStateForAnnotation(aWindow, aAnnotationType, aShouldOpenForHighlight, aTarget = null,
+                                aCallback = null) {
     log.debug("_setAppMenuStateForAnnotation:", aAnnotationType);
     log.debug("_setAppMenuStateForAnnotation: Menu is expected to be:", aShouldOpenForHighlight ? "open" : "closed");
 
     // If the panel is in the desired state, we're done.
     let panelIsOpen = aWindow.PanelUI.panel.state != "closed";
     if (aShouldOpenForHighlight == panelIsOpen) {
       log.debug("_setAppMenuStateForAnnotation: Panel already in expected state");
       if (aCallback)
@@ -1030,17 +1031,26 @@ this.UITour = {
       this.appMenuOpenForAnnotation.add(aAnnotationType);
     } else {
       this.appMenuOpenForAnnotation.delete(aAnnotationType);
     }
 
     // Actually show or hide the menu
     if (this.appMenuOpenForAnnotation.size) {
       log.debug("_setAppMenuStateForAnnotation: Opening the menu");
-      this.showMenu(aWindow, "appMenu", aCallback);
+      this.showMenu(aWindow, "appMenu", async () => {
+        // PanelMultiView's like the AppMenu might shuffle the DOM, which might result
+        // in our target being invalidated if it was anonymous content (since the XBL
+        // binding it belonged to got destroyed). We work around this by re-querying for
+        // the node and stuffing it into the old target structure.
+        log.debug("_setAppMenuStateForAnnotation: Refreshing target");
+        let refreshedTarget = await this.getTarget(aWindow, aTarget.targetName);
+        aTarget.node = refreshedTarget.node;
+        aCallback();
+      });
     } else {
       log.debug("_setAppMenuStateForAnnotation: Closing the menu");
       this.hideMenu(aWindow, "appMenu");
       if (aCallback)
         aCallback();
     }
 
   },
@@ -1147,16 +1157,17 @@ this.UITour = {
     // Prevent showing a panel at an undefined position.
     if (!this.isElementVisible(aTarget.node)) {
       log.warn("showHighlight: Not showing a highlight since the target isn't visible", aTarget);
       return;
     }
 
     this._setAppMenuStateForAnnotation(aChromeWindow, "highlight",
                                        this.targetIsInAppMenu(aTarget),
+                                       aTarget,
                                        showHighlightPanel.bind(this));
   },
 
   hideHighlight(aWindow) {
     let highlighter = aWindow.document.getElementById("UITourHighlight");
     this._removeAnnotationPanelMutationObserver(highlighter.parentElement);
     highlighter.parentElement.hidePopup();
     highlighter.removeAttribute("active");
@@ -1276,19 +1287,27 @@ this.UITour = {
     }
 
     // Prevent showing a panel at an undefined position.
     if (!this.isElementVisible(aAnchor.node)) {
       log.warn("showInfo: Not showing since the target isn't visible", aAnchor);
       return;
     }
 
+    // We need to bind the anchor argument to the showInfoPanel function call
+    // after _setAppMenuStateForAnnotation has finished, since
+    // _setAppMenuStateForAnnotation might have refreshed the anchor node.
+    let callShowInfoPanel = () => {
+      showInfoPanel.call(this, this._correctAnchor(aAnchor.node));
+    };
+
     this._setAppMenuStateForAnnotation(aChromeWindow, "info",
                                        this.targetIsInAppMenu(aAnchor),
-                                       showInfoPanel.bind(this, this._correctAnchor(aAnchor.node)));
+                                       aAnchor,
+                                       callShowInfoPanel);
   },
 
   isInfoOnTarget(aChromeWindow, aTargetName) {
     let document = aChromeWindow.document;
     let tooltip = document.getElementById("UITourTooltip");
     return tooltip.getAttribute("targetName") == aTargetName && tooltip.state != "closed";
   },
 
--- a/browser/components/uitour/test/browser_UITour2.js
+++ b/browser/components/uitour/test/browser_UITour2.js
@@ -10,32 +10,36 @@ var gContentWindow;
 function test() {
   UITourTest();
 }
 
 var tests = [
   function test_info_customize_auto_open_close(done) {
     let popup = document.getElementById("UITourTooltip");
     gContentAPI.showInfo("customize", "Customization", "Customize me please!");
-    UITour.getTarget(window, "customize").then((customizeTarget) => {
-      waitForPopupAtAnchor(popup, customizeTarget.node, function checkPanelIsOpen() {
-        isnot(PanelUI.panel.state, "closed", "Panel should have opened before the popup anchored");
-        ok(PanelUI.panel.hasAttribute("noautohide"), "@noautohide on the menu panel should have been set");
+
+    let shownPromise = promisePanelShown(window);
+    shownPromise.then(() => {
+      UITour.getTarget(window, "customize").then((customizeTarget) => {
+        waitForPopupAtAnchor(popup, customizeTarget.node, function checkPanelIsOpen() {
+          isnot(PanelUI.panel.state, "closed", "Panel should have opened before the popup anchored");
+          ok(PanelUI.panel.hasAttribute("noautohide"), "@noautohide on the menu panel should have been set");
 
-        // Move the info outside which should close the app menu.
-        gContentAPI.showInfo("appMenu", "Open Me", "You know you want to");
-        UITour.getTarget(window, "appMenu").then((target) => {
-          waitForPopupAtAnchor(popup, target.node, function checkPanelIsClosed() {
-            isnot(PanelUI.panel.state, "open",
-                  "Panel should have closed after the info moved elsewhere.");
-            ok(!PanelUI.panel.hasAttribute("noautohide"), "@noautohide on the menu panel should have been cleaned up on close");
-            done();
-          }, "Info should move to the appMenu button");
-        });
-      }, "Info panel should be anchored to the customize button");
+          // Move the info outside which should close the app menu.
+          gContentAPI.showInfo("appMenu", "Open Me", "You know you want to");
+          UITour.getTarget(window, "appMenu").then((target) => {
+            waitForPopupAtAnchor(popup, target.node, function checkPanelIsClosed() {
+              isnot(PanelUI.panel.state, "open",
+                    "Panel should have closed after the info moved elsewhere.");
+              ok(!PanelUI.panel.hasAttribute("noautohide"), "@noautohide on the menu panel should have been cleaned up on close");
+              done();
+            }, "Info should move to the appMenu button");
+          });
+        }, "Info panel should be anchored to the customize button");
+      });
     });
   },
   function test_info_customize_manual_open_close(done) {
     let popup = document.getElementById("UITourTooltip");
     // Manually open the app menu then show an info panel there. The menu should remain open.
     let shownPromise = promisePanelShown(window);
     gContentAPI.showMenu("appMenu");
     shownPromise.then(() => {