389090 - fix treeView.js getImageSrc() and nsNavHistoryResultNode::GetIcon() to use GetFaviconSpecForIconString(), simplify the nsIFaviconService interface

Status: RESOLVED WONTFIX

(Firefox :: Bookmarks & History, defect)

Description

[(not reading, please use [e-mail] instead)]

fix treeView.js getImageSrc() and nsNavHistoryResultNode::GetIcon() to use GetFaviconSpecForIconString()

fix nsNavHistoryResultNode::GetIcon() to take a string and not a nsIURI and call GetFaviconSpecForIconString() instead of GetFaviconLinkForIconString()

also, getImageSrc doesn't need to do:  "node.icon || defaultFavicon", as mano points out:  GetFaviconSpecForIconString does it for us

Comment 1

[(not reading, please use [e-mail] instead)]

So I'm proposing that we remove GetIcon() and do GetIconSpec() instead.

http://lxr.mozilla.org/seamonkey/source/toolkit/components/places/public/nsIFaviconService.idl

looking at the callers, we seem to always do node.icon.spec anyways.

There also appear to be unused methods / attributes on the nsIFaviconService interface that we should remove.

this is more code clean / refactoring, but would be worth doing before fx 3.  

blocking the perf meta bug, as this code path is hit when painting trees.

Comment 2

[Dietrich Ayala (:dietrich)]

sdwilsh and marco: you guys did a bunch of work on the favicon service - are these suggestions still valid?

Comment 3

[Shawn Wilsher :sdwilsh]

We don't even have these methods any more it would seem - INVALID?

Comment 4

[Marco Bonardo [:mak]]

the suggestion is similar to bug 399490, but less generic. We never use nsIURI for favicons, we always need only spec, but we do return nsIURIs, so the suggestion is to expose GetFaviconSpecForIconString in faviconService, and use that in every place we need to set .icon property on a node.
That should give back some perf in views, not sure about how much we could gain, i don't expect much.
So status of this bug depends if we are fine returning uri strings rather than nsIURI. We should consider that in the above methods we probably wrap uri in nsIURI just to return it, and we unwrap nsIURI to spec to use it.

Comment 5

[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 6

[Marco Bonardo [:mak]]

This is no more relevant, if there's something we should do is stop passing an icon property out at all, the ui can use the page-icon protocol directly.