Closed Bug 411966 Opened 17 years ago Closed 16 years ago

Wrong favicon appears in the bookmarks list

Categories

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

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: firefox.monkey, Assigned: mak)

References

(Blocks 1 open bug)

Details

Attachments

(8 files, 5 obsolete files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2 When I click on a favicon in my bookmark list, sometimes it will take the favicon from the site I just went to, and make it a favicon for another site in my bookmarks. In the attached image, it shows the gmail bookmark with the bugzilla favicon. It changed right after I went to the bugzilla site. This also happens with other bookmarks, randomly. Usually it is fixed when I click the bookmark with the incorrect favicon in my bookmarks. Reproducible: Sometimes Steps to Reproduce: 1. Click the bookmarks tab on the menu bar. 2.Click a bookmark with a favicon Actual Results: Happens once and a while, at random. Expected Results: Load the correct favicon of the site. Beltzner has told me that Firefox loads the favicon each time, so perhaps it is looking in the wrong place, or getting the incorrect favicon that is already stored in the bookmarks folder.
Attached image Wrong favicon (deleted) —
Picture showing the bugzilla favicon for the gmail bookmark.
Attached image Another example of wrong favicon (deleted) —
Microsoft favicon on the gmail bookmark. TV favicon on the microsoft bookmark.
Severity: normal → major
Component: Bookmarks → Places
QA Contact: bookmarks → places
Confirmed; I just found a bookmark in the Unfiled Bookmarks folder from http://www.badkamervoordelig.nl with a Gmail favicon.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-firefox3?
Flags: blocking-firefox3?
Version: unspecified → Trunk
Flags: blocking-firefox3?
Bug still occurs in the nightly builds as of Jan 27 2008.
Do we have any solid STR here? This is an annoyance, but I'm curious to know how often it happens.
I would say it happens to me at least once every day, always using the latest nightly build.
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
This is noticeable and I've only been using trunk builds for a few days. It happens at least a couple of times per day for me.
I was planning to file a new bug related to favicons, but perhaps my observations would better shed some light on this bug. I have very clearly observed 3b3 not displaying the same favicon image in the tab bar that it displays for the favicon URL in the Page Info|Media tab. In fact 3b3 is quite frustrating in its determination not to show the favicon last retrieved. I was mucking around with mod_rewrite to get a site favicon returned correctly. At one point, I was sending back a redirect rather than rewriting locally. Oops, not what I intended and it doesn't work. I think maybe redirects are not supported for favicons. Then I fixed this problem and my favicon still does not display. I've had problems with FF in the past caching broken/outdated favicons like pirate plunder. I type the favicon URL directly into the URL bar. FF displays the correct favicon image *and* updates both the tab and URL bar favicons to match. I go back to a URL from the site where I want the favicon to display. No dice. That annoying bland default page icon is still there. Ctrl-I into Page Info|Media: my desired favicon is the first URL listed, and correctly displayed in the Media Preview pane. It has type icon, a correct size, and correct dimensions (16x16). What do I have to do to force the fetched icon to update the tab/URL bars? Ctrl-alt-meta-cokebottle-refresh never works. Let's get out the heavy artillery. Clear-private-data|Cache. I just did this with Page Info open. Nothing changed. It behaves as if all the items are still available. However, when I close Page Info and then bring it up again, the Media tab *does* change to reflect the cleared cache: all the URLs are now displayed in dim text, the type field still has contents, but the size is now Unknown (not cached), and the Media Preview pane is empty. I click on the favicon entry at the top of the list to select it. Nothing changes. I press down-arrow to move focus to the second item on the list. Everything as expected: another blank pane. Now I press up-arrow to return focus to my favicon Media object. Amazing! The Media Preview pane now displays the favicon image. The "size" column still shows "Unknown" in dim text, but the "Size:" descriptive field now says 0.87 KB (894 bytes) / Dimensions: 16px x 16px. I now press down-arrow to go back to the second item in the list. This item, too, now displays the correct image and size. At this point if I close Page Info and bring it up fresh, the top two items are now displayed in black text with all fields filled in, with description, with preview. I can also cause a media object to update itself by changing focus with the mouse. No matter how you do it, a missing preview materializes the second time you focus the record. Ignoring the weirdness of the Page Info|Media tab for a moment, it's distressing that I can clearly see my icon in here but it *still* won't display on my tab/URL bar for the page in question. This is true even after another page on the site (the favicon page itself) *has* displayed this icon correctly on the tab/URL bars *fetched from the same URL*. There's something broken about the way FF caches icons that survives drastic-refresh and full cache discharge. OK, I might as well finish what I've started. I tailed /var/log/apache2/access.log and repeated these steps. I can now see that after clearing the cache, when I bring up Page Info, as soon as I focus the Media tab, all the items show in dim with the first item selected by default. FF then quickly fetches this selected item in background, but doesn't update the Media display at all. If you cursor away from this item and back into it, FF updates the description and preview portions of the display, but not the column info grid. That's broken any way you slice it. It would be a good start to fix favicon handling so that the favicon in the tab/URL bar *always* showed the same image the Media pane shows for the same URL. That's the bigger issue. Then people wouldn't need to speculate about how many hundred years some previously horded favicon was buried in the sand before FF popped it onto the URL bar in preference to the image it recently fetched for public consumption in the Media pane. Yes, this has been a long-standing source of frustration, but 3b3 seems worse than 1.5 used to, because I usually did manage to force icon update in the old days, but this time, still no go. My Ubuntu server in the closet with FF 2.x displays my favicon correctly, while on the same page my FF 3b3 *after all this* still won't. I'm not sure why 3b3 has such a grip on the wrong icon for this page, but bear in mind during my mod_rewrite exploits I did send it some strange results for this favicon request along the way. It would probably fix itself if I restarted 3b3, but I hate doing that, I have far too many tabs open with half-edited wiki pages.
Cliff Notes version, please?
I was just trying to view an icon file to select an icon for a companion web site to the one from this morning using the same FF instance. The icon didn't display on the first few attempts (after other icons had). I caught FF 3b3 spitting errors onto my console during the failed icon load attempts. A back/forward and a click, then the icon worked. Meanwhile, the failed attempts had spit out: (Gecko:3804): Gdk-CRITICAL **: gdk_x11_visual_get_xvisual: assertion `visual != NULL' failed (Gecko:3804): Gdk-WARNING **: GdkWindow 0x393ed98 unexpectedly destroyed (Gecko:3804): Gdk-WARNING **: GdkWindow 0x393ed94 unexpectedly destroyed (Gecko:3804): Gdk-CRITICAL **: gdk_x11_visual_get_xvisual: assertion `visual != NULL' failed My 3b3 has been running for 10 days now, and mostly worked great (much better than 3b1 or 3b2 under the same work load). The cliff edition of my previous post: 1) Under 3b3 the Page Info|Media pane fetches item in background whenever an item is selected, but fails to update the display in multiple different ways (c.f. War and Peace, above). 2) Under 3b3 the wrong icon was displaying in my URL bar and the page tab, despite massive efforts to reload the icon, and evidence that the icon refresh had succeeded at the level of Page Info|Media pane, and as viewed by my Apache log. A different FF directed at the same web site *does* display the icon correctly. There is some situation with 3b3 where it decides for a certain set of pages to display some other icon than the correct icon most recently fetched, even when it will correctly associate the same icon fetched from the same URL on other pages at that site.
After reporting those error accessing an icon image, my FF instance went insane, now requires a restart. These came spewing on a back/forward pages transitions for all types of pages. Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9b3) Gecko/2008020513 Firefox/3.0b3 (Gecko:3804): Gdk-CRITICAL **: gdk_window_set_user_data: assertion `window != NULL' failed (Gecko:3804): Gdk-CRITICAL **: _gdk_window_destroy_hierarchy: assertion `window != NULL' failed (Gecko:3804): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed (Gecko:3804): Gdk-WARNING **: GdkWindow 0x393edc9 unexpectedly destroyed (Gecko:3804): Gdk-CRITICAL **: gdk_window_set_user_data: assertion `window != NULL' failed (Gecko:3804): Gdk-CRITICAL **: gdk_window_set_back_pixmap: assertion `GDK_IS_WINDOW (window)' failed (Gecko:3804): GLib-GObject-CRITICAL **: g_object_set_data: assertion `G_IS_OBJECT (object)' failed (Gecko:3804): GLib-GObject-CRITICAL **: g_object_set_data: assertion `G_IS_OBJECT (object)' failed (Gecko:3804): Gdk-CRITICAL **: gdk_x11_visual_get_xvisual: assertion `visual != NULL' failed (Gecko:3804): Gdk-CRITICAL **: gdk_window_hide: assertion `GDK_IS_WINDOW (window)' failed (Gecko:3804): Gdk-CRITICAL **: gdk_window_set_user_data: assertion `window != NULL' failed (Gecko:3804): Gdk-CRITICAL **: _gdk_window_destroy_hierarchy: assertion `window != NULL' failed (Gecko:3804): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed (Gecko:3804): Gdk-CRITICAL **: gdk_window_hide: assertion `GDK_IS_WINDOW (window)' failed (Gecko:3804): Gdk-CRITICAL **: gdk_window_set_user_data: assertion `window != NULL' failed (Gecko:3804): Gdk-CRITICAL **: _gdk_window_destroy_hierarchy: assertion `window != NULL' failed (Gecko:3804): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed
Since the last crash, I've spent an hour reproducing various scenarios, and I can now shed some light on the situation. Under 3b3, if FF receives a 404 when it first attempts to fetch http://site.domain/favicon.ico, it's pretty much impossible to get that favicon to actively display in the URL bar or tab without a FF restart, even if: 1) you press wimpy refresh 2) you press hardcore refresh 3) you clear the browser cache 4) you visit http://site.domain/favicon.ico directly 5) you witness Page Info|Media background fetch http://site.domain/favicon.ico In case (4) you are momentarily deluded by FF previewing the current icon image in the URL/tab bar. Later I noticed it preview icons from other locations as well, and then the light came on. In case (5) the favicon URL was in my Media list because MediaWiki supplied the following headers: <meta name="keywords" content="Favicon,Security portal" /> <link rel="shortcut icon" href="/favicon.ico" /> I observed FF fetch this URL in background (tailing the Apache access log) while I was focusing objects in the Page Info|Media tab, but it still wouldn't update for the site URL/tab bars. In the case where FF 3b3 obtains a valid icon on its first access to http://site.domain/favicon.ico, it's less difficult to refresh it with a different after changing the icon file. I believe accessing the icon directly has this effect, as does reloading the page after clearing the cache. My housemate has XP 2.0.0.12 running. She had visited pages at http://site.domain/ before I installed the icon file. Her older FF was also reluctant to use this new icon. Refresh/hard refresh did not work. Going into Page Info|Media appears to have had the same effect of loading the icon file in background, only the information update was even more partial and broken that 3b3. The size was correctly updated to 3072 bytes while the dim was stuck at 0px x 0px and the display pane was blank. However, after leaving Page Info, the next page transition within the site caused the icon to materialize on the tab bar. Going back into the Media tab, it was also now showing the icon properly. That was how I used to activate icons back when I used 1.5.x I would bumble around in Page Info|Media and then refresh pages until the desired icon appeared. This no longer works under 3b3, nor apparently does any other measure I've tried short of restarting the browser instance. Both 2.x and 3b3 update the Media pane in strange and partial ways. I don't recall if 1.5.x was strange in this dept. as well. I understand after reading the Wikipedia article that there is a desire to suppress unnecessary loads to the site favicon.ico when the file is 404. The current algorithm is *too* successful and violates the principle of least astonishment: after I *see* the browser fetch my most recent http://site.domain/favicon.ico, I certainly expect to then see it displayed on tabs associated with that site. The data structure which records the 404 response to attempted icon loads needs to ask the cache "give me this if you have it, but don't attempt to fetch". If this rule was implemented, a person wishing to refresh a site favicon could visit the icon URL directly, and force the cache to obtain the file by that method. Furthermore, clearing the Firefox cache should also clear the favicon 404 memo structure, allowing that method to also work for triggering a favicon reload. I have no idea whether my browser sickness on trying to access a favicon image was associated with these observations, or was a case of random browser sickness while I was mostly messing with icons problems. Finally, the Page Info|Media pane could do a much better job of staying up to date with the image cache.
Same problem appearing on my system (Linux, FF3B4.) I don't think it was happening with the first couple of betas (though I may not have noticed) but it seems to have been happening at least since Beta3. I'm actually spotting this with bookmarks in my toolbar which point at link collections on a server on my local network: the correct favicon initially displays in the bookmark but about once a day, it will change to the favicon of a site I've navigated to FROM that page. As an example, in the screenshot I'll add, the "Home" link (actually a bookmark because I don't like the icon from the theme I'm using) has picked up Spamcop's favicon when I navigated there from my home links page. Similarly the "Connections" bookmark has taken the favicon from a site I took a link to from the bookmarked page.
This isn't great, but I don't think we have solid STR here, and I'm willing to ship with this.
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
OS: Windows XP → All
I've actually noticed this as well. It's only happened to me once, but I did think it was worth noting. When I tried to access my wiki-on-a-stick (which has no favicon) through my bookmark, I did not have my personal server running (own mistake) so I got a page not found error. Firefox then began using the favicon of the site I had come from for my own wiki - which as I said, has no favicon.
When that happens could you execute this SQL query in sqlite manager? SELECT f.url from moz_places h JOIN moz_favicons f ON f.id = h.favicon_id WHERE h.url = "address_with_wrong_favicon" clearly replacing address_with_wrong_favicon with the failing address, then report if it finds anything and if the found url is pointing to the wrong icon. At least we will know if the wrong icon is saved in the db, or if it's a frontend only problem.
Hi Marco, I've run the query on my system and it returns the URL of the favicon being displayed, rather than the one that SHOULD be displayed. So the problem seems to be what's being saved in the database.
Depends on: 428481
Rather than start a new bug I thought I would tack this here since it seems related. 1 - Go to http://weather.yahoo.com/forecast/USIL0225_f.html 2 - Bookmark it, it will have the yahoo favicon 3 - Clear the cache and close the window 4 - Open the window and go to the forecast via the bookmark 5 - Now click on the 6-10 Day Extended Forecast link 6 - Check the bookmark and the favicon will be for the weather channel 7 - Click the back button and the bookmark favicon goes back to yahoo Somehow I doubt this is how it's supposed to work.
(In reply to comment #19) > I can't reproduce your step 6.
> (In reply to comment #20) > I'm using FF3b5 on a Mac. I don't know what is going on. Yesterday I tried this five times with the results I noted in comment #19. I just tried again and can't reproduce step 6. If I rediscover the "magic" I'll post. I've noted that favicon behavior is a little flaky. When I moved to FF3b4 from FF2 I had a number of custom favicons in the bookmarks.html file. When I started using FF3 some of those custom favicons stuck while others reverted to the default for the sites when I visited the site. I poked around in the places.sqlite data base with a SQLite editor but was unable to see why.
It happened again, see attachment. See my comments 19 and 21.
what i was noticing from the various reports is that: 1. the problem does not always happen 2. it does not happen to many users (i cannot reproduce it, and is not reported so frequently) 3. it is happening when you visit a page starting from another, and the new page takes the favicon of the referring one (this feels like important). 4. we are explicitely asking to save that favicon in the db, since it exists in the db(exclude all frontend possible icon initialization bugs). 5. i did not found problems adding tests to the favicon service. Due to this i'm starting thinking that this could be an issue due to our code to handle favicons for redirects, we try to be nice using the originating favicon through UpdateBookmarkRedirectFavicon, thich uses bookmarks->GetBookmarkedURIFor which uses our internal bookmarks forward hash or the from_visit history column. Now, if i suppose that, for any reason, a referrer is wrongly marked as a redirect, i end up setting the wrong favicon on the items originating(referred) from that page. I'm trying to follow the code to see if this is possible, but could be a candidate culprit.
Let's see if we can detect such a situation. When you see a wrong favicon, please try executing this query in Sqlite Manager extension, and look for the uri that has wrong favicon (paste that result here please): SELECT (SELECT url FROM moz_places WHERE id = v1.place_id), (SELECT url FROM moz_places WHERE id = v2.place_id), v2.visit_type FROM moz_historyvisits v1 JOIN moz_historyvisits v2 on v2.from_visit = v1.id WHERE v2.visit_type = 5 OR v2.visit_type = 6 GROUP BY v2.place_id what should be checked is if the other url near our can be connected to the showed wrong favicon.
An interesting result to the above SQL. I actually get several entries for one of the offending URLS: http://gent/cgi-bin/links.cgi http://www.xcalibre.co.uk/status/status.php 6 http://gent/cgi-bin/links.cgi https://cp.xcalibre.co.uk/login.php 6 http://gent/cgi-bin/links.cgi http://www.xcalibre.co.uk/status/status.php 6 http://gent/cgi-bin/links.cgi https://cp.xcalibre.co.uk 6 http://gent/cgi-bin/links.cgi http://www.thinkbroadband.com/ 6 http://gent/cgi-bin/links.cgi http://www.misco.co.uk/ 6 http://gent/cgi-bin/links.cgi http://www.misco.co.uk/indexuk.asp? 6 http://gent/cgi-bin/links.cgi http://vms.pdv-systeme.de/users/martinv/pgg/06S/06S008.html 5 I think this may not be helpful, as none of these points to where the favicon being displayed for the link comes from. However, in my case at least, assumption 3 is the wrong way round. The link to a page is taking the favicon from the page I take a link to from it. For example, http://gent/cgi-bin/links.cgi is actually displaying the favicon from http://www.spamcop.net, which is linked to on links.cgi
So, on links.cgi you have a link to spamcop.net, if you visit spamcop.net and then links.cgi is not available, its favicon is replaces with that from spamcop?
I'm not sure what you mean by "links.cgi is not available" but the gist of what you say is correct. Originally links.cgi had the correct favicon (and occasionally gets it back) but on the bookmarks bar, it displays the spamcop favicon. This can be seen in the screenshot I'll attach straight after saving this: the link at the left titled "Home" points to links.cgi.
Attached image Screenshot showing wrong favicon (deleted) —
Attached patch patch for tryserver build (this is NOT a fix) (obsolete) (deleted) — Splinter Review
So let's do an experiment, this is a patch that disables UpdateBookmarkRedirectFavicon. We can create a Tryserver build with this in (Dietrich will follow me with the link to download the build), you should try using that build for some hour and tell if you notice any change. Then you should (after creating a backup of you profile) try to clear history and again navigate some time with it, trying to reproduce the problem... if the problem will be still visible in both cases we can probably discard this theory.
(In reply to comment #24) > Let's see if we can detect such a situation. > When you see a wrong favicon, please try executing this query in Sqlite Manager > extension, and look for the uri that has wrong favicon (paste that result here > please): > > SELECT > (SELECT url FROM moz_places WHERE id = v1.place_id), > (SELECT url FROM moz_places WHERE id = v2.place_id), > v2.visit_type > FROM moz_historyvisits v1 > JOIN moz_historyvisits v2 on v2.from_visit = v1.id > WHERE v2.visit_type = 5 OR v2.visit_type = 6 > GROUP BY v2.place_id > > what should be checked is if the other url near our can be connected to the > showed wrong favicon. > Can you give some more steps on how to do this. I'm not understanding what do do after before or after running this statement. I don't get any thing except three columns and nothing under them. BTW I haven't seen this mentioned yet, but I also see this in Firefox 2.0. And this is where I am running this query in.
I've downloaded the Linux build; probably won't be able to give it a good testing until tomorrow. However, I've just spotted something that might be significant.The pages I've been seeing this on are on a webserver that doesn't actually HAVE a favicon.ico in the DocumentRoot directory. As soon as I copy one into that directory and open one of the pages, the icon alongside the link "magically" changes into the correct one. So the problem may not be with general favicon handling, but instead could be in the way Firefox handles a page it can't find a favicon for.
Given the test build a good try and it seems to have fixed the problem. Apologies for taking so long to report back.
please still continue to try with all cases were previously creating problems to you... did you have to do anything special like clearing cache, history or restarts?
I'm carrying on using the test build as my standard browser to see if anything recurs, but at the moment it's looking good. I've even cleared browsing history and cache without any problems so far.
I am going to risk making a fool of myself, so ignore this completely if it obvious I don't know what I am talking about. Based on my experience of when this happens - need one expired icon (i1) and one new or expired icon (i2) and a page that takes a while to load (eg tinderbox), should get page1=>i1 page2=>i2 but get instead page1=>i2 - I have gone through the faviconloadlistener code several times and I can't see how it holds a reference to stop the URIs mPageURI and mFaviconURI from dying. I think I expect addref for them in FaviconLoadListener and release in ~FaviconLoadListener. Sorry if this is a red herring and the lifetime is correct from someting that I've missed.
John's suggestion fits with what I'm seeing. The pages whose favicons are replacing those of the pages navigated from ARE slow loaders. At least the ones I'm getting the problem with are. Spamcop, for example takes quite a while to load and consistently replaces the favicon of my link page. The expired icon is the one that's getting replaced.
(In reply to comment #36) > Sorry if this is a red herring and the lifetime is correct from something that > I've missed. OK I've got it now! I'll go back to quietly reading the code...
Okay, so I had a little poke around after seeing this on my bookmark toolbar again. Here's what I found. SELECT * FROM moz_places WHERE favicon_id IN ( SELECT id FROM moz_favicons WHERE url LIKE '%techreport.com%' GROUP BY id ) Only noteworthy result was this: 527 http://surrey.facebook.com/home.php? Facebook | Home moc.koobecaf.yerrus. 2 0 1 1764 300 I then ran the SQL above > SELECT > (SELECT url FROM moz_places WHERE id = v1.place_id), > (SELECT url FROM moz_places WHERE id = v2.place_id), > v2.visit_type > FROM moz_historyvisits v1 > JOIN moz_historyvisits v2 on v2.from_visit = v1.id > WHERE v2.visit_type = 5 OR v2.visit_type = 6 > GROUP BY v2.place_id There was no obvious mismatch in the results.
Attached file places.sqlite before (deleted) —
OK, this fails for me every time (well four times in a row): 1. From the profile manager create a new profile 2. Before starting minefield copy the attachment (places.sqlite) into the profile's folder 3. Start minefield 4. Click on mz bookmark in toolbar 5. Click on 'Tree Status' in right navigation (I could replace step 2 put it would involve a step 'wait more than 24 hours') The mozillazine icon is replaced with the tinderbox one.
Attached file places.sqlite after (deleted) —
While reading the source I have noticed an oddity [I can't call it a bug] in the bookmark hash, involving bookmarks/redirect/timeout but I don't think it has any effect here. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/places/src/nsNavHistory.cpp&rev=1.306&mark=99-101#99 The define for BOOKMARK_REDIRECT_TIME_THRESHOLD suggests that this should be two minutes, but the comment implies that it is only twelve seconds. Perhaps this is why PR_USEC_PER_SEC was invented.
(In reply to comment #42) > While reading the source I have noticed an oddity [I can't call it a bug] in > the bookmark hash, involving bookmarks/redirect/timeout but I don't think it > has any effect here. can you fill a new bug for that please?
- you visit "http://www.mozillazine.org/" (visit_id = 3) - clicking on the tree status link adds a visit to "http://www.mozillazine.org/rd/mztb/" (from_visit = 3) - it redirects to "http://tinderbox.mozilla.org/Firefox" with a TRANSITION_REDIRECT_TEMPORARY(6) (from_visit = 3) - and then to http://tinderbox.mozilla.org/Firefox/ with a TRANSITION_REDIRECT_PERMANENT(5) (from_visit = 3) you have a bookmark on "http://www.mozillazine.org/" (place_id = 44) so i suppose my first theory about our smart handling of bookmark redirect favicon was not totally wrong, we are taking icon from a site that is a TEMPORARY/PERMANENT redirect from a link in your bookmarked page.
Attached patch patch (obsolete) (deleted) — Splinter Review
This still requires some way for automatic testing but probably we are on the issue. The issue i see in the above comments (thank you John for having provided a reproduceable test, i spent much time trying to reproduce without any luck, till now) is that we always set from_visit to the original URI in case of redirects, so when calling AddVisitChain recursively all pages are marked as if they were visited from the original bookmarked page, instead only the link is, redirects should have from_visit set to the page we have been redirected from. my STRs to reproduce the problem: 1. create a new profile 2. Install Sqlite Manager 3. Visit http://www.mozillazine.org/ 4. Click on the "Tree status link" in the right panel menu 5. open places.sqlite with Sqlite Manager 6. execute the following query: SELECT url FROM moz_historyvisits JOIN moz_places h ON h.id = place_id WHERE from_visit = (SELECT v.id FROM moz_historyvisits v JOIN moz_places p ON p.id = v.place_id WHERE p.url="http://www.mozillazine.org/") 7. In the resultSet we should not have tinderbox.mozilla.org pages since we have not been redirected directly from http://www.mozillazine.org/ in general, if you replace http://www.mozillazine.org/ with the url you are seeing the icon replaced, you should be able to see that you get urls of the page the favicon is taken from. Dietrich, i don't think we can take this without some sort of unit test (i'm figuring out how to make that), i'm setting the r? to have your attention and opinion on this.
Attachment #320699 - Flags: review?(dietrich)
Attachment #319398 - Attachment is obsolete: true
Whiteboard: [has possible patch][needs unit test]
Attached file test first take (obsolete) (deleted) —
this is a somehow complex test for this, not suitable for unit test since it accesses the mozillazine page to test the thing. so 2 problems: 1. this kind of test requires a web server 2. now in case of a page -> link -> redirect we do redirect.from_visit = page. my patch changes it as redirect_from_visit = link. But is that correct? The favicon service uses from_visit to do smart favicon replacement, that works fine in the latter case, but in the former one it thinks that redirect is a permanent_redirect for page, and replaces favicon.
Just a note that this, currently, seems similarly broken in the case when it _should_ update the icon - 1. Context click on bookmark toolbar, and add new bookmark Name: is Location: http://www.bris.ac.uk/is 2. Click on the new bookmark Icon on toolbar doesn't change. 3 -> from_visit 0, place_id 45 (http://www.bris.ac.uk/is), visit_type 3 4 -> from_visit 0, place_id 46 (http://www.bris.ac.uk/is/), visit_type 5
Confirming bug in RC1
Just visit all the pages again then they may be recovered
Simple procedure to reproduce: 1. Bookmark http://www.mozillazine.org/.(Yes it's mozillaZine.) 2. Open above page. 3. Click "Today's Checkins" link on the right side. The favicon of mozillaZine will change the one of Bonsai. It will be return to the correct favicon if you back to the mozillaZine page.
(In reply to comment #50) > Simple procedure to reproduce: > 1. Bookmark http://www.mozillazine.org/.(Yes it's mozillaZine.) > 2. Open above page. > 3. Click "Today's Checkins" link on the right side. > > The favicon of mozillaZine will change the one of Bonsai. > It will be return to the correct favicon if you back to the mozillaZine page. I am sorry for wrong information. I have bookmarked the above link("Today's Checkins") and delete the bookmark. So I cannot reproduce with the procedure any longer.
The firefox max changed the way to retrieve favicon. When image is disabled, it's more likely to cause wrong favicon
Comment on attachment 320699 [details] [diff] [review] patch >Index: toolkit/components/places/src/nsNavHistory.cpp >=================================================================== >RCS file: /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v >retrieving revision 1.306 >diff -u -8 -p -r1.306 nsNavHistory.cpp >--- toolkit/components/places/src/nsNavHistory.cpp 9 May 2008 07:25:29 -0000 1.306 >+++ toolkit/components/places/src/nsNavHistory.cpp 13 May 2008 12:33:19 -0000 >@@ -4319,16 +4319,20 @@ nsNavHistory::AddVisitChain(nsIURI* aURI > &referringVisit, aSessionID, aRedirectBookmark); > NS_ENSURE_SUCCESS(rv, rv); > > // for redirects in frames, we don't want to see those items in history > // see bug #381453 for more details > if (!aToplevel) { > transitionType = nsINavHistoryService::TRANSITION_EMBED; > } >+ >+ // this call will create the visit and create/update the page entry >+ return AddVisit(aURI, visitTime, redirectURI, transitionType, >+ aIsRedirect, *aSessionID, aVisitID); > } else if (aReferrer) { > // We do not want to add a new visit if the referring site is the same as > // the new site. This is the situation where a page refreshes itself to > // give the user updated information. > PRBool referrerIsSame; > if (NS_SUCCEEDED(aURI->Equals(aReferrer, &referrerIsSame)) && referrerIsSame) > return NS_OK; > the change looks correct, r=me. fwiw, i believe the mochitest webserver does support customizing HTTP response codes, so you could probably get a test that way. however, i'm in favor of getting this in the nightlies and getting feedback instead of blocking on that. one change: i don't like the early return here. please just create a temp variable for the referrer, and modify it in the redirect case.
Attachment #320699 - Flags: review?(dietrich) → review+
Flags: in-testsuite?
Whiteboard: [has possible patch][needs unit test] → [needs new patch]
Priority: P3 → P2
Hardware: PC → All
Target Milestone: --- → Firefox 3.1
Attached patch fixed comments (obsolete) (deleted) — Splinter Review
dietrich: if that's ok we can check-in this, i'll move taking a look at the mochikit webserver custom response, i've found documentation about that
Attachment #331310 - Flags: review?(dietrich)
Whiteboard: [needs new patch] → [has patch][needs mochikit test]
requesting a litmus test for this. see comment #50 for succinct reproduction steps.
Flags: in-testsuite? → in-litmus?
Comment on attachment 331310 [details] [diff] [review] fixed comments r=me thanks
Attachment #331310 - Flags: review?(dietrich) → review+
Dietrich, I'm unable to reproduce with STR's of comment 19 or 50. Is it a consensus that this bug is not 100% reproducible? I can add a test case to Litmus with STR of comment 50, if you wish. However, I'd prefer to have a reliably reproducible test.
Whiteboard: [has patch][needs mochikit test] → [has patch]
For people able to reproduce: have you determined if it matters how the bookmark was created (Star vs Menu) and/or where it is placed (Toolbar vs Menu vs Unsorted vs other)?
Assignee: nobody → mak77
Status: NEW → ASSIGNED
I placed on Unsorted by clicking star, and bookmarks inside subfolders has wrong icon and Unsorted bookmark almost doesn't have wrong icon
the simplest thing is probably a mochitest, i'll go for it, i'm actually looking after another mochitest for trees that gave me problems, this will be the next finally
Attachment #320699 - Attachment is obsolete: true
Attachment #320741 - Attachment is obsolete: true
Comment on attachment 331310 [details] [diff] [review] fixed comments now, i have a valid mochitest but something has changed and the patch does not work anymore, the problem is still the same though... So i'm investigating on what happened in the meantime
Attachment #331310 - Attachment is obsolete: true
Attached patch patch with mochitest (obsolete) (deleted) — Splinter Review
Please ignore comment #63, it was a bustage in my custom build, after a full rebuild i got patch and test both working. Asking review for the mochitest part, it's quite verbose but i wanted to reproduce the exact case we have talked about in these comments, so a page with a link that makes 2 redirects to a third page. I'm not directly checking the favicon, but the database correctness for the from_visit column. Notice that after applying the patch some favicon *could* still be wrong due to old visits with a wrong from_visit value. New visits will not have the problem.
Attachment #332577 - Flags: review?(dietrich)
Comment on attachment 332577 [details] [diff] [review] patch with mochitest ok, i think this is ok for verifying the from-visit changes. r=me. however, without solid steps to reproduce, i think it's still not clear the original problem is fully addressed. let's get this in for mitigation of the issue, and keep an eye on feedback.
Attachment #332577 - Flags: review?(dietrich) → review+
Keywords: checkin-needed
(In reply to comment #65) > (From update of attachment 332577 [details] [diff] [review]) > ok, i think this is ok for verifying the from-visit changes. r=me. > > however, without solid steps to reproduce, i think it's still not clear the > original problem is fully addressed. let's get this in for mitigation of the > issue, and keep an eye on feedback. > I have a 100% guaranteed sequence of events on my system (one involving a site I need to log in) which will make this happen. I'm happy to install a build with the fix in to test it. (though due to other bugs, I can only use a standard 32 bit build temporarily as there are other issues on my 64 bit system.)
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch]
Target Milestone: Firefox 3.1 → Firefox 3.1a2
Comment on attachment 332577 [details] [diff] [review] patch with mochitest >+ setTimeout("checkDBOnTimeout()", 4000); next time, please write this as: setTimeout(checkDBOnTimeout, 4000);
Flags: in-testsuite+
backed out... gmake[6]: *** No rule to make target `/builds/moz2_slave/mozilla-central-linux-debug/build/toolkit/components/places/tests/mochitest/test_bug_411966/Makefile.in', needed by `mochitest/test_bug_411966/Makefile'. Stop. gmake[6]: Leaving directory `/builds/moz2_slave/mozilla-central-linux-debug/build/obj-firefox/toolkit/components/places/tests' gmake[5]: *** [export] Error 2 gmake[5]: Leaving directory `/builds/moz2_slave/mozilla-central-linux-debug/build/obj-firefox/toolkit/components/places' gmake[4]: *** [export] Error 2 gmake[4]: Leaving directory `/builds/moz2_slave/mozilla-central-linux-debug/build/obj-firefox/toolkit/components' gmake[3]: *** [export_tier_toolkit] Error 2 gmake[3]: Leaving directory `/builds/moz2_slave/mozilla-central-linux-debug/build/obj-firefox' gmake[2]: *** [tier_toolkit] Error 2 gmake[2]: Leaving directory `/builds/moz2_slave/mozilla-central-linux-debug/build/obj-firefox' make[1]: *** [default] Error 2 make[1]: Leaving directory `/builds/moz2_slave/mozilla-central-linux-debug/build/obj-firefox' make: *** [build] Error 2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 3.1a2 → ---
Flags: in-testsuite+
Attached patch fixed bustage (deleted) — Splinter Review
problem was due to a case sensitive naming bug :( it is compiling fine for me. While there i've changed the test to use NSPIPlaces to get the places db connection, a minor needed change though.
Attachment #332577 - Attachment is obsolete: true
Keywords: checkin-needed
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Hrm, I think I still see this issue, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080823032129 Minefield/3.1a2pre I have a bookmark of http://planet.mozilla.org/ and a bookmark of http://developer.mozilla.org/web-tech/ The planet.mozilla.org sometimes gets the favicon of the developer.mozilla.org/web-tech/ in that trunk build. I guess I should file a new bug for that or something?
(In reply to comment #72) > I have a bookmark of http://planet.mozilla.org/ > and a bookmark of http://developer.mozilla.org/web-tech/ > The planet.mozilla.org sometimes gets the favicon of the > developer.mozilla.org/web-tech/ in that trunk build. In planet mozilla i see a link to http://developer.mozilla.org/web-tech that will be clearly redirected by the server to http://developer.mozilla.org/web-tech/ with a HTTP/1.x 301 Moved Permanently. Your bookmark is on the correct url (with final slash) or on the bad url? can you execute this query in sqlite manager? SELECT v.id, v.from_visit, v.visit_type, h.url FROM moz_historyvisits v JOIN moz_places h ON h.id = v.place_id WHERE url = "http://planet.mozilla.org/" OR url = "http://developer.mozilla.org/web-tech" OR url = "http://developer.mozilla.org/web-tech/" ORDER BY from_visit i get this result in FX3.0 "18885","0","1","http://planet.mozilla.org/" "18890","18885","1","http://developer.mozilla.org/web-tech" "18891","18885","5","http://developer.mozilla.org/web-tech/" while after the patch i should see "18891","18890",... and as you can see everything is originating from 18885 that is planet mozilla. so the problem appear the same that the patch should solve, as i stated in comment 64 the patch does not solve the problem for existing visits but only for new ones, so you could try removing visits relative to web-tech and see if that still happens. If you can try the query to remove old visits is DELETE FROM moz_historyvisits v WHERE id IN ( SELECT v.id FROM moz_historyvisits v JOIN moz_places h ON h.id = place_id WHERE url >= "http://developer.mozilla.org/web-tech" AND url < "http://developer.mozilla.org/web-tech~" ) after that you can try to go to planet mozilla, visit the web tech wrong link and execute the query above to check from_visit column, visit with type 5 (redirect permanente) should definately not report planet mozilla as the from_visit.
Why is this marked as Resolved/Fixed when it still happens? Even after the very latest update? My experience of this bug has been with external links on certain sites where the site uses a referrer URL. You expect the link to to: www.targetsite.com It actually goes to: www.previoussite.com/reflink?=1 - which then redirects to targetsite.com Because of this, Firefox appears to load the targetsite.com favicon as the favicon for previoussite.com.
(In reply to comment #78) > Why is this marked as Resolved/Fixed when it still happens? Even after the very > latest update? because this will be fixed for 3.1 and you're using current version that is 3.0.4.
I am still seeing the problem with 3.1beta2, having this page : http://damien.wyart.free.fr/bookmarks.html in my bookmarks toolbar, when clicking some links, the favicon is changed to the one of the visited site. Maybe it is less frequent than with 3.0.4 but anyway, this happens. I have been annoyed by this for months and was hoping 3.1 would solve the problem, but it seems this is not the case for now.
you should try after a clear history, since old redirects could still be there
Ok, this seems to solve the problem, thanks for the suggestion.
you're welcome, please report back if in future you see the issue again
Blocks: 120352
Test cases have been updated on litmus for regression testing on 3.x test runs. For 3.1, https://litmus.mozilla.org/show_test.cgi?id=5953 For 3.0, https://litmus.mozilla.org/show_test.cgi?id=3995
Flags: in-litmus? → in-litmus+
I'm running 3.1beta3, and I still get wrong favicon sometimes (although I think less than before).
I've been using 3.1beta3 for longer now, and I think I get EVEN MORE wrong favicons than I did in 3.0. It definitely doesn't seem to be fixed, can we change the status of this bug?
(In reply to comment #93) > can we change the status of this bug? No, if you have any issue you should file a new bug specifying steps to reproduce and address of pages that give you wrong icon.
Frankly, that's dumb. It loses the bug history, the context of past issues, and the links to duplicate bug filings and similar issues. The only reason I can see for doing that seems to be 'face saving' for marking a bug fixed when it wasn't.
It has nothing to do with "face saving", and everything to do with using the tools we have effectively. The bug summary might describe the same symptoms, but this bug was used to track a specific code-level change, and re-using the same bug to track multiple such changes is a nightmare for developers.
No, this entry is to track the bug. Not to track code changes in an attempt to fix the bug, which turn out not to have fixed the bug. If you've been habitually using bug tracking like that, it's a misunderstanding. Tickets are associated with issues, not attempt to fix the issue. You don't ask people to have to keep refiling a ticket each time you did something new to try and fix it which didn't work.
Again, I'd like to stress this... Developers, if you are using Public Bug Tickets as Project Management... STOP IT. If you have to have a ticket to center project management around, open your own one in private. Bug Report Tickets are for Reporting Bugs. Don't muddy it by using them for project management, since that causes problems when the bug turns out not to be fixed, or has more than one cause.
john: you're a guest here. we may at some point change how we're doing things. but i'd suggest you read http://www.bartleby.com/59/3/youcancatchm.html and think about it carefully. as it happens, bugzilla is being used by our engineers to track technical bugs. everyone else is a guest. if you want support, we offer http://support.mozilla.com/ You should also read https://bugzilla.mozilla.org/page.cgi?id=etiquette.html which is on the front page of our bugzilla .... especially note 1.3.
Okay, here's the 'more flies with honey' version... You're making extra work for yourself by tying bug reports to individual project management, instead of having separated tickets for that project management. Here are the demonstrated downsides of doing it your way, from this bug alone, 1) Confusion as the bug reported for the 3.0 branch is marked as fixed, but the fix is only in the 3.1 branch. 2) Users reporting bugs have to read through long project management discussions to find out if they're reporting the same bug, or look for duplicates of this bug. 3) The ticket becomes tunnel-vision focused on what the project management is directed at. 4) Because they are the same Bug Description, other bug tickets that are not addressed by the 'fixes' being worked on are marked as duplicates of this bug and closed. 5) The bug ends up being marked resolved, because one problem that project management is focused on is fixed, the bug is closed as resolved, without checking that all the examples of the bug behavior are fixed. 6) Since the bug isn't fixed, people complain. 7) People are told to *deliberately duplicate the ticket*. If you separate out bug tickets from project management tickets, most of these problems don't happen.
so, you've again violated this bug. more flies with honey doesn't mean smear honey over any random place. if you put honey on a computer, you'll still have a very unhappy computer. and yes, i'm well aware of what you want. i wrote a real proposal on this subject *years* ago: http://viper.haque.net/~timeless/blog/77/ http://viper.haque.net/~timeless/blog/78/ But you're missing the point. Advocacy of any kind does not belong in bugs about other issues. And it certainly doesn't belong in resolved bugs. And again, advocacy from guests is generally unwelcome. Contribute positively, establish a good reputation, and *then* make a proposal in the appropriate forum. Note: I'm redacting these comments as I go. Bugzilla will keep them and certain people will be able to see them, however, normal users will be saved from reading irrelevant comments in this bug. If you comment again, for any reason, i'll have your account suspended.
Thanks for the suggestion on how to run a bug tracker installation.
I created bug 487003 for this continuing problem. I took the liberty of adding everyone CCed on this bug to the new bug as well. I hope some people will make a renewed effort to reliably reproduce this bug. It happens to me many times every day, but I can't make it occur on demand. :-(
This bug is NOT FIXED! Please change the status back. I am using 3.5.2 and still getting this problem all over the place. It's actually fairly annoying since I don't use text labels on my bookmarks toolbar and just rely on the favicons.
I still get the "Badware" favicon for a site in my awesome bar results that currently has no favicon, and has been clean for months. (msy.com.au) Mozilla/5.0 (Windows; U; Windows NT 6.1; en-AU; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729) ID:20090729225027
please go to bug 487003. this bug is closed and won't be reopened.
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
this bug is not fixed: it's still broken in firefox 3.6. not only broken but consistently broken. for example, on my machine, the favicon for yahoo!mail is replaced with the last page visited until the next visit to yahoo!mail. this is the case for both windows & osx - it's not operating system dependent. this bug needs to be reopened & fixed properly.
(In reply to comment #109) > this bug needs to be reopened & fixed properly. no, goto bug 487003.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: