Bug 1381020 - Account for UITour panel targets having been invalidated due to DOM shuffle and XBL teardown. r?MattN draft
authorMike Conley <mconley@mozilla.com>
Wed, 19 Jul 2017 14:33:08 -0700
changeset 612231 e4ace4420e4dee502185896778567c1295e51ef5
parent 611331 c5ea72577f794635a334a30780d104a1d89267a0
child 638362 3baba81cb28693d86c9400093d36fdd263636c91
push id69446
push userbmo:mconley@mozilla.com
push dateThu, 20 Jul 2017 15:07:57 +0000
reviewersMattN
bugs1381020
milestone56.0a1
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(() => {