Closed Bug 1765311 Opened 3 years ago Closed 4 months ago

Bookmark favicon not displayed for urls having a root favicon behind a port

Categories

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

Product:
Firefox
Component:
Bookmarks & History
Platform:
x86_64
Windows 10
Type:
defect
Points:
5

Tracking

()

Status:
VERIFIED FIXED
Milestone:
133 Branch
Tracking Flags:
Tracking Status
firefox133 --- verified
firefox134 --- verified

People

(Reporter: bugsgalore, Assigned: yazan)

References

Details

(Keywords: nightly-community, Whiteboard: [favicons-2024][sng])

User Story

We can refactor the favicons code to pass around nsIURI instead of a url string, so it's easier to extract parts of the uri. This may be a pre-requisite bug.
We must check that root icon is appropriately stored and retrieved with the port, and add tests for both sides (storing and retrieving).
We should also check the page-icon protocol does the right thing.

Attachments

(1 file)

Bug 1765311 - Modify favicon fetching to account for urls containing a port. r?mak
48 bytes, text/x-phabricator-request
Details | Review

Go to http://websdr.ewi.utwente.nl:8901/

Bookmark the page into the bookmarks toolbar.

The favicon of the page is not displayed in the bookmark.

Keywords: nightly-community

The icon is at http://websdr.ewi.utwente.nl:8901/favicon.ico

I suspect the port is confusing the code that is likely looking for http://websdr.ewi.utwente.nl/favicon.ico that of course doesn't exist.

Severity: -- → S4
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Summary: Bookmark favicon not displayed for http://websdr.ewi.utwente.nl:8901/ → Bookmark favicon not displayed for urls having a root favicon behind a port

I imagine now that we can use nsIURI across threads it would be simpler to change https://searchfox.org/mozilla-central/source/toolkit/components/places/FaviconHelpers.cpp to actually pass nsIURI instead of url strings, that would simplify extracting host, port or prePath more easily instead of keeping around both spec and host (and potentially port)

I just wanted to note that what I think is the same bug can not only cause no favicon to be found, but indeed for the wrong favicon to be used.

Anytime you save a bookmark that uses a non-standard port, the favicon is either not found, or worse, if the domain also has a server running on a standard port, you will get that favicon as well.

E.g.,

If http://mydomain.com:8080 uses http://mydomain.com:8080/favicon.ico, but there is also http://mydomain.com/favicon.ico, the icon for the latter site will be used for the bookmark for the former site.

I find this most annoying if doing development on localhost, and I always get http://localhost/favicon.ico, even when I'm working on a web app being tested on http://localhost:[port]/ and it has a different favicon at http://localhost:[port[/favicon.ico.

Blocks: 1900261
Whiteboard: [favicons-2024]
Whiteboard: [favicons-2024] → [favicons-2024][sng]
Points: --- → 5
User Story: (updated)
Assignee: nobody → yalmacki
Depends on: 1923175
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f8e700c1102c Modify favicon fetching to account for urls containing a port. r=places-reviewers,Standard8

https://hg.mozilla.org/mozilla-central/rev/f8e700c1102c

Status: NEW → RESOLVED
Closed: 4 months ago
status-firefox133: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 133 Branch
QA Whiteboard: [qa-133b-p2]

Reproducible on a 2024-10-16 Nightly build on Windows 10.
Verified as fixed on Firefox 133.0b3 and Firefox Nightly 134.0a1 on Windows 10, Ubuntu 22, macOS 14.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-133b-p2]
status-firefox133: fixed → verified
status-firefox134: --- → verified
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: