Closed Bug 1592976 Opened 5 years ago Closed 4 years ago

Favicons of redirecting pages may never be expired

Categories

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

defect
Points:
5

Tracking

()

RESOLVED FIXED
Firefox 75
Iteration:
75.2 - Feb 24 - Mar 8
Tracking Status
firefox75 --- fixed

People

(Reporter: mak, Assigned: mak)

References

Details

Attachments

(1 file)

While looking into another report of broken favicons, speaking with Gijs, I noticed how we may end up never removing icons for redirecting pages.

Suppose a redirecting page like https://docs.google.com/ was assigned a wrong favicon from an old and bogus Firefox version.
That page is unlikely to get another favicon load, because it just redirects, so it has not link rel.
Because we remove expired icons only when new icons arrive (to avoid leaving pages with no favicon), it's likely the bogus entries will live there forever.

As a first step we could probably remove all the icons older than a week for redirecting pages, if the page has a root favicon (domain/favicon.ico). We have a fallback in this case, so UI entries would not be left icon-less.
When? it's hard to tell, it could be part of weekly maintenance or expiration. Unfortunately we still have users that never see maintenance due to idle timing bugs and doing this as part of expiration could be excessive.
Provided we fixed the race conditions causing wrong favicons, running this once as a migration, and then on weekly maintenance, could be ok.

Long term we could evaluate different solutions, for example Gijs suggested marking the last stored icon for a domain as a "fallback" icon. That's also a possibility, that could even solve bug 323933.

Priority: P3 → P2

This sounds like a sensible expiration to do:

Favicons for redirecting pages:

SELECT page_url, icon_url FROM moz_icons i
JOIN moz_icons_to_pages ip ON icon_id = i.id
JOIN moz_pages_w_icons p ON page_id = p.id
JOIN moz_places h ON h.url_hash = page_url_hash
LEFT JOIN moz_bookmarks b ON b.fk = h.id
WHERE h.id IN(
  SELECT v.place_id
  FROM moz_historyvisits v
  JOIN moz_historyvisits v2 on v2.from_visit = v.id
  WHERE v2.visit_type IN(5,6)
)
AND b.id ISNULL

Favicons not updated in the last year for urls containing a ref:

SELECT page_url, icon_url, datetime(expire_ms / 1000, 'unixepoch') FROM moz_icons i
JOIN moz_icons_to_pages ip ON icon_id = i.id
JOIN moz_pages_w_icons p ON page_id = p.id
JOIN moz_places h ON h.url_hash = page_url_hash
LEFT JOIN moz_bookmarks b ON b.fk = h.id
WHERE expire_ms BETWEEN 1 AND strftime('%s','now','localtime','start of day','-365 days','utc') * 1000
AND b.id ISNULL AND page_url LIKE '%#%'

Overall, I think we should be safe removing icons that are expired more than 1 year ago and that fall into one of these categories (redirect source or url with ref) for urls that are not bookmarked.

Assignee: nobody → mak
Status: NEW → ASSIGNED

Expire favicons older than 6 months when:

  • they are for permanently redirecting urls, that are unlikely to receive
    updated favicons
  • they are for urls with refs (often mail, docs) that have a fallback root
    favicon for their origin

Expiration happens in chunks, mostly on idle-daily.

Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/02f45e7e36b0
Expire some favicons more aggressively. r=Standard8
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 75
Iteration: --- → 75.2 - Feb 24 - Mar 8
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: