Bug 1452621 - Cleanup some tag queries related code. r=standard8
MozReview-Commit-ID: L8L3i5W1CHe
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@@ -171,17 +171,17 @@ PlacesController.prototype = {
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.nodeIsTagQuery(selectedNode) ||
- PlacesUtils.getConcreteItemId(selectedNode) != -1);
+ PlacesUtils.nodeIsBookmark(selectedNode));
}
case "placesCmd_reload": {
// Livemark containers
let selectedNode = this._view.selectedNode;
return selectedNode && this.hasCachedLivemarkInfo(selectedNode);
}
case "placesCmd_sortBy:name": {
let selectedNode = this._view.selectedNode;
--- a/browser/components/places/tests/browser/browser_bookmarkProperties_readOnlyRoot.js
+++ b/browser/components/places/tests/browser/browser_bookmarkProperties_readOnlyRoot.js
@@ -1,21 +1,21 @@
"use strict";
-add_task(async function() {
- info("Bug 479348 - Properties on a root should be read-only.");
-
+add_task(async function test_dialog() {
+ info("Bug 479348 - Properties dialog on a root should be read-only.");
await withSidebarTree("bookmarks", async function(tree) {
tree.selectItems([PlacesUtils.bookmarks.unfiledGuid]);
- Assert.ok(tree.controller.isCommandEnabled("placesCmd_show:info"),
- "'placesCmd_show:info' on current selected node is enabled");
+ Assert.ok(!tree.controller.isCommandEnabled("placesCmd_show:info"),
+ "'placesCmd_show:info' on current selected node is disabled");
await withBookmarksDialog(
true,
function openDialog() {
+ // Even if the cmd is disabled, we can execute it regardless.
tree.controller.doCommand("placesCmd_show:info");
},
async function test(dialogWin) {
// Check that the dialog is read-only.
Assert.ok(dialogWin.gEditItemOverlay.readOnly, "Dialog is read-only");
// Check that accept button is disabled
let acceptButton = dialogWin.document.documentElement.getButton("accept");
Assert.ok(acceptButton.disabled, "Accept button is disabled");
@@ -24,8 +24,30 @@ add_task(async function() {
let namepicker = dialogWin.document.getElementById("editBMPanel_namePicker");
Assert.ok(namepicker.readOnly, "Name field is read-only");
Assert.equal(namepicker.value,
PlacesUtils.getString("OtherBookmarksFolderTitle"), "Node title is correct");
}
);
});
});
+
+add_task(async function test_library() {
+ info("Bug 479348 - Library info pane on a root should be read-only.");
+ let library = await promiseLibrary("UnfiledBookmarks");
+ registerCleanupFunction(async function() {
+ await promiseLibraryClosed(library);
+ });
+ let PlacesOrganizer = library.PlacesOrganizer;
+ let tree = PlacesOrganizer._places;
+ tree.focus();
+ Assert.ok(!tree.controller.isCommandEnabled("placesCmd_show:info"),
+ "'placesCmd_show:info' on current selected node is disabled");
+
+ // Check that the pane is read-only.
+ Assert.ok(library.gEditItemOverlay.readOnly, "Info pane is read-only");
+
+ // Check that name picker is read only
+ let namepicker = library.document.getElementById("editBMPanel_namePicker");
+ Assert.ok(namepicker.readOnly, "Name field is read-only");
+ Assert.equal(namepicker.value,
+ PlacesUtils.getString("OtherBookmarksFolderTitle"), "Node title is correct");
+});
--- a/browser/components/places/tests/browser/head.js
+++ b/browser/components/places/tests/browser/head.js
@@ -204,17 +204,17 @@ var withBookmarksDialog = async function
skipOverlayWait = false) {
let closed = false;
let dialogPromise = new Promise(resolve => {
Services.ww.registerNotification(function winObserver(subject, topic, data) {
if (topic == "domwindowopened") {
let win = subject.QueryInterface(Ci.nsIDOMWindow);
win.addEventListener("load", function() {
ok(win.location.href.startsWith(dialogUrl),
- "The bookmark properties dialog is open");
+ "The bookmark properties dialog is open: " + win.location.href);
// This is needed for the overlay.
waitForFocus(() => {
resolve(win);
}, win);
}, {once: true});
} else if (topic == "domwindowclosed") {
Services.ww.unregisterNotification(winObserver);
closed = true;
@@ -250,29 +250,24 @@ var withBookmarksDialog = async function
let closePromise = () => Promise.resolve();
if (closeFn) {
closePromise = closeFn(dialogWin);
}
try {
await taskFn(dialogWin);
} finally {
- if (!closed && !autoCancel) {
- // Give the dialog a little time to close itself in the manually closing
- // case.
- await BrowserTestUtils.waitForCondition(() => closed,
- "The test should have closed the dialog!");
- }
- if (!closed) {
+ if (!closed && autoCancel) {
info("withBookmarksDialog: canceling the dialog");
-
doc.documentElement.cancelDialog();
-
await closePromise;
}
+ // Give the dialog a little time to close itself.
+ await BrowserTestUtils.waitForCondition(() => closed,
+ "The dialog should be closed!");
}
};
/**
* Opens the contextual menu on the element pointed by the given selector.
*
* @param selector
* Valid selector syntax
--- a/toolkit/components/places/PlacesUtils.jsm
+++ b/toolkit/components/places/PlacesUtils.jsm
@@ -141,82 +141,54 @@ function serializeNode(aNode, aIsLivemar
data.id = aNode.itemId;
data.itemGuid = aNode.bookmarkGuid;
data.livemark = aIsLivemark;
// Add an instanceId so we can tell which instance of an FF session the data
// is coming from.
data.instanceId = PlacesUtils.instanceId;
let guid = aNode.bookmarkGuid;
- let grandParentId;
// Some nodes, e.g. the unfiled/menu/toolbar ones can have a virtual guid, so
// we ignore any that are a folder shortcut. These will be handled below.
if (guid && !PlacesUtils.bookmarks.isVirtualRootItem(guid) &&
!PlacesUtils.isVirtualLeftPaneItem(guid)) {
if (aNode.parent) {
data.parent = aNode.parent.itemId;
data.parentGuid = aNode.parent.bookmarkGuid;
}
- let grandParent = aNode.parent && aNode.parent.parent;
- if (grandParent) {
- grandParentId = grandParent.itemId;
- }
-
data.dateAdded = aNode.dateAdded;
data.lastModified = aNode.lastModified;
let annos = getAnnotationsForItem(data.id);
if (annos.length > 0)
data.annos = annos;
}
if (PlacesUtils.nodeIsURI(aNode)) {
// Check for url validity.
- NetUtil.newURI(aNode.uri);
-
- // Tag root accepts only folder nodes, not URIs.
- if (data.parent == PlacesUtils.tagsFolderId)
- throw new Error("Unexpected node type");
-
+ new URL(aNode.uri);
data.type = PlacesUtils.TYPE_X_MOZ_PLACE;
data.uri = aNode.uri;
-
if (aNode.tags)
data.tags = aNode.tags;
- } else if (PlacesUtils.nodeIsContainer(aNode)) {
- // Tag containers accept only uri nodes.
- if (grandParentId == PlacesUtils.tagsFolderId)
- throw new Error("Unexpected node type");
-
- let concreteId = PlacesUtils.getConcreteItemId(aNode);
- if (concreteId != -1) {
- // This is a bookmark or a tag container.
- if (PlacesUtils.nodeIsQuery(aNode) || concreteId != aNode.itemId) {
- // This is a folder shortcut.
- data.type = PlacesUtils.TYPE_X_MOZ_PLACE;
- data.uri = aNode.uri;
- data.concreteId = concreteId;
- data.concreteGuid = PlacesUtils.getConcreteItemGuid(aNode);
- } else {
- // This is a bookmark folder.
- data.type = PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER;
- }
- } else {
- // This is a grouped container query, dynamically generated.
+ } else if (PlacesUtils.nodeIsFolder(aNode)) {
+ if (aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT) {
data.type = PlacesUtils.TYPE_X_MOZ_PLACE;
data.uri = aNode.uri;
+ data.concreteId = PlacesUtils.getConcreteItemId(aNode);
+ data.concreteGuid = PlacesUtils.getConcreteItemGuid(aNode);
+ } else {
+ data.type = PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER;
}
+ } else if (PlacesUtils.nodeIsQuery(aNode)) {
+ data.type = PlacesUtils.TYPE_X_MOZ_PLACE;
+ data.uri = aNode.uri;
} else if (PlacesUtils.nodeIsSeparator(aNode)) {
- // Tag containers don't accept separators.
- if (data.parent == PlacesUtils.tagsFolderId ||
- grandParentId == PlacesUtils.tagsFolderId)
- throw new Error("Unexpected node type");
-
data.type = PlacesUtils.TYPE_X_MOZ_PLACE_SEPARATOR;
}
return JSON.stringify(data);
}
// Imposed to limit database size.
const DB_URL_LENGTH_MAX = 65536;
@@ -839,18 +811,16 @@ var PlacesUtils = {
/**
* 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 result node.
* @return the concrete item-guid for aNode.
- * @note unlike getConcreteItemId, this doesn't allow retrieving the guid of a
- * ta container.
*/
getConcreteItemGuid(aNode) {
if (aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER_SHORTCUT)
return asQuery(aNode).targetFolderGuid;
return aNode.bookmarkGuid;
},
/**