diff --git a/browser/components/places/content/treeView.js b/browser/components/places/content/treeView.js index c4938d1ac71e..cb10e2726066 100644 --- a/browser/components/places/content/treeView.js +++ b/browser/components/places/content/treeView.js @@ -147,7 +147,7 @@ PlacesTreeView.prototype = { * node which isn't supposed to be in the tree (e.g. separators in * sorted trees). * @param [optional] aForceBuild - * @see isPlainContainer. + * @see _isPlainContainer. * If true, the row will be computed even if the node still isn't set * in our rows array. * @param [optional] aParentRow @@ -255,8 +255,6 @@ PlacesTreeView.prototype = { return this._rows[aRow] = parent.getChild(aRow - parentRow - 1); }, - _rootNode: null, - /** * This takes a container and recursively appends our rows array per its * contents. Assumes that the rows arrays has no rows for the given @@ -284,9 +282,8 @@ PlacesTreeView.prototype = { // iteration. let cc = aContainer.childCount; let newElements = new Array(cc); - this._rows = - this._rows.slice(0, aFirstChildRow).concat(newElements) - .concat(this._rows.slice(aFirstChildRow, this._rows.length)); + this._rows = this._rows.splice(0, aFirstChildRow) + .concat(newElements, this._rows); if (this._isPlainContainer(aContainer)) return cc; @@ -295,12 +292,12 @@ PlacesTreeView.prototype = { const trueLiteral = PlacesUIUtils.RDF.GetLiteral("true"); let sortingMode = this._result.sortingMode; - let rowsInsertedCounter = 0; + let rowsInserted = 0; for (let i = 0; i < cc; i++) { let curChild = aContainer.getChild(i); let curChildType = curChild.type; - let row = aFirstChildRow + rowsInsertedCounter; + let row = aFirstChildRow + rowsInserted; // Don't display separators when sorted. if (curChildType == Ci.nsINavHistoryResultNode.RESULT_TYPE_SEPARATOR) { @@ -314,7 +311,7 @@ PlacesTreeView.prototype = { } this._rows[row] = curChild; - rowsInsertedCounter++; + rowsInserted++; // Recursively do containers. if (!this._flatList && @@ -327,12 +324,12 @@ PlacesTreeView.prototype = { if (isopen != curChild.containerOpen) aToOpen.push(curChild); else if (curChild.containerOpen && curChild.childCount > 0) - rowsAddedCounter += this._buildVisibleSection(curChild, aToOpen, - row + 1); + rowsInserted += this._buildVisibleSection(curChild, aToOpen, + row + 1); } } - return rowsInsertedCounter; + return rowsInserted; }, /** @@ -343,9 +340,9 @@ PlacesTreeView.prototype = { function PTV__countVisibleRowsForNodeAtRow(aNodeRow) { let node = this._rows[aNodeRow]; - // If it's not listed yet, we know that it's a leaf node. - if (node === undefined || - !(node instanceof Ci.nsINavHistoryContainerResultNode)) + // If it's not listed yet, we know that it's a leaf node (instanceof also + // null-checks). + if (!(node instanceof Ci.nsINavHistoryContainerResultNode)) return 1; let outerLevel = node.indentLevel; @@ -415,12 +412,16 @@ PlacesTreeView.prototype = { let parent = aOldNode.parent; if (parent) { // If the node's parent is still set, the node is not obsolete - // and we should just find out its new position. However, if the node's - // parent is closed, the node is invisible. - if (parent.containerOpen) - return this._getRowForNode(aOldNode, true); + // and we should just find out its new position. + // However, if any of the node's ancestor is closed, the node is + // invisible. + let ancestors = PlacesUtils.nodeAncestors(aOldNode); + for (let ancestor in ancestors) { + if (!ancestor.containerOpen) + return -1; + } - return -1; + return this._getRowForNode(aOldNode, true); } // There's a broken edge case here. @@ -472,7 +473,7 @@ PlacesTreeView.prototype = { // If only one node was previously selected and there's no selection now, // select the node at its old row, if any. - if (aNodesInfo.length == 1 && selection.getRangeCount() == 0) { + if (aNodesInfo.length == 1 && selection.count == 0) { let row = Math.min(aNodesInfo[0].oldRow, this._rows.length - 1); selection.rangedSelect(row, row, true); if (aNodesInfo[0].wasVisible && scrollToRow == -1) @@ -1105,7 +1106,7 @@ PlacesTreeView.prototype = { isContainer: function PTV_isContainer(aRow) { // Only leaf nodes aren't listed in the rows array. let node = this._rows[aRow]; - if (!node) + if (node === undefined) return false; if (PlacesUtils.nodeIsContainer(node)) { @@ -1568,6 +1569,7 @@ function PlacesTreeView(aFlatList, aOnOpenFlatContainer) { this._tree = null; this._result = null; this._selection = null; + this._rootNode = null; this._rows = []; this._flatList = aFlatList; this._openContainerCallback = aOnOpenFlatContainer; diff --git a/toolkit/components/places/src/utils.js b/toolkit/components/places/src/utils.js index 82095eacbeb6..6dcce29100d1 100644 --- a/toolkit/components/places/src/utils.js +++ b/toolkit/components/places/src/utils.js @@ -266,6 +266,19 @@ var PlacesUtils = { return aNode.type == Ci.nsINavHistoryResultNode.RESULT_TYPE_QUERY; }, + /** + * Generator for a node's ancestors. + * @param aNode + * A result node + */ + nodeAncestors: function PU_nodeAncestors(aNode) { + let node = aNode.parent; + while (node) { + yield node; + node = node.parent; + } + }, + /** * Cache array of read-only item IDs. *