517719 - Drop or move nsINavHistoryResultTreeViewer from toolkit

Status: RESOLVED FIXED

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

Description

[Asaf Romano (gone)]

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.

Comment 1

[Gervase Markham [:gerv]]

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.

Gerv

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 517719 - Remove unnecessary nsINavHistoryResultTreeViewer interface.

Review commit: https://reviewboard.mozilla.org/r/214366/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/214366/

Comment 3

[Mark Banner (:standard8)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=efde191433099958d5c93c161836640b11c982d2

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8944038 [details]
Bug 517719 - Remove unnecessary nsINavHistoryResultTreeViewer interface.

https://reviewboard.mozilla.org/r/214366/#review220214

::: 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.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8944038 [details]
Bug 517719 - Remove unnecessary nsINavHistoryResultTreeViewer interface.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/214366/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1e076595d32
Remove unnecessary nsINavHistoryResultTreeViewer interface. r=mak

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/b1e076595d32