Some favicons disappear when I open Firefox

VERIFIED FIXED in Firefox 66

Status

()

defect
P1
normal
VERIFIED FIXED
6 months ago
5 months ago

People

(Reporter: trash0, Assigned: mak)

Tracking

({dataloss, regression})

63 Branch
mozilla67
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox65 wontfix, firefox66 verified, firefox67 verified)

Details

Attachments

(1 attachment)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

I open Firefox, all the bookmark favicons are displayed, I close Firefox.

I found that the option causing the issue is this one: Clear history when Firefox closes ---> Settings... BROWSING HISTORY AND DOWNLOAD HISTORY.

I have version 64 but it started with version 63. Other users discussed it here: http://forums.mozillazine.org/viewtopic.php?f=38&t=3042626

Actual results:

When I open Firefox some specific bookmark favicons disappear sometimes, they become the default globe instead and I need to click on the bookmark and open its website in order to make the favicon come back. Like I said it only happens with some bookmarks like this one: https://store.steampowered.com/explore/new/
It doesn't happen every time I open Firefox, but it seems to happen more easily if I have visited that bookmark last time.

Expected results:

The favicons should not disappear.

Component: Untriaged → Bookmarks & History

Would you comfortable with sharing your places.sqlite database with me, through a private mail to mak AT mozilla.com (you can use Firefox Send to create a self destructing download link)?
That file contains details about your history and bookmarks.

My suspect is that your database is somehow corrupt or not coherent.

Flags: needinfo?(trash0)

(In reply to Marco Bonardo [::mak] from comment #1)

Would you comfortable with sharing your places.sqlite database with me, through a private mail to mak AT mozilla.com (you can use Firefox Send to create a self destructing download link)?
That file contains details about your history and bookmarks.

My suspect is that your database is somehow corrupt or not coherent.

I already tried moving places.sqlite and favicons.sqlite so that new ones would be created. I also tried with a new profile with every setting entered manually except for the bookmarks (importing a JSON file). It was all good until I changed that option I wrote in the description (clear browsing and download history).

Sorry, I can't share private information.

Flags: needinfo?(trash0)

OK, I'll have a second look at our clearing methods, though icons for pages having a bookmark should not really be removed.

Also, do you ever use third party apps to clear Firefox information, like Crap Cleaner or similar?

Flags: needinfo?(mak77)

For the question in comment 3

Flags: needinfo?(mak77) → needinfo?(trash0)

(In reply to Marco Bonardo [::mak] from comment #3)

OK, I'll have a second look at our clearing methods, though icons for pages having a bookmark should not really be removed.

Also, do you ever use third party apps to clear Firefox information, like Crap Cleaner or similar?

I have CCleaner but the bug still happens even when I didn't use it recently (and the only selected option is "cookies"). It doesn't run automatically or in background.

My other settings:

  • third-party cookies are blocked and cookies are deleted when Firefox is closed (with a few exceptions);
  • always "Do Not Track";
  • history is deleted when Firefox is closed (except for cookies and site preferences);
  • the bookmarks are on the toolbar next to the back/forward/reload buttons (the bookmark bar is empty and hidden);
  • the only active extension is Adblock Plus.

The bug appears only when I select that single option. I'm going to try with a brand new profile with only that single option and nothing else and see if it happens.

Flags: needinfo?(trash0)

(In reply to trash0 from comment #5)

I have CCleaner but the bug still happens even when I didn't use it recently (and the only selected option is "cookies"). It doesn't run automatically or in background.

it's enough to use it once to corrupt the database for the foreseeable future, doesn't metter if it was used recently or long time ago.

I tried like I said and the bug happens, with a brand new profile with absolutely nothing changed besides these settings:

  • Use custom settings for history
    -- Do NOT remember browsing, download, search and form;
    -- Clear history when Firefox closes.

By doing this the specific guilty subsetting "Browsing history and download history" is selected by default.

(In reply to Marco Bonardo [::mak] from comment #6)

(In reply to trash0 from comment #5)

I have CCleaner but the bug still happens even when I didn't use it recently (and the only selected option is "cookies"). It doesn't run automatically or in background.

it's enough to use it once to corrupt the database for the foreseeable future, doesn't metter if it was used recently or long time ago.

A new profile doesn't solve this? Do I need to reinstall Firefox?

Sure, a new profile resolves it, as well as deleting the old databases, because it creates a new database. What I meant is that it can corrupt a database, if you replace the database with a new one, the problem obviously doesn't exist anymore.

Anyway, thanks for confirmation, I'll look into this.

Flags: needinfo?(mak77)

I checked the code again, it looks correct afaict. I also tried to reproduce the bug in a new profile by visiting and bookmarking the url suggested, but I never lost the bookmark favicon.
Unfortunately to proceed from here we need steps to reproduce in a new profile, because otherwise we can only blindly look at the code, that doesn't show any obvious flaws.

Flags: needinfo?(mak77)

The steps are:

  1. I create a new profile.

  2. In history settings:

  • I select "Use custom settings for history";
  • I uncheck "Remember my browsing and download history";
  • I uncheck "Remember search and form history";
  • I check "Clear history when Firefox closes" (the "Browsing and download history" subsetting should be checked by default).
  1. I enable the bookmarks bar (so that it appears below the toolbar).

  2. I add https://store.steampowered.com/explore/new/ to the bookmars bar and I click on it so that the favicon appears.

  3. I visit a few pages from that bookmark, for example all these ones:

  1. I close Firefox and I open it. The favicon is gone.

Even with these steps I cannot reproduce yet... Mark, could you please try?

Flags: needinfo?(standard8)

Yes, I can reproduce it with those steps, in 65 and in nightly.

Here's a little bit of info from running with mozStorage log and looking at the databases:

  • When the site is visited, we insert three lots of root=1 favicons for the site, here's the first one (minus the actual data):
    • INSERT INTO moz_icons (icon_url, fixed_icon_url_hash, width, root, expire_ms, data) VALUES ('https://store.steampowered.com/favicon.ico', hash(fixup_url('https://store.steampowered.com/favicon.ico')), 16, 1, 1549463106000, <data>)
  • After shutdown, moz_icons no longer has the previously stored root=1 favicons.
  • moz_origins does have store.steampowered.com within it (frecency = 0).

There are deletes happening on shutdown, but from a log inspection level I can't see that they would clear the favicons.

Flags: needinfo?(standard8) → needinfo?(mak77)
Status: UNCONFIRMED → NEW
Ever confirmed: true

Thanks, the log was extremely useful, I think I found the bug.

We initially set root = 1 on the icon, but then a call to ReplaceFaviconData executes an update (apparently a non useful one) and sets root to 0.
Optimizing writes may end up in a separate bug if it's too complex, for sure we can avoid overwriting root.

Assignee: nobody → mak77
Status: NEW → ASSIGNED
Flags: needinfo?(mak77)
Component: Bookmarks & History → Places
Priority: -- → P1
Product: Firefox → Toolkit
Duplicate of this bug: 1526810
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/c2784634bafa
Some bookmark favicons disappear after clearing history. r=Standard8
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Comment on attachment 9042485 [details]
Bug 1519058 - Some bookmark favicons disappear after clearing history. r=Standard8!

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

Bug 1468968

User impact if declined

Removing history can sometimes wrongly remove favicons for pages that are still referenced. This has been reported by multiple users, especially when they use Clear History on Shutdown (but it's not limited to that).

Is this code covered by automated tests?

Yes

Has the fix been verified in Nightly?

No

Needs manual test from QE?

Yes

If yes, steps to reproduce

The steps to reproduce are reported in the opening comment, Mark could reproduce the bug using those steps.

List of other uplifts needed

none

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

the risk is limited to the query clearing favicons, should not cause unexpected behavoiors to other kind of data

String changes made/needed

none

Attachment #9042485 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Whiteboard: [qa-triaged]

I have managed to reproduce this issue on an affected Firefox 66.0a1 (20190110214210) build using Windows 10 x64 by following the STR from comment 10.

This issue is verified fixed using Firefox 67.0a1 (20190213102848) on the following OSes: Windows 10 x64, Ubuntu 18.04 x64, Windows 7 x64, macOS 10.13.6.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Comment on attachment 9042485 [details]
Bug 1519058 - Some bookmark favicons disappear after clearing history. r=Standard8!

Fix for favicon bug, verified in Nightly.
OK for uplift for beta 8.

Attachment #9042485 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+

This issue is verified fixed using Firefox 66.0b8 on the following OSes: Windows 10 x64, Ubuntu 18.04 x64, Windows 7 x64, macOS 10.13.6.

QA Whiteboard: [qa-triaged]
Whiteboard: [qa-triaged]
You need to log in before you can comment on or make changes to this bug.