Bug 1387697 - Make UITour locate page actions on the urlbar first then on the page action panel, r?Gijs draft
authorFischer.json <fischer.json@gmail.com>
Mon, 14 Aug 2017 18:45:25 +0800
changeset 648666 51b568e72861c38b8630d4a29805356d8c067cb4
parent 648068 9d4366310802186d20c0f3349084098aedd92ae1
child 726907 9fa46ed996d25beb646afeaf78d09abdba4bde1b
push id74846
push userbmo:fliu@mozilla.com
push dateFri, 18 Aug 2017 06:19:51 +0000
reviewersGijs
bugs1387697
milestone57.0a1
Bug 1387697 - Make UITour locate page actions on the urlbar first then on the page action panel, r?Gijs MozReview-Commit-ID: 8TocyThHLzZ
browser/components/uitour/UITour-lib.js
browser/components/uitour/UITour.jsm
browser/components/uitour/test/browser_UITour4.js
browser/components/uitour/test/browser_UITour_availableTargets.js
--- a/browser/components/uitour/UITour-lib.js
+++ b/browser/components/uitour/UITour-lib.js
@@ -98,30 +98,29 @@ if (typeof Mozilla == "undefined") {
    * @see Mozilla.UITour.showInfo
    *
    * @description Valid values:<ul>
    * <li>accountStatus
    * <li>addons
    * <li>appMenu
    * <li>backForward
    * <li>bookmarks
-   * <li>bookmark-star-button
    * <li>controlCenter-trackingUnblock
    * <li>controlCenter-trackingBlock
    * <li>customize
    * <li>devtools
    * <li>forget
    * <li>help
    * <li>home
    * <li>library
    * <li>pageActionButton
-   * <li>pageAction-panel-bookmark
-   * <li>pageAction-panel-copyURL
-   * <li>pageAction-panel-emailLink
-   * <li>pageAction-panel-sendToDevice
+   * <li>pageAction-bookmark
+   * <li>pageAction-copyURL
+   * <li>pageAction-emailLink
+   * <li>pageAction-sendToDevice
    * <li>pocket
    * <li>privateWindow
    * <li>quit
    * <li>readerMode-urlBar
    * <li>search
    * <li>searchIcon
    * <li>searchPrefsLink
    * <li>selectedTabIcon
--- a/browser/components/uitour/UITour.jsm
+++ b/browser/components/uitour/UITour.jsm
@@ -147,17 +147,25 @@ this.UITour = {
       query: "#panic-button",
       widgetName: "panic-button",
     }],
     ["help",        {query: "#appMenu-help-button"}],
     ["home",        {query: "#home-button"}],
     ["library",     {query: "#appMenu-library-button"}],
     ["pocket", {
       allowAdd: true,
-      query: "#pocket-button",
+      query: (aDocument) => {
+        // The pocket's urlbar page action button is pre-defined in the DOM.
+        // It would be hidden if toggled off from the urlbar.
+        let node = aDocument.getElementById("pocket-button-box");
+        if (node && node.hidden == false) {
+          return node;
+        }
+        return aDocument.getElementById("pageAction-panel-pocket");
+      },
     }],
     ["privateWindow", {query: "#appMenu-private-window-button"}],
     ["quit",        {query: "#appMenu-quit-button"}],
     ["readerMode-urlBar", {query: "#reader-mode-button"}],
     ["search",      {
       infoPanelOffsetX: 18,
       infoPanelPosition: "after_start",
       query: "#searchbar",
@@ -204,30 +212,44 @@ this.UITour = {
     }],
     ["urlbar",      {
       query: "#urlbar",
       widgetName: "urlbar-container",
     }],
     ["pageActionButton", {
       query: "#pageActionButton"
     }],
-    ["pageAction-panel-bookmark", {
-      query: "#pageAction-panel-bookmark"
-    }],
-    ["pageAction-panel-copyURL", {
-      query: "#pageAction-panel-copyURL"
+    ["pageAction-bookmark", {
+      query: (aDocument) => {
+        // The bookmark's urlbar page action button is pre-defined in the DOM.
+        // It would be hidden if toggled off from the urlbar.
+        let node = aDocument.getElementById("star-button-box");
+        if (node && node.hidden == false) {
+          return node;
+        }
+        return aDocument.getElementById("pageAction-panel-bookmark");
+      },
     }],
-    ["pageAction-panel-emailLink", {
-      query: "#pageAction-panel-emailLink"
+    ["pageAction-copyURL", {
+      query: (aDocument) => {
+        return aDocument.getElementById("pageAction-urlbar-copyURL") ||
+               aDocument.getElementById("pageAction-panel-copyURL");
+      },
     }],
-    ["pageAction-panel-sendToDevice", {
-      query: "#pageAction-panel-sendToDevice"
+    ["pageAction-emailLink", {
+      query: (aDocument) => {
+        return aDocument.getElementById("pageAction-urlbar-emailLink") ||
+               aDocument.getElementById("pageAction-panel-emailLink");
+      },
     }],
-    ["bookmark-star-button", {
-      query: "#star-button"
+    ["pageAction-sendToDevice", {
+      query: (aDocument) => {
+        return aDocument.getElementById("pageAction-urlbar-sendToDevice") ||
+               aDocument.getElementById("pageAction-panel-sendToDevice");
+      },
     }]
   ]),
 
   init() {
     log.debug("Initializing UITour");
     // Lazy getter is initialized here so it can be replicated any time
     // in a test.
     delete this.seenPageIDs;
--- a/browser/components/uitour/test/browser_UITour4.js
+++ b/browser/components/uitour/test/browser_UITour4.js
@@ -13,24 +13,24 @@ add_UITour_task(async function test_high
   // Test highlighting the page action button on the urlbar
   let pageActionPanel = BrowserPageActions.panelNode;
   let highlightVisiblePromise = elementVisiblePromise(highlight, "Should show highlight");
   gContentAPI.showHighlight("pageActionButton");
   await highlightVisiblePromise;
   is(pageActionPanel.state, "closed", "Shouldn't open the page action panel while highlighting the pageActionButton");
   is(getShowHighlightTargetName(), "pageActionButton", "Should highlight the pageActionButton");
 
-  // Test switching the highlight to the bookmark button on the page action panel
+  // Test switching the highlight to the copyURL button on the page action panel
   let panelShownPromise = promisePanelElementShown(window, pageActionPanel);
   highlightVisiblePromise = elementVisiblePromise(highlight, "Should show highlight");
-  gContentAPI.showHighlight("pageAction-panel-bookmark");
+  gContentAPI.showHighlight("pageAction-copyURL");
   await highlightVisiblePromise;
   await panelShownPromise;
-  is(pageActionPanel.state, "open", "Should open the page action panel for highlighting the pageAction-panel-bookmark");
-  is(getShowHighlightTargetName(), "pageAction-panel-bookmark", "Should highlight the pageAction-panel-bookmark");
+  is(pageActionPanel.state, "open", "Should open the page action panel for highlighting the pageAction-copyURL");
+  is(getShowHighlightTargetName(), "pageAction-copyURL", "Should highlight the pageAction-copyURL");
 
   // Test hiding highlight
   let panelHiddenPromise = promisePanelElementHidden(window, pageActionPanel);
   let highlightHiddenPromise = elementHiddenPromise(highlight, "Should hide highlight");
   gContentAPI.hideHighlight();
   await highlightHiddenPromise;
   await panelHiddenPromise;
   is(pageActionPanel.state, "closed", "Should close the page action panel after hiding highlight");
@@ -52,23 +52,23 @@ add_UITour_task(async function test_high
   is(appMenu.state, "open", "Should open the app menu to highlight the addons button");
   is(pageActionPanel.state, "closed", "Shouldn't open the page action panel");
   is(getShowHighlightTargetName(), "addons", "Should highlight the addons button on the app menu");
 
   // Test switching the highlight to the copyURL button on the page action panel
   let appMenuHiddenPromise = promisePanelElementHidden(window, appMenu);
   let pageActionPanelShownPromise = promisePanelElementShown(window, pageActionPanel);
   highlightVisiblePromise = elementVisiblePromise(highlight, "Should show highlight");
-  gContentAPI.showHighlight("pageAction-panel-copyURL");
+  gContentAPI.showHighlight("pageAction-copyURL");
   await appMenuHiddenPromise;
   await pageActionPanelShownPromise;
   await highlightVisiblePromise;
   is(appMenu.state, "closed", "Should close the app menu after no more highlight for the addons button");
   is(pageActionPanel.state, "open", "Should open the page action panel to highlight the copyURL button");
-  is(getShowHighlightTargetName(), "pageAction-panel-copyURL", "Should highlight the copyURL button on the page action panel");
+  is(getShowHighlightTargetName(), "pageAction-copyURL", "Should highlight the copyURL button on the page action panel");
 
   // Test hiding highlight
   let pageActionPanelHiddenPromise = promisePanelElementHidden(window, pageActionPanel);
   let highlightHiddenPromise = elementHiddenPromise(highlight, "Should hide highlight");
   gContentAPI.hideHighlight();
   await pageActionPanelHiddenPromise
   await highlightHiddenPromise;
   is(appMenu.state, "closed", "Shouldn't open the app menu after hiding highlight");
@@ -80,22 +80,22 @@ add_UITour_task(async function test_show
   is_element_hidden(tooltip, "Tooltip should initially be hidden");
 
   let appMenu = window.PanelUI.panel;
   let pageActionPanel = BrowserPageActions.panelNode;
 
   // Test showing info tooltip on the emailLink button on the page action panel
   let pageActionPanelShownPromise = promisePanelElementShown(window, pageActionPanel);
   let tooltipVisiblePromise = elementVisiblePromise(tooltip, "Should show info tooltip");
-  await showInfoPromise("pageAction-panel-emailLink", "title", "text");
+  await showInfoPromise("pageAction-emailLink", "title", "text");
   await pageActionPanelShownPromise;
   await tooltipVisiblePromise;
   is(appMenu.state, "closed", "Shouldn't open the app menu");
   is(pageActionPanel.state, "open", "Should open the page action panel to show info on the copyURL button");
-  is(getShowInfoTargetName(), "pageAction-panel-emailLink", "Should show info tooltip on the emailLink button on the page action panel");
+  is(getShowInfoTargetName(), "pageAction-emailLink", "Should show info tooltip on the emailLink button on the page action panel");
 
   // Test switching info tooltip to the customize button on the app menu
   let appMenuShownPromise = promisePanelElementShown(window, appMenu);
   let pageActionPanelHiddenPromise = promisePanelElementHidden(window, pageActionPanel);
   tooltipVisiblePromise = elementVisiblePromise(tooltip, "Should show info tooltip");
   await showInfoPromise("customize", "title", "text");
   await appMenuShownPromise;
   await pageActionPanelHiddenPromise;
@@ -121,22 +121,22 @@ add_UITour_task(async function test_high
   is_element_hidden(tooltip, "Tooltip should initially be hidden");
 
   let appMenu = window.PanelUI.panel;
   let pageActionPanel = BrowserPageActions.panelNode;
 
   // Test highlighting the sendToDevice button on the page action panel
   let pageActionPanelShownPromise = promisePanelElementShown(window, pageActionPanel);
   let highlightVisiblePromise = elementVisiblePromise(highlight, "Should show highlight");
-  gContentAPI.showHighlight("pageAction-panel-sendToDevice");
+  gContentAPI.showHighlight("pageAction-sendToDevice");
   await pageActionPanelShownPromise;
   await highlightVisiblePromise;
   is(appMenu.state, "closed", "Shouldn't open the app menu");
   is(pageActionPanel.state, "open", "Should open the page action panel to highlight the sendToDevice button");
-  is(getShowHighlightTargetName(), "pageAction-panel-sendToDevice", "Should highlight the sendToDevice button on the page action panel");
+  is(getShowHighlightTargetName(), "pageAction-sendToDevice", "Should highlight the sendToDevice button on the page action panel");
 
   // Test showing info tooltip on the privateWindow button on the app menu
   let appMenuShownPromise = promisePanelElementShown(window, appMenu);
   let tooltipVisiblePromise = elementVisiblePromise(tooltip, "Should show info tooltip");
   let pageActionPanelHiddenPromise = promisePanelElementHidden(window, pageActionPanel);
   let highlightHiddenPromise = elementHiddenPromise(highlight, "Should hide highlight");
   await showInfoPromise("privateWindow", "title", "text");
   await appMenuShownPromise;
@@ -175,24 +175,24 @@ add_UITour_task(async function test_show
   is(pageActionPanel.state, "closed", "Shouldn't open the page action panel");
   is(getShowInfoTargetName(), "privateWindow", "Should show info tooltip on the privateWindow button on the app menu");
 
   // Test highlighting the sendToDevice button on the page action panel
   let pageActionPanelShownPromise = promisePanelElementShown(window, pageActionPanel);
   let highlightVisiblePromise = elementVisiblePromise(highlight, "Should show highlight");
   let appMenuHiddenPromise = promisePanelElementHidden(window, appMenu);
   let tooltipHiddenPromise = elementHiddenPromise(tooltip, "Should hide info");
-  gContentAPI.showHighlight("pageAction-panel-sendToDevice");
+  gContentAPI.showHighlight("pageAction-sendToDevice");
   await pageActionPanelShownPromise;
   await highlightVisiblePromise;
   await appMenuHiddenPromise;
   await tooltipHiddenPromise;
   is(appMenu.state, "closed", "Should close the app menu");
   is(pageActionPanel.state, "open", "Should open the page action panel to highlight the sendToDevice button");
-  is(getShowHighlightTargetName(), "pageAction-panel-sendToDevice", "Should highlight the sendToDevice button on the page action panel");
+  is(getShowHighlightTargetName(), "pageAction-sendToDevice", "Should highlight the sendToDevice button on the page action panel");
 
   // Test hiding highlight
   let pageActionPanelHiddenPromise = promisePanelElementHidden(window, pageActionPanel);
   let highlightHiddenPromise = elementHiddenPromise(highlight, "Should hide highlight");
   gContentAPI.hideHighlight();
   await pageActionPanelHiddenPromise;
   await highlightHiddenPromise;
   is(pageActionPanel.state, "closed", "Should close the page action panel after hiding highlight");
@@ -210,31 +210,31 @@ add_UITour_task(async function test_show
   gContentAPI.showMenu("appMenu");
   await appMenuShownPromise;
   is(appMenu.state, "open", "Should open the app menu");
   is(pageActionPanel.state, "closed", "Shouldn't open the page action panel");
 
   // Test highlighting the sendToDevice button on the page action panel
   let pageActionPanelShownPromise = promisePanelElementShown(window, pageActionPanel);
   let highlightVisiblePromise = elementVisiblePromise(highlight, "Should show highlight");
-  gContentAPI.showHighlight("pageAction-panel-sendToDevice");
+  gContentAPI.showHighlight("pageAction-sendToDevice");
   await pageActionPanelShownPromise;
   await highlightVisiblePromise;
   is(appMenu.state, "open", "Shouldn't close the app menu because it is opened explictly by api user.");
   is(pageActionPanel.state, "open", "Should open the page action panel to highlight the sendToDevice button");
-  is(getShowHighlightTargetName(), "pageAction-panel-sendToDevice", "Should highlight the sendToDevice button on the page action panel");
+  is(getShowHighlightTargetName(), "pageAction-sendToDevice", "Should highlight the sendToDevice button on the page action panel");
 
   // Test hiding the app menu wouldn't affect the highlight on the page action panel
   let appMenuHiddenPromise = promisePanelElementHidden(window, appMenu);
   gContentAPI.hideMenu("appMenu");
   await appMenuHiddenPromise;
   is_element_visible(highlight, "Highlight should still be visible");
   is(appMenu.state, "closed", "Should close the app menu");
   is(pageActionPanel.state, "open", "Shouldn't close the page action panel");
-  is(getShowHighlightTargetName(), "pageAction-panel-sendToDevice", "Should still highlight the sendToDevice button on the page action panel");
+  is(getShowHighlightTargetName(), "pageAction-sendToDevice", "Should still highlight the sendToDevice button on the page action panel");
 
   // Test hiding highlight
   let pageActionPanelHiddenPromise = promisePanelElementHidden(window, pageActionPanel);
   let highlightHiddenPromise = elementHiddenPromise(highlight, "Should hide highlight");
   gContentAPI.hideHighlight();
   await pageActionPanelHiddenPromise;
   await highlightHiddenPromise;
   is(appMenu.state, "closed", "Shouldn't open the app menu");
--- a/browser/components/uitour/test/browser_UITour_availableTargets.js
+++ b/browser/components/uitour/test/browser_UITour_availableTargets.js
@@ -10,27 +10,26 @@ var hasQuit = AppConstants.platform != "
 requestLongerTimeout(2);
 
 function getExpectedTargets() {
   return [
     "accountStatus",
     "addons",
     "appMenu",
     "backForward",
-    "bookmark-star-button",
     "customize",
     "devtools",
     "help",
     "home",
     "library",
     "pageActionButton",
-    "pageAction-panel-bookmark",
-    "pageAction-panel-copyURL",
-    "pageAction-panel-emailLink",
-    "pageAction-panel-sendToDevice",
+    "pageAction-bookmark",
+    "pageAction-copyURL",
+    "pageAction-emailLink",
+    "pageAction-sendToDevice",
       ...(hasPocket ? ["pocket"] : []),
     "privateWindow",
       ...(hasQuit ? ["quit"] : []),
     "readerMode-urlBar",
     "search",
     "searchIcon",
     "trackingProtection",
     "urlbar",
@@ -38,17 +37,16 @@ function getExpectedTargets() {
 }
 
 add_task(setup_UITourTest);
 
 add_UITour_task(async function test_availableTargets() {
   let data = await getConfigurationPromise("availableTargets");
   let expecteds = getExpectedTargets();
   ok_targets(data, expecteds);
-
   ok(UITour.availableTargetsCache.has(window),
      "Targets should now be cached");
 });
 
 add_UITour_task(async function test_availableTargets_changeWidgets() {
   CustomizableUI.addWidgetToArea("bookmarks-menu-button", CustomizableUI.AREA_NAVBAR, 0);
   ok(!UITour.availableTargetsCache.has(window),
      "Targets should be evicted from cache after widget change");
@@ -72,23 +70,86 @@ add_UITour_task(async function test_avai
   let expecteds = getExpectedTargets();
   // Default minus "search" and "searchIcon"
   expecteds = expecteds.filter(target => target != "search" && target != "searchIcon");
   ok_targets(data, expecteds);
 
   CustomizableUI.reset();
 });
 
+add_UITour_task(async function test_availableTargets_removeUrlbarPageActionsAll() {
+  pageActionsHelper.setActionsUrlbarState(false);
+  UITour.clearAvailableTargetsCache();
+  let data = await getConfigurationPromise("availableTargets");
+  let expecteds = getExpectedTargets();
+  ok_targets(data, expecteds);
+  let expectedActions = [
+    [ "pocket", "pageAction-panel-pocket" ],
+    [ "pageAction-bookmark", "pageAction-panel-bookmark" ],
+    [ "pageAction-copyURL", "pageAction-panel-copyURL" ],
+    [ "pageAction-emailLink", "pageAction-panel-emailLink" ],
+    [ "pageAction-sendToDevice", "pageAction-panel-sendToDevice" ],
+  ];
+  for (let [ targetName, expectedNodeId ] of expectedActions) {
+    await assertTargetNode(targetName, expectedNodeId);
+  }
+  pageActionsHelper.restoreActionsUrlbarState();
+});
+
+add_UITour_task(async function test_availableTargets_addUrlbarPageActionsAll() {
+  pageActionsHelper.setActionsUrlbarState(true);
+  UITour.clearAvailableTargetsCache();
+  let data = await getConfigurationPromise("availableTargets");
+  let expecteds = getExpectedTargets();
+  ok_targets(data, expecteds);
+  let expectedActions = [
+    [ "pocket", "pocket-button-box" ],
+    [ "pageAction-bookmark", "star-button-box" ],
+    [ "pageAction-copyURL", "pageAction-urlbar-copyURL" ],
+    [ "pageAction-emailLink", "pageAction-urlbar-emailLink" ],
+    [ "pageAction-sendToDevice", "pageAction-urlbar-sendToDevice" ],
+  ];
+  for (let [ targetName, expectedNodeId ] of expectedActions) {
+    await assertTargetNode(targetName, expectedNodeId);
+  }
+  pageActionsHelper.restoreActionsUrlbarState();
+});
+
 function ok_targets(actualData, expectedTargets) {
   // Depending on how soon after page load this is called, the selected tab icon
   // may or may not be showing the loading throbber.  Check for its presence and
   // insert it into expectedTargets if it's visible.
   let selectedTabIcon =
     document.getAnonymousElementByAttribute(gBrowser.selectedTab,
                                             "anonid",
                                             "tab-icon-image");
   if (selectedTabIcon && UITour.isElementVisible(selectedTabIcon))
     expectedTargets.push("selectedTabIcon");
 
   ok(Array.isArray(actualData.targets), "data.targets should be an array");
   is(actualData.targets.sort().toString(), expectedTargets.sort().toString(),
      "Targets should be as expected");
 }
+
+async function assertTargetNode(targetName, expectedNodeId) {
+  let target = await UITour.getTarget(window, targetName);
+  is(target.node.id, expectedNodeId, "UITour should get the right target node");
+}
+
+var pageActionsHelper = {
+  setActionsUrlbarState(inUrlbar) {
+    this._originalStates = [];
+    PageActions._actionsByID.forEach(action => {
+      this._originalStates.push([ action, action.shownInUrlbar ]);
+      action.shownInUrlbar = inUrlbar;
+    });
+  },
+
+  restoreActionsUrlbarState() {
+    if (!this._originalStates) {
+      return;
+    }
+    for (let [ action, originalState] of this._originalStates) {
+      action.shownInUrlbar = originalState;
+    }
+    this._originalStates = null;
+  }
+};