search

mozilla-mobile / firefox-ios

Firefox for iOS
Mozilla Public License 2.0
12.25k stars 2.94k forks source link

[regression] Hacker News tab’s favicon changed from an orange Y to a red N #21725

Open cpeterso opened 2 months ago

cpeterso commented 2 months ago

Steps to reproduce

  1. In Firefox 9000 (44732), load Hacker News: https://news.ycombinator.com/
  2. Open the tab tray screen.

Expected behavior

The Hacker News tab’s favicon should be an orange Y.

image

Actual behavior

The Hacker News tab’s favicon is a red N.

image

Device & build information

Is this regression caused by favicon refactor https://github.com/mozilla-mobile/firefox-ios/pull/21608?

┆Issue is synchronized with this Jira Task

data-sync-user commented 2 months ago

➤ ih-codes commented:

Update:

As per Issam Mani ‘s research ( https://mozilla.slack.com/archives/C05C9RET70F/p1725289491118429?thread_ts=1725126733.939879&cid=C05C9RET70F ), this is actually due to the fact Kingfisher does not support fetching and caching SVGs. Which means this is technically a new feature we need to add. The reason we didn’t see this before is because the Hacker News website had a bundled asset previously, so we weren’t fetching it from the website.

I double checked that we scrape the correct URL from the JavaScript parser if a user visits this page.

I will also double check if our URL fetcher code can get the correct URL too (https://news.ycombinator.com/y18.svg).

data-sync-user commented 2 months ago

➤ ih-codes commented:

Related feature request: https://mozilla-hub.atlassian.net/browse/FXIOS-9807 ( https://mozilla-hub.atlassian.net/browse/FXIOS-9807|smart-link )

data-sync-user commented 2 months ago

➤ ih-codes commented:

QA Notes:

data-sync-user commented 2 months ago

➤ Diana Andreea Barladeanu commented:

Verified as fixed on v131 (45403), with iPhone 15 (17.5).

!ImportedPhoto.748344183.07096.jpeg|width=1179,height=2556,alt="ImportedPhoto.748344183.07096.jpeg"!