Bug 1397501 - Page actions added to the urlbar should always come before the bookmark star. r?Gijs draft
authorDrew Willcoxon <adw@mozilla.com>
Thu, 07 Sep 2017 16:44:34 -0700
changeset 661122 bfa67a9a67bc2104ff0991670bc5f4075a9f08ba
parent 660422 d8e238b811d3dc74515065ae8cab6c74baf0295f
child 730456 18ac822b0227d7ac80c587d9e2e915567de94985
push id78635
push userdwillcoxon@mozilla.com
push dateThu, 07 Sep 2017 23:54:18 +0000
reviewersGijs
bugs1397501
milestone57.0a1
Bug 1397501 - Page actions added to the urlbar should always come before the bookmark star. r?Gijs MozReview-Commit-ID: 6pVlr9d0crn
browser/base/content/browser-pageActions.js
browser/extensions/pocket/bootstrap.js
browser/modules/PageActions.jsm
browser/modules/test/browser/browser_PageActions.js
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -89,17 +89,17 @@ var BrowserPageActions = {
 
   /**
    * Adds or removes as necessary DOM nodes for the action in the panel.
    *
    * @param  action (PageActions.Action, required)
    *         The action to place.
    */
   placeActionInPanel(action) {
-    let insertBeforeID = PageActions.nextActionID(action, PageActions.actions);
+    let insertBeforeID = PageActions.nextActionIDInPanel(action);
     let id = this._panelButtonNodeIDForActionID(action.id);
     let node = document.getElementById(id);
     if (!node) {
       let panelViewNode;
       [node, panelViewNode] = this._makePanelButtonNodeForAction(action);
       node.id = id;
       let insertBeforeNode = null;
       if (insertBeforeID) {
@@ -108,17 +108,16 @@ var BrowserPageActions = {
         insertBeforeNode = document.getElementById(insertBeforeNodeID);
       }
       this.mainViewBodyNode.insertBefore(node, insertBeforeNode);
       action.onPlacedInPanel(node);
       if (panelViewNode) {
         action.subview.onPlaced(panelViewNode);
       }
     }
-    return node;
   },
 
   _makePanelButtonNodeForAction(action) {
     if (action.__isSeparator) {
       let node = document.createElement("toolbarseparator");
       return [node, null];
     }
 
@@ -295,30 +294,29 @@ var BrowserPageActions = {
 
   /**
    * Adds or removes as necessary a DOM node for the given action in the urlbar.
    *
    * @param  action (PageActions.Action, required)
    *         The action to place.
    */
   placeActionInUrlbar(action) {
-    let insertBeforeID =
-      PageActions.nextActionID(action, PageActions.actionsInUrlbar);
+    let insertBeforeID = PageActions.nextActionIDInUrlbar(action);
     let id = this._urlbarButtonNodeIDForActionID(action.id);
     let node = document.getElementById(id);
 
     if (!action.shownInUrlbar) {
       if (node) {
         if (action.__urlbarNodeInMarkup) {
           node.hidden = true;
         } else {
           node.remove();
         }
       }
-      return null;
+      return;
     }
 
     let newlyPlaced = false;
     if (action.__urlbarNodeInMarkup) {
       newlyPlaced = node && node.hidden;
       node.hidden = false;
     } else if (!node) {
       newlyPlaced = true;
@@ -344,18 +342,16 @@ var BrowserPageActions = {
       if (!node.hasAttribute("tooltiptext")) {
         let panelNodeID = this._panelButtonNodeIDForActionID(action.id);
         let panelNode = document.getElementById(panelNodeID);
         if (panelNode) {
           node.setAttribute("tooltiptext", panelNode.getAttribute("label"));
         }
       }
     }
-
-    return node;
   },
 
   _makeUrlbarButtonNode(action) {
     let buttonNode = document.createElement("image");
     buttonNode.classList.add("urlbar-icon", "urlbar-page-action");
     buttonNode.setAttribute("role", "button");
     if (action.tooltip) {
       buttonNode.setAttribute("tooltiptext", action.tooltip);
--- a/browser/extensions/pocket/bootstrap.js
+++ b/browser/extensions/pocket/bootstrap.js
@@ -94,17 +94,16 @@ var PocketPageAction = {
       this.pageAction = PageActions.addAction(new PageActions.Action({
         id,
         title: gPocketBundle.GetStringFromName("saveToPocketCmd.label"),
         shownInUrlbar: true,
         wantsIframe: true,
         urlbarIDOverride: "pocket-button-box",
         anchorIDOverride: "pocket-button",
         _insertBeforeActionID: PageActions.ACTION_ID_BOOKMARK_SEPARATOR,
-        _urlbarInsertBeforeActionID: PageActions.ACTION_ID_BOOKMARK,
         _urlbarNodeInMarkup: true,
         onBeforePlacedInWindow(window) {
           let doc = window.document;
 
           if (doc.getElementById("pocket-button-box")) {
             return;
           }
 
--- a/browser/modules/PageActions.jsm
+++ b/browser/modules/PageActions.jsm
@@ -25,16 +25,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
   "resource://gre/modules/BinarySearch.jsm");
 
 
 const ACTION_ID_BOOKMARK = "bookmark";
 const ACTION_ID_BOOKMARK_SEPARATOR = "bookmarkSeparator";
 const ACTION_ID_BUILT_IN_SEPARATOR = "builtInSeparator";
 
 const PREF_PERSISTED_ACTIONS = "browser.pageActions.persistedActions";
+const PERSISTED_ACTIONS_CURRENT_VERSION = 1
 
 
 this.PageActions = {
   /**
    * Inits.  Call to init.
    */
   init() {
     let callbacks = this._deferredAddActionCalls;
@@ -105,21 +106,16 @@ this.PageActions = {
     return this._nonBuiltInActions.slice();
   },
 
   /**
    * The list of actions in the urlbar, sorted in the order in which they should
    * appear there.  Not live.  (array of Action objects)
    */
   get actionsInUrlbar() {
-    if (!this._persistedActions) {
-      // This is the case before init is called.  No one should be calling us
-      // then, but return something sensible.
-      return [];
-    }
     // Remember that IDs in idsInUrlbar may belong to actions that aren't
     // currently registered.
     return this._persistedActions.idsInUrlbar.reduce((actions, id) => {
       let action = this.actionForID(id);
       if (action) {
         actions.push(action);
       }
       return actions;
@@ -214,48 +210,84 @@ this.PageActions = {
       // A non-built-in action, like a non-bundled extension potentially.
       // Keep this list sorted by title.
       let index = BinarySearch.insertionIndexOf((a1, a2) => {
         return a1.title.localeCompare(a2.title);
       }, this._nonBuiltInActions, action);
       this._nonBuiltInActions.splice(index, 0, action);
     }
 
-    if (this._persistedActions.ids[action.id]) {
+    if (this._persistedActions.ids.includes(action.id)) {
       // The action has been seen before.  Override its shownInUrlbar value
       // with the persisted value.  Set the private version of that property
       // so that onActionToggledShownInUrlbar isn't called, which happens when
       // the public version is set.
       action._shownInUrlbar =
         this._persistedActions.idsInUrlbar.includes(action.id);
     } else {
       // The action is new.  Store it in the persisted actions.
-      this._persistedActions.ids[action.id] = true;
-      if (action.shownInUrlbar) {
-        // Also store it in idsInUrlbar.
-        let index =
-          !action.__urlbarInsertBeforeActionID ? -1 :
-          this._persistedActions.idsInUrlbar.indexOf(
-            action.__urlbarInsertBeforeActionID
-          );
-        if (index < 0) {
-          // Append the action.
-          index = this._persistedActions.idsInUrlbar.length;
+      this._updateIDsInUrlbarForAction(action);
+    }
+  },
+
+  _updateIDsInUrlbarForAction(action) {
+    let index = this._persistedActions.idsInUrlbar.indexOf(action.id);
+    if (action.shownInUrlbar) {
+      if (index < 0) {
+        let nextID = this.nextActionIDInUrlbar(action.id);
+        let nextIndex =
+          nextID ? this._persistedActions.idsInUrlbar.indexOf(nextID) : -1;
+        if (nextIndex < 0) {
+          nextIndex = this._persistedActions.idsInUrlbar.length;
         }
-        this._persistedActions.idsInUrlbar.splice(index, 0, action.id);
+        this._persistedActions.idsInUrlbar.splice(nextIndex, 0, action.id);
       }
-      this._storePersistedActions();
+    } else if (index >= 0) {
+      this._persistedActions.idsInUrlbar.splice(index, 1);
     }
+    this._storePersistedActions();
   },
 
   _builtInActions: [],
   _nonBuiltInActions: [],
   _actionsByID: new Map(),
 
   /**
+   * Returns the ID of the action before which the given action should be
+   * inserted in the urlbar.
+   *
+   * @param  action (Action object, required)
+   *         The action you're inserting.
+   * @return The ID of the reference action, or null if your action should be
+   *         appended.
+   */
+  nextActionIDInUrlbar(action) {
+    // Actions in the urlbar are always inserted before the bookmark action,
+    // which always comes last if it's present.
+    if (action.id == ACTION_ID_BOOKMARK) {
+      return null;
+    }
+    let id = this._nextActionID(action, this.actionsInUrlbar);
+    return id || ACTION_ID_BOOKMARK;
+  },
+
+  /**
+   * Returns the ID of the action before which the given action should be
+   * inserted in the panel.
+   *
+   * @param  action (Action object, required)
+   *         The action you're inserting.
+   * @return The ID of the reference action, or null if your action should be
+   *         appended.
+   */
+  nextActionIDInPanel(action) {
+    return this._nextActionID(action, this.actions);
+  },
+
+  /**
    * The DOM nodes of actions should be ordered properly, both in the panel and
    * the urlbar.  This method returns the ID of the action that comes after the
    * given action in the given array.  You can use the returned ID to get a DOM
    * node ID to pass to node.insertBefore().
    *
    * Pass PageActions.actions to get the ID of the next action in the panel.
    * Pass PageActions.actionsInUrlbar to get the ID of the next action in the
    * urlbar.
@@ -264,17 +296,17 @@ this.PageActions = {
    *         The action whose node you want to insert into your DOM.
    * @param  actionArray
    *         The relevant array of actions, either PageActions.actions or
    *         actionsInUrlbar.
    * @return The ID of the action before which the given action should be
    *         inserted.  If the given action should be inserted last, returns
    *         null.
    */
-  nextActionID(action, actionArray) {
+  _nextActionID(action, actionArray) {
     let index = actionArray.findIndex(a => a.id == action.id);
     if (index < 0) {
       return null;
     }
     let nextAction = actionArray[index + 1];
     if (!nextAction) {
       return null;
     }
@@ -298,20 +330,22 @@ this.PageActions = {
       let index = list.findIndex(a => a.id == action.id);
       if (index >= 0) {
         list.splice(index, 1);
         break;
       }
     }
 
     // Remove the action from persisted storage.
-    delete this._persistedActions.ids[action.id];
-    let index = this._persistedActions.idsInUrlbar.indexOf(action.id);
-    if (index >= 0) {
-      this._persistedActions.idsInUrlbar.splice(index, 1);
+    for (let name of ["ids", "idsInUrlbar"]) {
+      let array = this._persistedActions[name];
+      let index = array.indexOf(action.id);
+      if (index >= 0) {
+        array.splice(index, 1);
+      }
     }
     this._storePersistedActions();
 
     for (let bpa of allBrowserPageActions()) {
       bpa.removeAction(action);
     }
   },
 
@@ -353,48 +387,71 @@ this.PageActions = {
    * @param  action (Action object, required)
    *         The action whose shownInUrlbar property changed.
    */
   onActionToggledShownInUrlbar(action) {
     if (!this.actionForID(action.id)) {
       // This may be called before the action has been added.
       return;
     }
-
-    // Update persisted storage.
-    let index = this._persistedActions.idsInUrlbar.indexOf(action.id);
-    if (action.shownInUrlbar) {
-      if (index < 0) {
-        this._persistedActions.idsInUrlbar.push(action.id);
-      }
-    } else if (index >= 0) {
-      this._persistedActions.idsInUrlbar.splice(index, 1);
-    }
-    this._storePersistedActions();
-
+    this._updateIDsInUrlbarForAction(action);
     for (let bpa of allBrowserPageActions()) {
       bpa.placeActionInUrlbar(action);
     }
   },
 
   _storePersistedActions() {
     let json = JSON.stringify(this._persistedActions);
     Services.prefs.setStringPref(PREF_PERSISTED_ACTIONS, json);
   },
 
   _loadPersistedActions() {
     try {
       let json = Services.prefs.getStringPref(PREF_PERSISTED_ACTIONS);
-      this._persistedActions = JSON.parse(json);
+      this._persistedActions = this._migratePersistedActions(JSON.parse(json));
     } catch (ex) {}
   },
 
+  _migratePersistedActions(actions) {
+    // Start with actions.version and migrate one version at a time, all the way
+    // up to the current version.
+    for (let version = actions.version || 0;
+         version < PERSISTED_ACTIONS_CURRENT_VERSION;
+         version++) {
+      let methodName = `_migratePersistedActionsTo${version + 1}`;
+      actions = this[methodName](actions);
+      actions.version = version + 1;
+    }
+    return actions;
+  },
+
+  _migratePersistedActionsTo1(actions) {
+    // The `ids` object is a mapping: action ID => true.  Convert it to an array
+    // to save space in the prefs.
+    let ids = [];
+    for (let id in actions.ids) {
+      ids.push(id);
+    }
+    // Move the bookmark ID to the end of idsInUrlbar.  The bookmark action
+    // should always remain at the end of the urlbar, if present.
+    let bookmarkIndex = actions.idsInUrlbar.indexOf(ACTION_ID_BOOKMARK);
+    if (bookmarkIndex >= 0) {
+      actions.idsInUrlbar.splice(bookmarkIndex, 1);
+      actions.idsInUrlbar.push(ACTION_ID_BOOKMARK);
+    }
+    return {
+      ids,
+      idsInUrlbar: actions.idsInUrlbar,
+    };
+  },
+
   _persistedActions: {
-    // action ID => true, for actions that have ever been seen and not removed
-    ids: {},
+    version: PERSISTED_ACTIONS_CURRENT_VERSION,
+    // action IDs that have ever been seen and not removed, order not important
+    ids: [],
     // action IDs ordered by position in urlbar
     idsInUrlbar: [],
   },
 };
 
 /**
  * A single page action.
  *
@@ -497,21 +554,16 @@ function Action(options) {
 
     // private
 
     // (string, optional)
     // The ID of another action before which to insert this new action in the
     // panel.
     _insertBeforeActionID: false,
 
-    // (string, optional)
-    // The ID of another action before which to insert this new action in the
-    // urlbar.
-    _urlbarInsertBeforeActionID: false,
-
     // (bool, optional)
     // True if this isn't really an action but a separator to be shown in the
     // page action panel.
     _isSeparator: false,
 
     // (bool, optional)
     // True if the action's urlbar button is defined in markup.  In that case, a
     // node with the action's urlbar node ID should already exist in the DOM
@@ -892,16 +944,17 @@ Button.prototype = {
 };
 
 this.PageActions.Button = Button;
 
 
 // These are only necessary so that Pocket and the test can use them.
 this.PageActions.ACTION_ID_BOOKMARK = ACTION_ID_BOOKMARK;
 this.PageActions.ACTION_ID_BOOKMARK_SEPARATOR = ACTION_ID_BOOKMARK_SEPARATOR;
+this.PageActions.PREF_PERSISTED_ACTIONS = PREF_PERSISTED_ACTIONS;
 
 // This is only necessary so that the test can access it.
 this.PageActions.ACTION_ID_BUILT_IN_SEPARATOR = ACTION_ID_BUILT_IN_SEPARATOR;
 
 
 // Sorted in the order in which they should appear in the page action panel.
 // Does not include the page actions of extensions bundled with the browser.
 // They're added by the relevant extension code.
--- a/browser/modules/test/browser/browser_PageActions.js
+++ b/browser/modules/test/browser/browser_PageActions.js
@@ -154,16 +154,26 @@ add_task(async function simple() {
   Assert.notEqual(urlbarButtonNode, null, "urlbarButtonNode");
   for (let name in action.nodeAttributes) {
     Assert.ok(urlbarButtonNode.hasAttribute(name), name,
               "Has attribute: " + name);
     Assert.equal(urlbarButtonNode.getAttribute(name),
                  action.nodeAttributes[name],
                  "Equal attribute: " + name);
   }
+
+  // The button should have been inserted before the bookmark star.
+  Assert.notEqual(urlbarButtonNode.nextSibling, null, "Should be a next node");
+  Assert.equal(
+    urlbarButtonNode.nextSibling.id,
+    PageActions.actionForID(PageActions.ACTION_ID_BOOKMARK).urlbarIDOverride,
+    "Next node should be the bookmark star"
+  );
+
+  // Click the urlbar button.
   onCommandExpectedButtonID = urlbarButtonID;
   EventUtils.synthesizeMouseAtCenter(urlbarButtonNode, {});
   Assert.equal(onCommandCallCount, 2, "onCommandCallCount should be inc'ed");
 
   // Set a new title.
   let newTitle = title + " new title";
   action.title = newTitle;
   Assert.equal(action.title, newTitle, "New title");
@@ -312,16 +322,24 @@ add_task(async function withSubview() {
   let panelViewButtonNodePanel =
     document.getElementById(panelViewButtonIDPanel);
   Assert.notEqual(panelViewButtonNodePanel, null, "panelViewButtonNodePanel");
 
   // The action's urlbar button should have been created.
   let urlbarButtonNode = document.getElementById(urlbarButtonID);
   Assert.notEqual(urlbarButtonNode, null, "urlbarButtonNode");
 
+  // The button should have been inserted before the bookmark star.
+  Assert.notEqual(urlbarButtonNode.nextSibling, null, "Should be a next node");
+  Assert.equal(
+    urlbarButtonNode.nextSibling.id,
+    PageActions.actionForID(PageActions.ACTION_ID_BOOKMARK).urlbarIDOverride,
+    "Next node should be the bookmark star"
+  );
+
   // Open the panel, click the action's button, click the subview's first
   // button.
   await promisePageActionPanelOpen();
   Assert.equal(onSubviewShowingCount, 0,
                "onSubviewShowingCount should remain 0");
   let subviewShownPromise = promisePageActionViewShown();
   onSubviewShowingExpectedPanelViewID = panelViewIDPanel;
 
@@ -433,16 +451,24 @@ add_task(async function withIframe() {
   // The action's panel button should have been created.
   let panelButtonNode = document.getElementById(panelButtonID);
   Assert.notEqual(panelButtonNode, null, "panelButtonNode");
 
   // The action's urlbar button should have been created.
   let urlbarButtonNode = document.getElementById(urlbarButtonID);
   Assert.notEqual(urlbarButtonNode, null, "urlbarButtonNode");
 
+  // The button should have been inserted before the bookmark star.
+  Assert.notEqual(urlbarButtonNode.nextSibling, null, "Should be a next node");
+  Assert.equal(
+    urlbarButtonNode.nextSibling.id,
+    PageActions.actionForID(PageActions.ACTION_ID_BOOKMARK).urlbarIDOverride,
+    "Next node should be the bookmark star"
+  );
+
   // Open the panel, click the action's button.
   await promisePageActionPanelOpen();
   Assert.equal(onIframeShownCount, 0, "onIframeShownCount should remain 0");
   EventUtils.synthesizeMouseAtCenter(panelButtonNode, {});
   await promisePanelShown(BrowserPageActions._activatedActionPanelID);
   Assert.equal(onCommandCallCount, 0, "onCommandCallCount should remain 0");
   Assert.equal(onIframeShownCount, 1, "onIframeShownCount should be inc'ed");
 
@@ -649,16 +675,17 @@ add_task(async function multipleNonBuilt
         PageActions.ACTION_ID_BUILT_IN_SEPARATOR
       )
     ),
     null,
     "Separator should be gone"
   );
 });
 
+
 // Makes sure the panel is correctly updated when a non-built-in action is
 // added before the built-in actions; and when all built-in actions are removed
 // and added back.
 add_task(async function nonBuiltFirst() {
   let initialActions = PageActions.actions;
 
   // Remove all actions.
   for (let action of initialActions) {
@@ -768,38 +795,39 @@ add_task(async function urlbarOrderNewWi
   let actions = [0, 1, 2].map(i => {
     return PageActions.addAction(new PageActions.Action({
       id: `test-urlbarOrderNewWindow-${i}`,
       title: `Test urlbarOrderNewWindow ${i}`,
       shownInUrlbar: true,
     }));
   });
 
-  // Make sure PageActions knows they're appended to the urlbar actions.
+  // Make sure PageActions knows they're inserted before the bookmark action in
+  // the urlbar.
   Assert.deepEqual(
     PageActions._persistedActions.idsInUrlbar.slice(
-      PageActions._persistedActions.idsInUrlbar.length - actions.length
+      PageActions._persistedActions.idsInUrlbar.length - (actions.length + 1)
     ),
-    actions.map(a => a.id),
-    "PageActions._persistedActions.idsInUrlbar has new actions appended"
+    actions.map(a => a.id).concat([PageActions.ACTION_ID_BOOKMARK]),
+    "PageActions._persistedActions.idsInUrlbar has new actions inserted"
   );
   Assert.deepEqual(
     PageActions.actionsInUrlbar.slice(
-      PageActions.actionsInUrlbar.length - actions.length
+      PageActions.actionsInUrlbar.length - (actions.length + 1)
     ).map(a => a.id),
-    actions.map(a => a.id),
-    "PageActions.actionsInUrlbar has new actions appended"
+    actions.map(a => a.id).concat([PageActions.ACTION_ID_BOOKMARK]),
+    "PageActions.actionsInUrlbar has new actions inserted"
   );
 
   // Reach into _persistedActions to move the new actions to the front of the
   // urlbar, same as if the user moved them.  That way we can test that insert-
   // before IDs are correctly non-null when the urlbar nodes are inserted in the
   // new window below.
   PageActions._persistedActions.idsInUrlbar.splice(
-    PageActions._persistedActions.idsInUrlbar.length - actions.length,
+    PageActions._persistedActions.idsInUrlbar.length - (actions.length + 1),
     actions.length
   );
   for (let i = 0; i < actions.length; i++) {
     PageActions._persistedActions.idsInUrlbar.splice(i, 0, actions[i].id);
   }
 
   // Save the right-ordered IDs to use below, just in case they somehow get
   // changed when the new window opens, which shouldn't happen, but maybe
@@ -834,16 +862,95 @@ add_task(async function urlbarOrderNewWi
   // Done, clean up.
   await BrowserTestUtils.closeWindow(win);
   for (let action of actions) {
     action.remove();
   }
 });
 
 
+// Stores version-0 (unversioned actually) persisted actions and makes sure that
+// migrating to version 1 works.
+add_task(async function migrate1() {
+  // Add the bookmark action first to make sure it ends up last after migration.
+  // Also include a non-default action (copyURL) to make sure we're not
+  // accidentally testing default behavior.
+  let ids = [
+    PageActions.ACTION_ID_BOOKMARK,
+    "pocket",
+    "copyURL",
+  ];
+  let persisted = ids.reduce((memo, id) => {
+    memo.ids[id] = true;
+    memo.idsInUrlbar.push(id);
+    return memo;
+  }, { ids: {}, idsInUrlbar: [] });
+
+  Services.prefs.setStringPref(
+    PageActions.PREF_PERSISTED_ACTIONS,
+    JSON.stringify(persisted)
+  );
+
+  // Migrate.
+  PageActions._loadPersistedActions();
+
+  Assert.equal(PageActions._persistedActions.version, 1, "Correct version");
+
+  // Need to set copyURL's _shownInUrlbar.  It won't be set since it's false by
+  // default and we reached directly into persisted storage above.
+  PageActions.actionForID("copyURL")._shownInUrlbar = true;
+
+  // expected order
+  let orderedIDs = [
+    "pocket",
+    "copyURL",
+    PageActions.ACTION_ID_BOOKMARK,
+  ];
+
+  // Check the ordering.
+  Assert.deepEqual(
+    PageActions._persistedActions.idsInUrlbar,
+    orderedIDs,
+    "PageActions._persistedActions.idsInUrlbar has right order"
+  );
+  Assert.deepEqual(
+    PageActions.actionsInUrlbar.map(a => a.id),
+    orderedIDs,
+    "PageActions.actionsInUrlbar has right order"
+  );
+
+  // Open a new window.
+  let win = await BrowserTestUtils.openNewBrowserWindow();
+  await BrowserTestUtils.openNewForegroundTab({
+    gBrowser: win.gBrowser,
+    url: "http://example.com/",
+  });
+
+  // Collect its urlbar nodes.
+  let actualUrlbarNodeIDs = [];
+  for (let node = win.BrowserPageActions.mainButtonNode.nextSibling;
+       node;
+       node = node.nextSibling) {
+    actualUrlbarNodeIDs.push(node.id);
+  }
+
+  // Now check that they're in the right order.
+  Assert.deepEqual(
+    actualUrlbarNodeIDs,
+    orderedIDs.map(id => win.BrowserPageActions._urlbarButtonNodeIDForActionID(id)),
+    "Expected actions in new window's urlbar"
+  );
+
+  // Done, clean up.
+  await BrowserTestUtils.closeWindow(win);
+  Services.prefs.clearUserPref(PageActions.PREF_PERSISTED_ACTIONS);
+  PageActions.actionForID("copyURL")._shownInUrlbar = false;
+});
+
+
 function promisePageActionPanelOpen() {
   let button = document.getElementById("pageActionButton");
   // The main page action button is hidden for some URIs, so make sure it's
   // visible before trying to click it.
   let dwu = window.QueryInterface(Ci.nsIInterfaceRequestor)
                   .getInterface(Ci.nsIDOMWindowUtils);
   return BrowserTestUtils.waitForCondition(() => {
     info("Waiting for main page action button to have non-0 size");