Bug 1421664 - 3 - Fix the UI and tests. r=standard8
authorMarco Bonardo <mbonardo@mozilla.com>
Thu, 22 Mar 2018 14:13:07 +0100
changeset 410770 147023fe220ca9b4235aa2900972a4affc79bab3
parent 410769 4b1ce6bf8518e00a5c5125d905173d28a11e72e2
child 410771 fa72a782c21cb6b46204aab9c537579df09db81c
push id33739
push usernbeleuzu@mozilla.com
push dateFri, 30 Mar 2018 21:47:45 +0000
treeherdermozilla-central@10c662d8416e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersstandard8
bugs1421664
milestone61.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 1421664 - 3 - Fix the UI and tests. r=standard8 test_419731.js can be removed since it is already covered by browser_bookmarkProperties_editTagContainer.js MozReview-Commit-ID: K0LFuTptWyW
browser/components/places/content/browserPlacesViews.js
browser/components/places/content/controller.js
browser/components/places/content/editBookmark.js
browser/components/places/content/tree.xml
browser/components/places/content/treeView.js
browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
browser/components/places/tests/browser/browser_drag_bookmarks_on_toolbar.js
browser/components/places/tests/browser/browser_library_delete_bookmarks_in_tags.js
toolkit/components/places/PlacesTransactions.jsm
toolkit/components/places/PlacesUtils.jsm
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/tests/unit/test_419731.js
toolkit/components/places/tests/unit/test_async_transactions.js
toolkit/components/places/tests/unit/test_resolveNullBookmarkTitles.js
toolkit/components/places/tests/unit/xpcshell.ini
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -200,20 +200,17 @@ PlacesViewBase.prototype = {
         // the insertion point is inside the folder, at the end.
         container = selectedNode;
         orientation = Ci.nsITreeView.DROP_ON;
       } else {
         // In all other cases the insertion point is before that node.
         container = selectedNode.parent;
         index = container.getChildIndex(selectedNode);
         if (PlacesUtils.nodeIsTagQuery(container)) {
-          tagName = container.title;
-          // TODO (Bug 1160193): properly support dropping on a tag root.
-          if (!tagName)
-            return null;
+          tagName = PlacesUtils.asQuery(container).query.tags[0];
         }
       }
     }
 
     if (this.controller.disallowInsertion(container))
       return null;
 
     return new PlacesInsertionPoint({
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -170,17 +170,18 @@ PlacesController.prototype = {
       return this._canInsert();
     case "placesCmd_new:separator":
       return this._canInsert() &&
              !PlacesUtils.asQuery(this._view.result.root).queryOptions.excludeItems &&
              this._view.result.sortingMode ==
                  Ci.nsINavHistoryQueryOptions.SORT_BY_NONE;
     case "placesCmd_show:info": {
       let selectedNode = this._view.selectedNode;
-      return selectedNode && PlacesUtils.getConcreteItemId(selectedNode) != -1;
+      return selectedNode && (PlacesUtils.nodeIsTagQuery(selectedNode) ||
+                              PlacesUtils.getConcreteItemId(selectedNode) != -1);
     }
     case "placesCmd_reload": {
       // Livemark containers
       let selectedNode = this._view.selectedNode;
       return selectedNode && this.hasCachedLivemarkInfo(selectedNode);
     }
     case "placesCmd_sortBy:name": {
       let selectedNode = this._view.selectedNode;
@@ -802,46 +803,55 @@ PlacesController.prototype = {
       if (this._shouldSkipNode(node, removedFolders))
         continue;
 
       totalItems++;
 
       if (PlacesUtils.nodeIsTagQuery(node.parent)) {
         // This is a uri node inside a tag container.  It needs a special
         // untag transaction.
-        let tag = node.parent.title;
+        let tag = node.parent.title || "";
         if (!tag) {
-          // TODO: Bug 1432405 Try using getConcreteItemGuid.
-          let tagItemId = PlacesUtils.getConcreteItemId(node.parent);
-          let tagGuid = await PlacesUtils.promiseItemGuid(tagItemId);
-          tag = (await PlacesUtils.bookmarks.fetch(tagGuid)).title;
+          // The parent may be the root node, that doesn't have a title.
+          // Until we fix bug 1293445, we have two ways to get tags:
+          if (node.parent.queryOptions.resultType ==
+                Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_CONTENTS) {
+            // Get the tag using the bookmarks API.
+            let tagItemId = PlacesUtils.getConcreteItemId(node.parent);
+            let tagGuid = await PlacesUtils.promiseItemGuid(tagItemId);
+            tag = (await PlacesUtils.bookmarks.fetch(tagGuid)).title;
+          } else {
+            // Extract the tag from the query itself.
+            tag = node.parent.query.tags[0];
+          }
         }
         transactions.push(PlacesTransactions.Untag({ urls: [node.uri], tag }));
-      } else if (PlacesUtils.nodeIsTagQuery(node) && node.parent &&
-               PlacesUtils.nodeIsQuery(node.parent) &&
-               PlacesUtils.asQuery(node.parent).queryOptions.resultType ==
-                 Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY) {
+      } else if (PlacesUtils.nodeIsTagQuery(node) &&
+                 node.parent &&
+                 PlacesUtils.nodeIsQuery(node.parent) &&
+                 PlacesUtils.asQuery(node.parent).queryOptions.resultType ==
+                   Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY) {
         // This is a tag container.
         // Untag all URIs tagged with this tag only if the tag container is
         // child of the "Tags" query in the library, in all other places we
         // must only remove the query node.
         let tag = node.title;
         let URIs = PlacesUtils.tagging.getURIsForTag(tag);
         transactions.push(PlacesTransactions.Untag({ tag, urls: URIs }));
       } else if (PlacesUtils.nodeIsURI(node) &&
-               PlacesUtils.nodeIsQuery(node.parent) &&
-               PlacesUtils.asQuery(node.parent).queryOptions.queryType ==
-                 Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
+                 PlacesUtils.nodeIsQuery(node.parent) &&
+                 PlacesUtils.asQuery(node.parent).queryOptions.queryType ==
+                   Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
         // This is a uri node inside an history query.
         PlacesUtils.history.remove(node.uri).catch(Cu.reportError);
         // History deletes are not undoable, so we don't have a transaction.
       } else if (node.itemId == -1 &&
-               PlacesUtils.nodeIsQuery(node) &&
-               PlacesUtils.asQuery(node).queryOptions.queryType ==
-                 Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
+                 PlacesUtils.nodeIsQuery(node) &&
+                 PlacesUtils.asQuery(node).queryOptions.queryType ==
+                   Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY) {
         // This is a dynamically generated history query, like queries
         // grouped by site, time or both.  Dynamically generated queries don't
         // have an itemId even if they are descendants of a bookmark.
         this._removeHistoryContainer(node);
         // History deletes are not undoable, so we don't have a transaction.
       } else {
         // This is a common bookmark item.
         if (PlacesUtils.nodeIsFolder(node)) {
--- a/browser/components/places/content/editBookmark.js
+++ b/browser/components/places/content/editBookmark.js
@@ -28,23 +28,22 @@ var gEditItemOverlay = {
     // folders), when the pane shows for them it's opened in read-only mode, showing the
     // properties of the target folder.
     let itemId = node ? node.itemId : -1;
     let itemGuid = node ? PlacesUtils.getConcreteItemGuid(node) : null;
     let isItem = itemId != -1;
     let isFolderShortcut = isItem &&
                            node.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT;
     let isTag = node && PlacesUtils.nodeIsTagQuery(node);
+    let tag = null;
     if (isTag) {
-      itemId = PlacesUtils.getConcreteItemId(node);
-      // For now we don't have access to the item guid synchronously for tags,
-      // so we'll need to fetch it later.
+      tag = PlacesUtils.asQuery(node).query.tags.length == 1 ? node.query.tags[0] : node.title;
     }
     let isURI = node && PlacesUtils.nodeIsURI(node);
-    let uri = isURI ? Services.io.newURI(node.uri) : null;
+    let uri = isURI || isTag ? Services.io.newURI(node.uri) : null;
     let title = node ? node.title : null;
     let isBookmark = isItem && isURI;
     let bulkTagging = !node;
     let uris = bulkTagging ? aInitInfo.uris : null;
     let visibleRows = new Set();
     let isParentReadOnly = false;
     let postData = aInitInfo.postData;
     let parentId = -1;
@@ -63,26 +62,26 @@ var gEditItemOverlay = {
     let focusedElement = aInitInfo.focusedElement;
     let onPanelReady = aInitInfo.onPanelReady;
 
     return this._paneInfo = { itemId, itemGuid, parentId, parentGuid, isItem,
                               isURI, uri, title,
                               isBookmark, isFolderShortcut, isParentReadOnly,
                               bulkTagging, uris,
                               visibleRows, postData, isTag, focusedElement,
-                              onPanelReady };
+                              onPanelReady, tag };
   },
 
   get initialized() {
     return this._paneInfo != null;
   },
 
   // Backwards-compatibility getters
   get itemId() {
-    if (!this.initialized || this._paneInfo.bulkTagging)
+    if (!this.initialized || this._paneInfo.isTag || this._paneInfo.bulkTagging)
       return -1;
     return this._paneInfo.itemId;
   },
 
   get uri() {
     if (!this.initialized)
       return null;
     if (this._paneInfo.bulkTagging)
@@ -115,17 +114,17 @@ var gEditItemOverlay = {
   // a certain item
   _firstEditedField: "",
 
   _initNamePicker() {
     if (this._paneInfo.bulkTagging)
       throw new Error("_initNamePicker called unexpectedly");
 
     // title may by null, which, for us, is the same as an empty string.
-    this._initTextField(this._namePicker, this._paneInfo.title || "");
+    this._initTextField(this._namePicker, this._paneInfo.title || this._paneInfo.tag || "");
   },
 
   _initLocationField() {
     if (!this._paneInfo.isURI)
       throw new Error("_initLocationField called unexpectedly");
     this._initTextField(this._locationField, this._paneInfo.uri.spec);
   },
 
@@ -602,28 +601,38 @@ var gEditItemOverlay = {
     Services.prefs.setCharPref("browser.bookmarks.editDialog.firstEditField", aNewField);
   },
 
   async onNamePickerChange() {
     if (this.readOnly || !(this._paneInfo.isItem || this._paneInfo.isTag))
       return;
 
     // Here we update either the item title or its cached static title
-    let newTitle = this._namePicker.value;
-    if (!newTitle && this._paneInfo.isTag) {
-      // We don't allow setting an empty title for a tag, restore the old one.
-      this._initNamePicker();
-    } else {
-      this._mayUpdateFirstEditField("namePicker");
+    if (this._paneInfo.isTag) {
+      let tag = this._namePicker.value;
+      if (!tag || tag.includes("&")) {
+        // We don't allow setting an empty title for a tag, restore the old one.
+        this._initNamePicker();
+        return;
+      }
+      // Get all the bookmarks for the old tag, tag them with the new tag, and
+      // untag them from the old tag.
+      let oldTag = this._paneInfo.tag;
+      this._paneInfo.tag = tag;
+      let title = this._paneInfo.title;
+      if (title == oldTag) {
+        this._paneInfo.title = tag;
+      }
+      await PlacesTransactions.RenameTag({ oldTag, tag }).transact();
+      return;
+    }
 
-      let guid = this._paneInfo.isTag
-                  ? (await PlacesUtils.promiseItemGuid(this._paneInfo.itemId))
-                  : this._paneInfo.itemGuid;
-      await PlacesTransactions.EditTitle({ guid, title: newTitle }).transact();
-    }
+    this._mayUpdateFirstEditField("namePicker");
+    await PlacesTransactions.EditTitle({ guid: this._paneInfo.itemGuid,
+                                         title: this._namePicker.value }).transact();
   },
 
   onDescriptionFieldChange() {
     if (this.readOnly || !this._paneInfo.isItem)
       return;
 
     let description = this._element("descriptionField").value;
     if (description != PlacesUIUtils.getItemDescription(this._paneInfo.itemId)) {
@@ -1015,18 +1024,18 @@ var gEditItemOverlay = {
       this._initTagsField();
       // Any tags change should be reflected in the tags selector.
       if (this._element("tagsSelector")) {
         this._rebuildTagsSelectorList();
       }
     }
   },
 
-  _onItemTitleChange(aItemId, aNewTitle) {
-    if (aItemId == this._paneInfo.itemId) {
+  _onItemTitleChange(aItemId, aNewTitle, aGuid) {
+    if (aItemId == this._paneInfo.itemId || aGuid == this._paneInfo.itemGuid) {
       this._paneInfo.title = aNewTitle;
       this._initTextField(this._namePicker, aNewTitle);
     } else if (this._paneInfo.visibleRows.has("folderRow")) {
       // If the title of a folder which is listed within the folders
       // menulist has been changed, we need to update the label of its
       // representing element.
       let menupopup = this._folderMenuList.menupopup;
       for (let menuitem of menupopup.childNodes) {
@@ -1051,17 +1060,17 @@ var gEditItemOverlay = {
   onItemChanged(aItemId, aProperty, aIsAnnotationProperty, aValue,
                 aLastModified, aItemType, aParentId, aGuid) {
     if (aProperty == "tags" && this._paneInfo.visibleRows.has("tagsRow")) {
       this._onTagsChange(aGuid).catch(Cu.reportError);
       return;
     }
     if (aProperty == "title" && (this._paneInfo.isItem || this._paneInfo.isTag)) {
       // This also updates titles of folders in the folder menu list.
-      this._onItemTitleChange(aItemId, aValue);
+      this._onItemTitleChange(aItemId, aValue, aGuid);
       return;
     }
 
     if (!this._paneInfo.isItem || this._paneInfo.itemId != aItemId) {
       return;
     }
 
     switch (aProperty) {
@@ -1120,16 +1129,15 @@ var gEditItemOverlay = {
   },
 
   onItemRemoved() { },
   onBeginUpdateBatch() { },
   onEndUpdateBatch() { },
   onItemVisited() { },
 };
 
-
 for (let elt of ["folderMenuList", "folderTree", "namePicker",
                  "locationField", "descriptionField", "keywordField",
                  "tagsField", "loadInSidebarCheckbox"]) {
   let eltScoped = elt;
   XPCOMUtils.defineLazyGetter(gEditItemOverlay, `_${eltScoped}`,
                               () => gEditItemOverlay._element(eltScoped));
 }
--- a/browser/components/places/content/tree.xml
+++ b/browser/components/places/content/tree.xml
@@ -78,18 +78,18 @@
           // preserve grouping
           var queryNode = PlacesUtils.asQuery(this.result.root);
           var options = queryNode.queryOptions.clone();
 
           // Make sure we're getting uri results.
           // We do not yet support searching into grouped queries or into
           // tag containers, so we must fall to the default case.
           if (PlacesUtils.nodeIsHistoryContainer(queryNode) ||
+              PlacesUtils.nodeIsTagQuery(queryNode) ||
               options.resultType == options.RESULTS_AS_TAG_QUERY ||
-              options.resultType == options.RESULTS_AS_TAG_CONTENTS ||
               options.resultType == options.RESULTS_AS_ROOTS_QUERY)
             options.resultType = options.RESULTS_AS_URI;
 
           var query = PlacesUtils.history.getNewQuery();
           query.searchTerms = filterString;
 
           if (folderRestrict) {
             query.setFolders(folderRestrict, folderRestrict.length);
@@ -523,23 +523,18 @@
                 index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
               }
             }
           }
 
           if (this.controller.disallowInsertion(container))
             return null;
 
-          // TODO (Bug 1160193): properly support dropping on a tag root.
-          let tagName = null;
-          if (PlacesUtils.nodeIsTagQuery(container)) {
-            tagName = container.title;
-            if (!tagName)
-              return null;
-          }
+          let tagName = PlacesUtils.nodeIsTagQuery(container) ?
+                          PlacesUtils.asQuery(container).query.tags[0] : null;
 
           return new PlacesInsertionPoint({
             parentId: PlacesUtils.getConcreteItemId(container),
             parentGuid: PlacesUtils.getConcreteItemGuid(container),
             index, orientation, tagName, dropNearNode
           });
         ]]></body>
       </method>
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -1346,18 +1346,18 @@ PlacesTreeView.prototype = {
     // Flat-lists may ignore expandQueries and other query options when
     // they are asked to open a container.
     if (this._flatList)
       return true;
 
     // Treat non-expandable childless queries as non-containers, unless they
     // are tags.
     if (PlacesUtils.nodeIsQuery(node) && !PlacesUtils.nodeIsTagQuery(node)) {
-      PlacesUtils.asQuery(node);
-      return node.queryOptions.expandQueries || node.hasChildren;
+      return PlacesUtils.asQuery(node).queryOptions.expandQueries ||
+             node.hasChildren;
     }
     return true;
   },
 
   isContainerOpen: function PTV_isContainerOpen(aRow) {
     if (this._flatList)
       return false;
 
@@ -1462,23 +1462,18 @@ PlacesTreeView.prototype = {
           index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
         }
       }
     }
 
     if (this._controller.disallowInsertion(container))
       return null;
 
-    // TODO (Bug 1160193): properly support dropping on a tag root.
-    let tagName = null;
-    if (PlacesUtils.nodeIsTagQuery(container)) {
-      tagName = container.title;
-      if (!tagName)
-        return null;
-    }
+    let tagName = PlacesUtils.nodeIsTagQuery(container) ?
+                    PlacesUtils.asQuery(container).query.tags[0] : null;
 
     return new PlacesInsertionPoint({
       parentId: PlacesUtils.getConcreteItemId(container),
       parentGuid: PlacesUtils.getConcreteItemGuid(container),
       index, orientation, tagName, dropNearNode
     });
   },
 
--- a/browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_editTagContainer.js
@@ -26,59 +26,64 @@ add_task(async function() {
   let fooTag = tagsContainer.getChild(0);
   let tagNode = fooTag;
   tree.selectNode(fooTag);
   Assert.equal(tagNode.title, "tag1", "tagNode title is correct");
 
   Assert.ok(tree.controller.isCommandEnabled("placesCmd_show:info"),
             "'placesCmd_show:info' on current selected node is enabled");
 
-  let promiseTitleResetNotification = PlacesTestUtils.waitForNotification(
-      "onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "tag1");
+  let promiseTagResetNotification = PlacesTestUtils.waitForNotification(
+    "onItemChanged", (itemId, prop) => {
+      let tags = PlacesUtils.tagging.getTagsForURI(uri);
+      return prop == "tags" && tags.length == 1 && tags[0] == "tag1";
+  });
 
   await withBookmarksDialog(
     true,
     function openDialog() {
       tree.controller.doCommand("placesCmd_show:info");
     },
     async function test(dialogWin) {
       // Check that the dialog is not read-only.
       Assert.ok(!dialogWin.gEditItemOverlay.readOnly, "Dialog should not be read-only");
 
       // Check that name picker is not read only
       let namepicker = dialogWin.document.getElementById("editBMPanel_namePicker");
       Assert.ok(!namepicker.readOnly, "Name field should not be read-only");
       Assert.equal(namepicker.value, "tag1", "Node title is correct");
-
-      let promiseTitleChangeNotification = PlacesTestUtils.waitForNotification(
-          "onItemChanged", (itemId, prop, isAnno, val) => prop == "title" && val == "tag2");
+      let promiseTagChangeNotification = PlacesTestUtils.waitForNotification(
+        "onItemChanged", (itemId, prop) => {
+          let tags = PlacesUtils.tagging.getTagsForURI(uri);
+          return prop == "tags" && tags.length == 1 && tags[0] == "tag2";
+      });
 
       fillBookmarkTextField("editBMPanel_namePicker", "tag2", dialogWin);
 
-      await promiseTitleChangeNotification;
+      await promiseTagChangeNotification;
 
-      Assert.equal(namepicker.value, "tag2", "Node title has been properly edited");
+      Assert.equal(namepicker.value, "tag2", "The title field has been changed");
 
       // Check the shortcut's title.
       Assert.equal(tree.selectedNode.title, "tag2", "The node has the correct title");
 
       // Try to set an empty title, it should restore the previous one.
       fillBookmarkTextField("editBMPanel_namePicker", "", dialogWin);
-      Assert.equal(namepicker.value, "tag2", "Title has not been changed");
+      Assert.equal(namepicker.value, "tag2", "The title field has been changed");
       Assert.equal(tree.selectedNode.title, "tag2", "The node has the correct title");
 
       // Check the tags have been edited.
       let tags = PlacesUtils.tagging.getTagsForURI(uri);
       Assert.equal(tags.length, 1, "Found the right number of tags");
       Assert.ok(tags.includes("tag2"), "Found the expected tag");
 
       // Ensure that the addition really is finished before we hit cancel.
       await PlacesTestUtils.promiseAsyncUpdates();
     }
   );
 
-  await promiseTitleResetNotification;
+  await promiseTagResetNotification;
 
   // Check the tag change has been reverted.
   let tags = PlacesUtils.tagging.getTagsForURI(uri);
   Assert.equal(tags.length, 1, "Found the right number of tags");
   Assert.deepEqual(tags, ["tag1"], "Found the expected tag");
 });
--- a/browser/components/places/tests/browser/browser_drag_bookmarks_on_toolbar.js
+++ b/browser/components/places/tests/browser/browser_drag_bookmarks_on_toolbar.js
@@ -111,24 +111,22 @@ function synthesizeDragWithDirection(aEl
                              startingPoint.y + yIncrement * 9,
                              { type: "mousemove" });
 
   return promise;
 }
 
 function getToolbarNodeForItemId(itemGuid) {
   var children = document.getElementById("PlacesToolbarItems").childNodes;
-  var node = null;
-  for (var i = 0; i < children.length; i++) {
-    if (itemGuid == children[i]._placesNode.bookmarkGuid) {
-      node = children[i];
-      break;
+  for (let child of children) {
+    if (itemGuid == child._placesNode.bookmarkGuid) {
+      return child;
     }
   }
-  return node;
+  return null;
 }
 
 function getExpectedDataForPlacesNode(aNode) {
   var wrappedNode = [];
   var flavors = ["text/x-moz-place",
                  "text/x-moz-url",
                  "text/plain",
                  "text/html"];
--- a/browser/components/places/tests/browser/browser_library_delete_bookmarks_in_tags.js
+++ b/browser/components/places/tests/browser/browser_library_delete_bookmarks_in_tags.js
@@ -31,18 +31,18 @@ add_task(async function test_tags() {
   // display in "logical" order of bm0 at the top, and bm2 at the bottom.
   children = children.reverse();
 
   await PlacesUtils.bookmarks.insertTree({
     guid: PlacesUtils.bookmarks.unfiledGuid,
     children,
   });
 
-  for (let i = 0; i < uris.length; i++) {
-    PlacesUtils.tagging.tagURI(uris[i], ["test"]);
+  for (let uri of uris) {
+    PlacesUtils.tagging.tagURI(uri, ["test"]);
   }
 
   let library = await promiseLibrary();
 
   // Select and open the left pane "Bookmarks Toolbar" folder.
   let PO = library.PlacesOrganizer;
 
   PO.selectLeftPaneBuiltIn("Tags");
--- a/toolkit/components/places/PlacesTransactions.jsm
+++ b/toolkit/components/places/PlacesTransactions.jsm
@@ -903,17 +903,17 @@ DefineTransaction.verifyInput = function
 // Update the documentation at the top of this module if you add or
 // remove properties.
 DefineTransaction.defineInputProps(["url", "feedUrl", "siteUrl"],
                                    DefineTransaction.urlValidate, null);
 DefineTransaction.defineInputProps(["guid", "parentGuid", "newParentGuid"],
                                    DefineTransaction.guidValidate);
 DefineTransaction.defineInputProps(["title", "postData"],
                                    DefineTransaction.strOrNullValidate, null);
-DefineTransaction.defineInputProps(["keyword", "oldKeyword", "tag",
+DefineTransaction.defineInputProps(["keyword", "oldKeyword", "oldTag", "tag",
                                     "excludingAnnotation"],
                                    DefineTransaction.strValidate, "");
 DefineTransaction.defineInputProps(["index", "newIndex"],
                                    DefineTransaction.indexValidate,
                                    PlacesUtils.bookmarks.DEFAULT_INDEX);
 DefineTransaction.defineInputProps(["annotation"],
                                    DefineTransaction.annotationObjectValidate);
 DefineTransaction.defineInputProps(["child"],
@@ -1619,16 +1619,106 @@ PT.Untag.prototype = {
       for (let f of onRedo) {
         await f();
       }
     };
   }
 };
 
 /**
+ * Transaction for renaming a tag.
+ *
+ * Required Input Properties: oldTag, tag.
+ */
+PT.RenameTag = DefineTransaction(["oldTag", "tag"]);
+PT.RenameTag.prototype = {
+  async execute({ oldTag, tag }) {
+    // For now this is implemented by untagging and tagging all the bookmarks.
+    // We should create a specialized bookmarking API to just rename the tag.
+    let onUndo = [], onRedo = [];
+    let urls = PlacesUtils.tagging.getURIsForTag(oldTag);
+    if (urls.length > 0) {
+      let tagTxn = TransactionsHistory.getRawTransaction(
+        PT.Tag({ urls, tags: [tag] })
+      );
+      await tagTxn.execute();
+      onUndo.unshift(tagTxn.undo.bind(tagTxn));
+      onRedo.push(tagTxn.redo.bind(tagTxn));
+      let untagTxn = TransactionsHistory.getRawTransaction(
+        PT.Untag({ urls, tags: [oldTag] })
+      );
+      await untagTxn.execute();
+      onUndo.unshift(untagTxn.undo.bind(untagTxn));
+      onRedo.push(untagTxn.redo.bind(untagTxn));
+
+      // Update all the place: queries that refer to this tag.
+      let db = await PlacesUtils.promiseDBConnection();
+      let rows = await db.executeCached(`
+        SELECT h.url, b.guid, b.title
+        FROM moz_places h
+        JOIN moz_bookmarks b ON b.fk = h.id
+        WHERE url_hash BETWEEN hash("place", "prefix_lo")
+                           AND hash("place", "prefix_hi")
+          AND url LIKE :tagQuery
+      `, { tagQuery: "%tag=%" });
+      for (let row of rows) {
+        let url = row.getResultByName("url");
+        try {
+          url = new URL(url);
+          let urlParams = new URLSearchParams(url.pathname);
+          let tags = urlParams.getAll("tag");
+          if (!tags.includes(oldTag))
+            continue;
+          if (tags.length > 1) {
+            // URLSearchParams cannot set more than 1 same-named param.
+            urlParams.delete("tag");
+            urlParams.set("tag", tag);
+            url = new URL(url.protocol + urlParams +
+                          "&tag=" + tags.filter(t => t != oldTag).join("&tag="));
+          } else {
+            urlParams.set("tag", tag);
+            url = new URL(url.protocol + urlParams);
+          }
+        } catch (ex) {
+          Cu.reportError("Invalid bookmark url: " + row.getResultByName("url") + ": " + ex);
+          continue;
+        }
+        let guid = row.getResultByName("guid");
+        let title = row.getResultByName("title");
+
+        let editUrlTxn = TransactionsHistory.getRawTransaction(
+          PT.EditUrl({ guid, url })
+        );
+        await editUrlTxn.execute();
+        onUndo.unshift(editUrlTxn.undo.bind(editUrlTxn));
+        onRedo.push(editUrlTxn.redo.bind(editUrlTxn));
+        if (title == oldTag) {
+          let editTitleTxn = TransactionsHistory.getRawTransaction(
+            PT.EditTitle({ guid, title: tag })
+          );
+          await editTitleTxn.execute();
+          onUndo.unshift(editTitleTxn.undo.bind(editTitleTxn));
+          onRedo.push(editTitleTxn.redo.bind(editTitleTxn));
+        }
+      }
+    }
+    this.undo = async function() {
+      for (let f of onUndo) {
+        await f();
+      }
+    };
+    this.redo = async function() {
+      for (let f of onRedo) {
+        await f();
+      }
+    };
+  }
+};
+
+/**
  * Transaction for copying an item.
  *
  * Required Input Properties: guid, newParentGuid
  * Optional Input Properties: newIndex, excludingAnnotations.
  */
 PT.Copy = DefineTransaction(["guid", "newParentGuid"],
                             ["newIndex", "excludingAnnotations"]);
 PT.Copy.prototype = {
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -777,19 +777,30 @@ var PlacesUtils = {
 
   /**
    * Determines whether or not a result-node is a tag container.
    * @param   aNode
    *          A result-node
    * @returns true if the node is a tag container, false otherwise
    */
   nodeIsTagQuery: function PU_nodeIsTagQuery(aNode) {
-    return aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY &&
-           asQuery(aNode).queryOptions.resultType ==
-             Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_CONTENTS;
+    if (aNode.type != Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY)
+      return false;
+    // Direct child of RESULTS_AS_TAG_QUERY.
+    let parent = aNode.parent;
+    if (parent && PlacesUtils.asQuery(parent).queryOptions.resultType ==
+                    Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY)
+      return true;
+    // We must also support the right pane of the Library, when the tag query
+    // is the root node. Unfortunately this is also valid for any tag query
+    // selected in the left pane that is not a direct child of RESULTS_AS_TAG_QUERY.
+    if (!parent && aNode == aNode.parentResult.root &&
+        PlacesUtils.asQuery(aNode).query.tags.length == 1)
+      return true;
+    return false;
   },
 
   /**
    * Determines whether or not a ResultNode is a container.
    * @param   aNode
    *          A result node
    * @returns true if the node is a container item, false otherwise
    */
@@ -817,25 +828,18 @@ var PlacesUtils = {
             this.nodeIsHost(aNode));
   },
 
   /**
    * Gets the concrete item-id for the given node. Generally, this is just
    * node.itemId, but for folder-shortcuts that's node.folderItemId.
    */
   getConcreteItemId: function PU_getConcreteItemId(aNode) {
-    if (aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT)
-      return asQuery(aNode).folderItemId;
-    else if (PlacesUtils.nodeIsTagQuery(aNode)) {
-      // RESULTS_AS_TAG_CONTENTS queries are similar to folder shortcuts
-      // so we can still get the concrete itemId for them.
-      let folders = aNode.query.getFolders();
-      return folders[0];
-    }
-    return aNode.itemId;
+    return aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT ?
+             asQuery(aNode).folderItemId : aNode.itemId;
   },
 
   /**
    * Gets the concrete item-guid for the given node. For everything but folder
    * shortcuts, this is just node.bookmarkGuid.  For folder shortcuts, this is
    * node.targetFolderGuid (see nsINavHistoryService.idl for the semantics).
    *
    * @param aNode
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -1125,17 +1125,18 @@ bool NeedToFilterResultSet(const RefPtr<
 }
 
 // ** Helper class for ConstructQueryString **/
 
 class PlacesSQLQueryBuilder
 {
 public:
   PlacesSQLQueryBuilder(const nsCString& aConditions,
-                        nsNavHistoryQueryOptions* aOptions,
+                        const RefPtr<nsNavHistoryQuery>& aQuery,
+                        const RefPtr<nsNavHistoryQueryOptions>& aOptions,
                         bool aUseLimit,
                         nsNavHistory::StringHash& aAddParams,
                         bool aHasSearchTerms);
 
   nsresult GetQueryString(nsCString& aQueryString);
 
 private:
   nsresult Select();
@@ -1168,37 +1169,44 @@ private:
   bool mIncludeHidden;
   uint16_t mSortingMode;
   uint32_t mMaxResults;
 
   nsCString mQueryString;
   nsCString mGroupBy;
   bool mHasDateColumns;
   bool mSkipOrderBy;
+
   nsNavHistory::StringHash& mAddParams;
 };
 
 PlacesSQLQueryBuilder::PlacesSQLQueryBuilder(
     const nsCString& aConditions,
-    nsNavHistoryQueryOptions* aOptions,
+    const RefPtr<nsNavHistoryQuery>& aQuery,
+    const RefPtr<nsNavHistoryQueryOptions>& aOptions,
     bool aUseLimit,
     nsNavHistory::StringHash& aAddParams,
     bool aHasSearchTerms)
 : mConditions(aConditions)
 , mUseLimit(aUseLimit)
 , mHasSearchTerms(aHasSearchTerms)
 , mResultType(aOptions->ResultType())
 , mQueryType(aOptions->QueryType())
 , mIncludeHidden(aOptions->IncludeHidden())
 , mSortingMode(aOptions->SortingMode())
 , mMaxResults(aOptions->MaxResults())
 , mSkipOrderBy(false)
 , mAddParams(aAddParams)
 {
   mHasDateColumns = (mQueryType == nsINavHistoryQueryOptions::QUERY_TYPE_BOOKMARKS);
+  // Force the default sorting mode for tag queries.
+  if (mSortingMode == nsINavHistoryQueryOptions::SORT_BY_NONE &&
+      aQuery->Tags().Length() > 0) {
+    mSortingMode = nsINavHistoryQueryOptions::SORT_BY_TITLE_ASCENDING;
+  }
 }
 
 nsresult
 PlacesSQLQueryBuilder::GetQueryString(nsCString& aQueryString)
 {
   nsresult rv = Select();
   NS_ENSURE_SUCCESS(rv, rv);
   rv = Where();
@@ -1649,16 +1657,19 @@ PlacesSQLQueryBuilder::SelectAsTag()
 {
   nsNavHistory *history = nsNavHistory::GetHistoryService();
   NS_ENSURE_STATE(history);
 
   // This allows sorting by date fields what is not possible with
   // other history queries.
   mHasDateColumns = true;
 
+  // TODO (Bug 1449939): This is likely wrong, since the tag name should
+  // probably be urlencoded, and we have no util for that in SQL, yet.
+  // We could encode the tag when the user sets it though.
   mQueryString = nsPrintfCString(
     "SELECT null, 'place:tag=' || title, "
            "title, null, null, null, null, null, dateAdded, "
            "lastModified, null, null, null, null, null, null "
     "FROM moz_bookmarks "
     "WHERE parent = %" PRId64,
     history->GetTagsFolder()
   );
@@ -2014,17 +2025,17 @@ nsNavHistory::ConstructQueryString(
     conditions += queryClause;
   }
 
   // Determine whether we can push maxResults constraints into the query
   // as LIMIT, or if we need to do result count clamping later
   // using FilterResultSet()
   bool useLimitClause = !NeedToFilterResultSet(aQuery, aOptions);
 
-  PlacesSQLQueryBuilder queryStringBuilder(conditions, aOptions,
+  PlacesSQLQueryBuilder queryStringBuilder(conditions, aQuery, aOptions,
                                            useLimitClause, aAddParams,
                                            hasSearchTerms);
   rv = queryStringBuilder.GetQueryString(queryString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
@@ -3035,18 +3046,19 @@ nsNavHistory::QueryToSelectClause(const 
            Str("AND lower(tags.title) IN (");
     for (uint32_t i = 0; i < tags.Length(); ++i) {
       nsPrintfCString param(":tag%d_", i);
       clause.Param(param.get());
       if (i < tags.Length() - 1)
         clause.Str(",");
     }
     clause.Str(")");
-    if (!aQuery->TagsAreNot())
+    if (!aQuery->TagsAreNot()) {
       clause.Str("GROUP BY bms.fk HAVING count(*) >=").Param(":tag_count");
+    }
     clause.Str(")");
   }
 
   // transitions
   const nsTArray<uint32_t>& transitions = aQuery->Transitions();
   for (uint32_t i = 0; i < transitions.Length(); ++i) {
     nsPrintfCString param(":transition%d_", i);
     clause.Condition("h.id IN (SELECT place_id FROM moz_historyvisits "
deleted file mode 100644
--- a/toolkit/components/places/tests/unit/test_419731.js
+++ /dev/null
@@ -1,123 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim:set ts=2 sw=2 sts=2 et: */
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-add_task(async function test_tag_updates() {
-  const url = "http://foo.bar/";
-  let lastModified = new Date(Date.now() - 10000);
-
-  // create 2 bookmarks
-  let bm1 = await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.menuGuid,
-    dateAdded: lastModified,
-    lastModified: new Date(),
-    title: "title 1",
-    url,
-  });
-  let bm2 = await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.menuGuid,
-    dateAdded: lastModified,
-    lastModified,
-    title: "title 2",
-    url,
-  });
-
-  // add a new tag
-  PlacesUtils.tagging.tagURI(uri(url), ["foo"]);
-
-  // get tag folder id
-  let options = PlacesUtils.history.getNewQueryOptions();
-  let query = PlacesUtils.history.getNewQuery();
-  query.setFolders([PlacesUtils.tagsFolderId], 1);
-  let result = PlacesUtils.history.executeQuery(query, options);
-  let tagRoot = result.root;
-  tagRoot.containerOpen = true;
-  let tagNode = tagRoot.getChild(0)
-                       .QueryInterface(Ci.nsINavHistoryContainerResultNode);
-  let tagItemGuid = tagNode.bookmarkGuid;
-  let tagItemId = tagNode.itemId;
-  tagRoot.containerOpen = false;
-
-  // change bookmark 1 title
-  await PlacesUtils.bookmarks.update({
-    guid: bm1.guid,
-    title: "new title 1",
-  });
-
-  // Workaround timers resolution and time skews.
-  bm2 = await PlacesUtils.bookmarks.fetch(bm2.guid);
-
-  lastModified = new Date(lastModified.getTime() + 1000);
-
-  await PlacesUtils.bookmarks.update({
-    guid: bm1.guid,
-    lastModified,
-  });
-
-  // Query the tag.
-  options = PlacesUtils.history.getNewQueryOptions();
-  options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
-  options.resultType = options.RESULTS_AS_TAG_QUERY;
-
-  query = PlacesUtils.history.getNewQuery();
-  result = PlacesUtils.history.executeQuery(query, options);
-  let root = result.root;
-  root.containerOpen = true;
-  Assert.equal(root.childCount, 1);
-
-  let theTag = root.getChild(0)
-                   .QueryInterface(Ci.nsINavHistoryContainerResultNode);
-  // Bug 524219: Check that renaming the tag shows up in the result.
-  Assert.equal(theTag.title, "foo");
-
-  await PlacesUtils.bookmarks.update({
-    guid: tagItemGuid,
-    title: "bar",
-  });
-
-  // Check that the item has been replaced
-  Assert.notEqual(theTag, root.getChild(0));
-  theTag = root.getChild(0)
-                   .QueryInterface(Ci.nsINavHistoryContainerResultNode);
-  Assert.equal(theTag.title, "bar");
-
-  // Check that tag container contains new title
-  theTag.containerOpen = true;
-  Assert.equal(theTag.childCount, 1);
-  let node = theTag.getChild(0);
-  Assert.equal(node.title, "new title 1");
-  theTag.containerOpen = false;
-  root.containerOpen = false;
-
-  // Change bookmark 2 title.
-  PlacesUtils.bookmarks.update({
-    guid: bm2.guid,
-    title: "new title 2"
-  });
-
-  // Workaround timers resolution and time skews.
-  lastModified = new Date(lastModified.getTime() + 1000);
-
-  await PlacesUtils.bookmarks.update({
-    guid: bm2.guid,
-    lastModified,
-  });
-
-  // Check that tag container contains new title
-  options = PlacesUtils.history.getNewQueryOptions();
-  options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
-  options.resultType = options.RESULTS_AS_TAG_CONTENTS;
-
-  query = PlacesUtils.history.getNewQuery();
-  query.setFolders([tagItemId], 1);
-  result = PlacesUtils.history.executeQuery(query, options);
-  root = result.root;
-
-  root.containerOpen = true;
-  Assert.equal(root.childCount, 1);
-  node = root.getChild(0);
-  Assert.equal(node.title, "new title 2");
-  root.containerOpen = false;
-});
--- a/toolkit/components/places/tests/unit/test_async_transactions.js
+++ b/toolkit/components/places/tests/unit/test_async_transactions.js
@@ -1854,8 +1854,72 @@ add_task(async function test_remove_mult
   // Redo remove.
   await PT.redo();
   await ensureNonExistent(...guids);
 
   // Cleanup
   await PT.clearTransactionsHistory();
   observer.reset();
 });
+
+add_task(async function test_renameTag() {
+  let url = "http://test.edit.keyword/";
+  await PT.Tag({ url, tags: ["t1", "t2"] }).transact();
+  ensureTagsForURI(url, ["t1", "t2"]);
+
+  // Create bookmark queries that point to the modified tag.
+  let bm1 = await PlacesUtils.bookmarks.insert({
+    url: "place:tag=t2",
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid
+  });
+  let bm2 = await PlacesUtils.bookmarks.insert({
+    url: "place:tag=t2&sort=1",
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid
+  });
+  // This points to 2 tags, and as such won't be touched.
+  let bm3 = await PlacesUtils.bookmarks.insert({
+    url: "place:tag=t2&tag=t1",
+    parentGuid: PlacesUtils.bookmarks.unfiledGuid
+  });
+
+  await PT.RenameTag({ oldTag: "t2", tag: "t3" }).transact();
+  ensureTagsForURI(url, ["t1", "t3"]);
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm1.guid)).url.href, "place:tag=t3",
+               "The fitst bookmark has been updated");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm2.guid)).url.href, "place:tag=t3&sort=1",
+               "The second bookmark has been updated");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm3.guid)).url.href, "place:tag=t3&tag=t1",
+               "The third bookmark has been updated");
+
+  await PT.undo();
+  ensureTagsForURI(url, ["t1", "t2"]);
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm1.guid)).url.href, "place:tag=t2",
+               "The fitst bookmark has been restored");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm2.guid)).url.href, "place:tag=t2&sort=1",
+               "The second bookmark has been restored");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm3.guid)).url.href, "place:tag=t2&tag=t1",
+               "The third bookmark has been restored");
+
+  await PT.redo();
+  ensureTagsForURI(url, ["t1", "t3"]);
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm1.guid)).url.href, "place:tag=t3",
+               "The fitst bookmark has been updated");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm2.guid)).url.href, "place:tag=t3&sort=1",
+               "The second bookmark has been updated");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm3.guid)).url.href, "place:tag=t3&tag=t1",
+               "The third bookmark has been updated");
+
+  await PT.undo();
+  ensureTagsForURI(url, ["t1", "t2"]);
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm1.guid)).url.href, "place:tag=t2",
+               "The fitst bookmark has been restored");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm2.guid)).url.href, "place:tag=t2&sort=1",
+               "The second bookmark has been restored");
+  Assert.equal((await PlacesUtils.bookmarks.fetch(bm3.guid)).url.href, "place:tag=t2&tag=t1",
+               "The third bookmark has been restored");
+
+  await PT.undo();
+  ensureTagsForURI(url, []);
+
+  await PT.clearTransactionsHistory();
+  ensureUndoState();
+  await PlacesUtils.bookmarks.eraseEverything();
+});
--- a/toolkit/components/places/tests/unit/test_resolveNullBookmarkTitles.js
+++ b/toolkit/components/places/tests/unit/test_resolveNullBookmarkTitles.js
@@ -1,15 +1,15 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim:set ts=2 sw=2 sts=2 et: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-add_task(async function test_resolveNullBookmarkTitles() {
+add_task(async function() {
   let uri1 = uri("http://foo.tld/");
   let uri2 = uri("https://bar.tld/");
 
   await PlacesTestUtils.addVisits([
     { uri: uri1, title: "foo title" },
     { uri: uri2, title: "bar title" }
   ]);
   await PlacesUtils.bookmarks.insert({
@@ -20,25 +20,38 @@ add_task(async function test_resolveNull
   await PlacesUtils.bookmarks.insert({
     parentGuid: PlacesUtils.bookmarks.menuGuid,
     url: uri2,
     title: null
   });
 
   PlacesUtils.tagging.tagURI(uri1, ["tag 1"]);
   PlacesUtils.tagging.tagURI(uri2, ["tag 2"]);
+});
 
+add_task(async function testAsTagQuery() {
   let options = PlacesUtils.history.getNewQueryOptions();
   options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
   options.resultType = options.RESULTS_AS_TAG_CONTENTS;
 
   let query = PlacesUtils.history.getNewQuery();
   // if we don't set a tag folder, RESULTS_AS_TAG_CONTENTS will return all
   // tagged URIs
   let root = PlacesUtils.history.executeQuery(query, options).root;
   root.containerOpen = true;
   Assert.equal(root.childCount, 2);
   // actually RESULTS_AS_TAG_CONTENTS return results ordered by place_id DESC
   // so they are reversed
   Assert.equal(root.getChild(0).title, "");
   Assert.equal(root.getChild(1).title, "");
   root.containerOpen = false;
 });
+
+add_task(async function testTagQuery() {
+  let options = PlacesUtils.history.getNewQueryOptions();
+  let query = PlacesUtils.history.getNewQuery();
+  query.tags = ["tag 1"];
+  let root = PlacesUtils.history.executeQuery(query, options).root;
+  root.containerOpen = true;
+  Assert.equal(root.childCount, 1);
+  Assert.equal(root.getChild(0).title, "");
+  root.containerOpen = false;
+});
--- a/toolkit/components/places/tests/unit/xpcshell.ini
+++ b/toolkit/components/places/tests/unit/xpcshell.ini
@@ -28,17 +28,16 @@ skip-if = (os == 'win' && ccov) # Bug 14
 # Bug 821781: test fails intermittently on Linux
 skip-if = os == "linux"
 [test_402799.js]
 [test_408221.js]
 [test_412132.js]
 [test_413784.js]
 [test_415460.js]
 [test_415757.js]
-[test_419731.js]
 [test_419792_node_tags_property.js]
 [test_425563.js]
 [test_429505_remove_shortcuts.js]
 [test_433317_query_title_update.js]
 [test_433525_hasChildren_crash.js]
 [test_454977.js]
 [test_463863.js]
 [test_485442_crash_bug_nsNavHistoryQuery_GetUri.js]