Closed Bug 517719 Opened 10 years ago Closed 2 years ago

Drop or move nsINavHistoryResultTreeViewer from toolkit


(Firefox :: Bookmarks & History, defect, P2)




Firefox 60
Tracking Status
firefox60 --- fixed


(Reporter: mano, Assigned: standard8)



(Whiteboard: [fxsearch])


(1 file)

nsINavHistoryResultTreeViewer does not belong in toolkit. It should have been moved to tabbrowser back when we "moved" the treeview implementation.

Actually, considering that the current implementaion is an internal JS object of browser/, we could use standard js for this couple of helpers.
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Component: Places → Bookmarks & History
QA Contact: places → bookmarks
Try push:
Assignee: nobody → standard8
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 8944038 [details]
Bug 517719 - Remove unnecessary nsINavHistoryResultTreeViewer interface.

::: browser/components/places/content/treeView.js:1212
(Diff revision 1)
>      return this._getNodeForRow(aIndex);
>    },
> -  treeIndexForNode: function PTV_treeNodeForIndex(aNode) {
> +  /**
> +   * Reverse of nodeForFlatIndex, returns the row index for a given result node.

I suppose this should be "nodeForTreeIndex", not "nodeForFlatIndex".

::: browser/components/places/content/treeView.js:1218
(Diff revision 1)
> +   *
> +   * Note: This sounds sort of obvious, but it got me: aNode must be a node
> +   *       retrieved from the same result that this viewer is for. If you
> +   *       execute another query and get a node from a _different_ result, this
> +   *       function will always return the index of that node in the tree that
> +   *       is attached to that result.

I think this comment is sort of outdated, and thus confusin.
You can likely just add to the basic method description that the node to retrieve should be part of the result for this tree.

the reason for the original comment was likely that the code was in cpp and was looking for the node in its own parent result.
Attachment #8944038 - Flags: review?(mak77) → review+
Pushed by
Remove unnecessary nsINavHistoryResultTreeViewer interface. r=mak
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Priority: -- → P2
Whiteboard: [fxsearch]
Depends on: 1444599
You need to log in before you can comment on or make changes to this bug.