Bug 1163447 - Allow Places tree selectItems() to recurse into folder-shortcuts. r=mak

This commit is contained in:
mchenryc 2015-06-04 13:02:00 -07:00
parent 4432747896
commit 2d987d18aa
4 changed files with 103 additions and 13 deletions

View File

@ -576,11 +576,11 @@
// Array of nodes found by findNodes which should be opened
var nodesToOpen = [];
// A set of URIs of container-nodes that were previously searched,
// A set of GUIDs of container-nodes that were previously searched,
// and thus shouldn't be searched again. This is empty at the initial
// start of the recursion and gets filled in as the recursion
// progresses.
var nodesURIChecked = [];
var checkedGuidsSet = new Set();
/**
* Recursively search through a node's children for items
@ -606,20 +606,18 @@
ids.splice(index, 1);
}
var concreteGuid = PlacesUtils.getConcreteItemGuid(node);
if (ids.length == 0 || !PlacesUtils.nodeIsContainer(node) ||
nodesURIChecked.indexOf(node.uri) != -1)
checkedGuidsSet.has(concreteGuid))
return foundOne;
// Don't try to open a query or a shurtcut, since it may return
// any duplicate data and be infinitely nested. Though, if it has
// been explicitly opened by the caller, search into it.
let shouldOpen = aOpenContainers &&
node.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_FOLDER;
// Only follow a query if it has been been explicitly opened by the caller.
let shouldOpen = aOpenContainers && PlacesUtils.nodeIsFolder(node);
PlacesUtils.asContainer(node);
if (!node.containerOpen && !shouldOpen)
return foundOne;
nodesURIChecked.push(node.uri);
checkedGuidsSet.add(concreteGuid);
// Remember the beginning state so that we can re-close
// this node if we don't find any additional results here.

View File

@ -4,6 +4,7 @@ support-files = head.js
[test_0_bug510634.xul]
[test_0_multiple_left_pane.xul]
[test_bug1163447_selectItems_through_shortcut.xul]
[test_bug427633_no_newfolder_if_noip.xul]
[test_bug485100-change-case-loses-tag.xul]
[test_bug549192.xul]

View File

@ -0,0 +1,89 @@
<?xml version="1.0"?>
<!--
Any copyright is dedicated to the Public Domain.
http://creativecommons.org/licenses/publicdomain/
-->
<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
type="text/css"?>
<?xml-stylesheet href="chrome://browser/content/places/places.css"?>
<?xml-stylesheet href="chrome://browser/skin/places/places.css"?>
<?xul-overlay href="chrome://browser/content/places/placesOverlay.xul"?>
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
title="1163447: selectItems in Places no longer selects items within Toolbar or Sidebar folders"
onload="runTest();">
<script type="application/javascript"
src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js" />
<script type="application/javascript" src="head.js" />
<body xmlns="http://www.w3.org/1999/xhtml" />
<tree id="tree"
type="places"
flex="1">
<treecols>
<treecol label="Title" id="title" anonid="title" primary="true" ordinal="1" flex="1"/>
</treecols>
<treechildren flex="1"/>
</tree>
<script type="application/javascript"><![CDATA[
/**
* Bug 1163447: places-tree should be able to select an item within the toolbar, and
* unfiled bookmarks. Yet not follow recursive folder-shortcuts infinitely.
*/
function runTest() {
SimpleTest.waitForExplicitFinish();
Task.spawn(function* () {
let bmu = PlacesUtils.bookmarks;
yield bmu.insert({
parentGuid: bmu.toolbarGuid,
index: bmu.DEFAULT_INDEX,
type: bmu.TYPE_BOOKMARK,
url: "place:folder=TOOLBAR",
title: "shortcut to self - causing infinite recursion if not handled properly"
});
yield bmu.insert({
parentGuid: bmu.toolbarGuid,
index: bmu.DEFAULT_INDEX,
type: bmu.TYPE_BOOKMARK,
url: "place:folder=UNFILED_BOOKMARKS",
title: "shortcut to unfiled, within toolbar"
});
let folder = yield bmu.insert({
parentGuid: bmu.unfiledGuid,
index: bmu.DEFAULT_INDEX,
type: bmu.TYPE_FOLDER,
title: "folder within unfiled"
});
// Setup the places tree contents.
let tree = document.getElementById("tree");
tree.place = "place:folder=TOOLBAR";
// Select the folder via the selectItems(itemId) API being tested
let itemId = yield PlacesUtils.promiseItemId(folder.guid);
tree.selectItems([itemId]);
is(tree.selectedNode && tree.selectedNode.itemId, itemId, "The node was selected through the shortcut");
// Cleanup
yield bmu.eraseEverything();
}).catch(err => {
ok(false, `Uncaught error: ${err}`);
}).then(SimpleTest.finish);
}
]]></script>
</window>

View File

@ -3610,12 +3610,14 @@ nsNavHistoryFolderResultNode::OnItemRemoved(int64_t aItemId,
const nsACString& aGUID,
const nsACString& aParentGUID)
{
// If this folder is a folder shortcut, we should never be notified for the
// removal of the shortcut (the parent node would be).
MOZ_ASSERT(mItemId == mTargetFolderItemId || aItemId != mItemId);
// Folder shortcuts should not be notified removal of the target folder.
MOZ_ASSERT_IF(mItemId != mTargetFolderItemId, aItemId != mTargetFolderItemId);
// Concrete folders should not be notified their own removal.
// Note aItemId may equal mItemId for recursive folder shortcuts.
MOZ_ASSERT_IF(mItemId == mTargetFolderItemId, aItemId != mItemId);
// In any case though, here we only care about the children removal.
if (mTargetFolderItemId == aItemId)
if (mTargetFolderItemId == aItemId || mItemId == aItemId)
return NS_OK;
MOZ_ASSERT(aParentFolder == mTargetFolderItemId, "Got wrong bookmark update");