--- 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
@@ -748,19 +748,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
*/
@@ -788,25 +799,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]
