1903721 - Bookmarks import from file throws a browser console error

Status: RESOLVED FIXED

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

Description

[Cristian Baica [:cbaica], Release Desktop QA]

Attachment: bookmarks-causing-error.html

Found in

• Fx 128.0b5

Affected versions

• Fx129.0a1
• Fx128.0b5
• Fx127.0.1RC

Affected platforms

• all

Steps to reproduce

1. Launch Firefox and bookmark several pages (e.g. youtube, reddit, facebook, twitter, 9gag).
2. Go to the library and export the bookmarks to an html file.
3. Close Firefox.
4. Open Firefox with a clean(new) profile.
5. Open the browser console(CTRL+SHIFT+J) and go to the library window.
6. Import the bookmarks exported in step 2.

Expected result

• No errors are thrown in the browser console.

Actual result

• The following error is displayed in the browser console two times:

Failed to import favicon data: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFaviconService.setFaviconForPage]
    insertFaviconForNode resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1085
    insertFaviconsForTree resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1124
    insertFaviconsForTree resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1128
    insertFaviconsForTree resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1128
    _importBookmarks resource://gre/modules/BookmarkHTMLUtils.sys.mjs:829
BookmarkHTMLUtils.sys.mjs:1092:15
    insertFaviconForNode resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1092
    insertFaviconsForTree resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1124
    insertFaviconsForTree resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1128
    insertFaviconsForTree resource://gre/modules/BookmarkHTMLUtils.sys.mjs:1128
    _importBookmarks resource://gre/modules/BookmarkHTMLUtils.sys.mjs:829

Regression range

• Doesn't look like a recent regression as I could reproduce the issue with a slightly different error format, but still reffering to favicons all the way back to Firefox 91.

Additional notes

• Issue is NOT reproducible with certain bookmark files. Uncertain if the actual bookmarked websites are causing the issue or this is something general. See attachments.
• Bookmarks are successfully imported and functional (hence the low severity).

Comment 1

[Cristian Baica [:cbaica], Release Desktop QA]

Attachment: bookmarks-noerror.html

Comment 2

[Marco Bonardo [:mak]]

This is fine, not all the favicons can be imported and when we can't be set we dump to the console.
The problem seems to be related to the "fake-favicon-uri" entries, but I'm not sure why. We must run this with a debugger, or in a debug build where the system console would likely contain a pseudo-stack for that NS_ERROR_FAILURE.. Maybe it's 1899218, or something else.

Comment 3

[Yazan Al Macki [:yazan]]

The issue is caused by data URIs which specify a type different than the actual data encoded.
One of the data URIs in the example given was data:image/png;base64,PHN2ZyB4... where the encoded data was actually that of an SVG file rather than PNG.
The channel getContentType method relies on the type specified at the start of the URI, and so this incorrect mime type is never corrected. A workaround would be to use imgLoader::GetMimeTypeFromContent to ensure correct type when setting favicons using data URIs.

Comment 4

[Yazan Al Macki [:yazan]]

Attachment: Bug 1903721 - Use imgLoader::GetMimeTypeFromContent in setFaviconForPage when dealing with data URIs to ensure correct mime type is passed. r?mak

Comment 5

[Yazan Al Macki [:yazan]]

Attachment: Bug 1903721 - Use imgLoader::GetMimeTypeFromContent in setFaviconForPage when dealing with data URIs to ensure correct mime type is passed. r?mak,standard8

Comment 6

[Yazan Al Macki [:yazan]]

Attachment: Bug 1903721 - Use imgLoader::GetMimeTypeFromContent in setFaviconForPage when dealing with data URIs to ensure correct mime type is passed. r?mak

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8642cccd19d7
Use imgLoader::GetMimeTypeFromContent in setFaviconForPage when dealing with data URIs to ensure correct mime type is passed. r=mak,places-reviewers

Comment 8

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/8642cccd19d7

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:yazan, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox130 to wontfix.

For more information, please visit BugBot documentation.