451499 - wrong folder icon appears on smart bookmarks

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[pal-moz]

Attachment: screenshot

similar bug, bug 411966

no STR, the icon turns into a different one suddenly.
but this is fixed after restart.

Comment 1

[Marco Bonardo [:mak]]

do you have any tag or is that container empty?

Comment 2

[pal-moz]

Attachment: screenshot

nothing.

this happens with new/clean profile.

Comment 3

[Marco Bonardo [:mak]]

i cant' reproduce... hwv this can't be due to redirects, appear related only to smart bookmarks, that's quite strange since they should have a null favicon_id and icon assigned through an attribute :\

Comment 4

[pal-moz]

this happens after browsing some bookmarked sites.
but I have no idea when this happens, after one site, after ten sites ....

Comment 5

[Radek 'sysKin' Czyz]

I'm suffering from this occasionally and I tried to find STR with similarly nothing came up.
BUT two observations:

1. The favicon changes at exactly the same moment when page is added to history

2. According to DOMi, the menu node (class "menu-iconic bookmark-item") gets a new attribute "image" whose value becomes (icon from w3 schools bookmark) "moz-anno:favicon:http://www.w3schools.com/favicon.ico".

Neighbouring "recent tags", which by random chance did not get a favicon this time around, doesn't have this "image" attribute at all. 

Hopefully that helps.

Comment 6

[Marco Bonardo [:mak]]

(In reply to comment #5)
> 1. The favicon changes at exactly the same moment when page is added to history

yes i see this, it happens on lazy timer (so after about 3s a visit is added)

> 2. According to DOMi, the menu node (class "menu-iconic bookmark-item") gets a
> new attribute "image" whose value becomes (icon from w3 schools bookmark)
> "moz-anno:favicon:http://www.w3schools.com/favicon.ico".

and that should not happen

i clearly see this, i'm trying to get some consistent STR. for now i've seen this often when visiting feed children from bookmarks menu.

Comment 7

[Marco Bonardo [:mak]]

this *could* be a regression from bug 433317.

My theory is that when nsNavHistoryQueryResultNode::OnItemChanged is called we call nsNavHistoryResultNode::OnItemChanged. if property is favicon of an itemId inside the container we do mFaviconURI = aValue and the place: query favicon changes.

The problem is finding why this happens for favicons

Comment 8

[Marco Bonardo [:mak]]

Attachment: patch + unit test

Not exactly a regression but that change made this visible.

When opening a nsNavHistoryQueryResultNode QUERY_TYPE_BOOKMARKS we add an AllBookmarksObserver. 

If a favicon lazy messages is executed while the container is open the nsNavHistoryQueryResultNode, as an AllBookmarksObserver, gets a onItemChanged with ATTRIBUTE_FAVICON, this request is forwarded to the underlying nsNavHistoryResult and the query gets the favicon.

I created the testcase before, finding the problem and reproducing it took some time, but the test is working (failing) as expected.

My fix is making nsNavHistoryResultNode::itemChanged working only for aItemId = mItemId, changing queryResultNode globals (favicon, title, and so on) because one of its child has changed does not appear correct.

PS: bug 433317 is missing the testcase check-in, i've however tested this patch against that test too.

Comment 9

[Marco Bonardo [:mak]]

asking blocking for possible view corruptions

Comment 10

[Dietrich Ayala (:dietrich)]

Comment on attachment 336398 [details] [diff] [review]
patch + unit test

r=me thanks

Comment 11

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/3065c0039e4d

Comment 12

[pal-moz]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080907032646 Minefield/3.1b1pre

seems to be fine.

Comment 13

[Tracy Walker [:tracy]]

verified: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b2pre) Gecko/20081112 Minefield/3.1b2pre

Comment 14

[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