Bug 1387697 - Make UITour locate page actions on the urlbar first then on the page action panel. draft
authorFischer.json <fischer.json@gmail.com>
Mon, 14 Aug 2017 18:45:25 +0800
changeset 646450 1a0adfeab397e651f3f6de9320dd46df04733748
parent 646265 b95b1638db48fc3d450b95b98da6bcd2f9326d2f
child 726252 8c5e690c575c903b9139fa0da1e4b29b3087a74d
push id74131
push userbmo:fliu@mozilla.com
push dateTue, 15 Aug 2017 09:19:48 +0000
bugs1387697
milestone57.0a1
Bug 1387697 - Make UITour locate page actions on the urlbar first then on the page action panel. 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
browser/components/uitour/test/head.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
@@ -145,17 +145,24 @@ 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) => {
+        let node = aDocument.getElementById("pageAction-urlbar-pocket") ||
+                   aDocument.getElementById("pageAction-panel-pocket");
+        if (node) {
+          return node;
+        }
+        return null;
+      },
       widgetName: "pocket-button",
     }],
     ["privateWindow", {query: "#appMenu-private-window-button"}],
     ["quit",        {query: "#appMenu-quit-button"}],
     ["readerMode-urlBar", {query: "#reader-mode-button"}],
     ["search",      {
       infoPanelOffsetX: 18,
       infoPanelPosition: "after_start",
@@ -203,30 +210,55 @@ this.UITour = {
     }],
     ["urlbar",      {
       query: "#urlbar",
       widgetName: "urlbar-container",
     }],
     ["pageActionButton", {
       query: "#pageActionButton"
     }],
-    ["pageAction-panel-bookmark", {
-      query: "#pageAction-panel-bookmark"
+    ["pageAction-bookmark", {
+      query: (aDocument) => {
+        let node = aDocument.getElementById("star-button") ||
+                   aDocument.getElementById("pageAction-panel-bookmark");
+        if (node) {
+          return node;
+        }
+        return null;
+      },
     }],
-    ["pageAction-panel-copyURL", {
-      query: "#pageAction-panel-copyURL"
+    ["pageAction-copyURL", {
+      query: (aDocument) => {
+        let node = aDocument.getElementById("pageAction-urlbar-copyURL") ||
+                   aDocument.getElementById("pageAction-panel-copyURL");
+        if (node) {
+          return node;
+        }
+        return null;
+      },
     }],
-    ["pageAction-panel-emailLink", {
-      query: "#pageAction-panel-emailLink"
+    ["pageAction-emailLink", {
+      query: (aDocument) => {
+        let node = aDocument.getElementById("pageAction-urlbar-emailLink") ||
+                   aDocument.getElementById("pageAction-panel-emailLink");
+        if (node) {
+          return node;
+        }
+        return null;
+      },
     }],
-    ["pageAction-panel-sendToDevice", {
-      query: "#pageAction-panel-sendToDevice"
-    }],
-    ["bookmark-star-button", {
-      query: "#star-button"
+    ["pageAction-sendToDevice", {
+      query: (aDocument) => {
+        let node = aDocument.getElementById("pageAction-urlbar-sendToDevice") ||
+                   aDocument.getElementById("pageAction-panel-sendToDevice");
+        if (node) {
+          return node;
+        }
+        return null;
+      },
     }]
   ]),
 
   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
@@ -1,36 +1,38 @@
 "use strict";
 
 var gTestTab;
 var gContentAPI;
 var gContentWindow;
 
 add_task(setup_UITourTest);
-
+function TMP_stop(s) {
+  return new Promise(res => setTimeout(res, s * 1000));
+}
 add_UITour_task(async function test_highligh_between_pageActionButtonOnUrlbar_and_buttonOnPageActionPanel() {
   let highlight = document.getElementById("UITourHighlight");
   is_element_hidden(highlight, "Highlight should initially be hidden");
 
   // 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 +54,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 +82,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 +123,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 +177,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 +212,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
@@ -1,54 +1,50 @@
 "use strict";
 
 var gTestTab;
 var gContentAPI;
 var gContentWindow;
 
-var hasPocket = Services.prefs.getBoolPref("extensions.pocket.enabled");
 var hasQuit = AppConstants.platform != "macosx";
 
 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",
-      ...(hasPocket ? ["pocket"] : []),
+    "pageAction-bookmark",
+    "pageAction-copyURL",
+    "pageAction-emailLink",
+    "pageAction-sendToDevice",
+    "pocket",
     "privateWindow",
       ...(hasQuit ? ["quit"] : []),
     "readerMode-urlBar",
     "search",
     "searchIcon",
     "trackingProtection",
     "urlbar",
   ];
 }
 
 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 +68,61 @@ 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);
+  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);
+  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");
 }
+
+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;
+  }
+};
--- a/browser/components/uitour/test/head.js
+++ b/browser/components/uitour/test/head.js
@@ -362,40 +362,57 @@ function setup_UITourTest() {
 
 // Use `add_task(setup_UITourTest);` instead as we will fold this into `setup_UITourTest` once all tests are using `add_UITour_task`.
 function UITourTest(usingAddTask = false) {
   Services.prefs.setBoolPref("browser.uitour.enabled", true);
   let testHttpsUri = Services.io.newURI("https://example.org");
   let testHttpUri = Services.io.newURI("http://example.org");
   Services.perms.add(testHttpsUri, "uitour", Services.perms.ALLOW_ACTION);
   Services.perms.add(testHttpUri, "uitour", Services.perms.ALLOW_ACTION);
+  turnOnPcoketPageAction();
 
   // If a test file is using add_task, we don't need to have a test function or
   // call `waitForExplicitFinish`.
   if (!usingAddTask) {
     waitForExplicitFinish();
   }
 
   registerCleanupFunction(function() {
     delete window.gContentWindow;
     delete window.gContentAPI;
     if (gTestTab)
       gBrowser.removeTab(gTestTab);
     delete window.gTestTab;
     Services.prefs.clearUserPref("browser.uitour.enabled");
     Services.perms.remove(testHttpsUri, "uitour");
     Services.perms.remove(testHttpUri, "uitour");
+    turnOffPcoketPageAction();
   });
 
   // When using tasks, the harness will call the next added task for us.
   if (!usingAddTask) {
     nextTest();
   }
 }
 
+function turnOnPcoketPageAction() {
+  // TODO: After the bug 1385418, should remove the code about Pocket page action
+  Services.prefs.setBoolPref("extensions.pocket.disablePageAction", false);
+  // Pocket would restart when the "extensions.pocket.enabled" changes so
+  // flip it once to have the "extensions.pocket.disablePageAction" take effect.
+  Services.prefs.setBoolPref("extensions.pocket.enabled", false);
+  Services.prefs.setBoolPref("extensions.pocket.enabled", true);
+}
+
+function turnOffPcoketPageAction() {
+  // TODO: After the bug 1385418, should remove the code about Pocket page action
+  Services.prefs.clearUserPref("extensions.pocket.disablePageAction");
+  Services.prefs.clearUserPref("extensions.pocket.enabled");
+}
+
 function done(usingAddTask = false) {
   info("== Done test, doing shared checks before teardown ==");
   return new Promise((resolve) => {
     executeSoon(() => {
       if (gTestTab)
         gBrowser.removeTab(gTestTab);
       gTestTab = null;