Open Bug 1785789 Opened 3 years ago Updated 7 months ago

When importing bookmarks many windows are opened in Firefox Snap and Firefox Flatpak - fake-favicon-uri

Categories

(Toolkit :: Places, defect, P3)

Firefox 125
x86_64
Linux
defect
Points:
3

Tracking

()

People

(Reporter: o2q2tcedsh0, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [sng][favicons-2024])

User Story

Investigate why internal protocols like fake-favicon-uri: are being requested and by which code.
Add MOZ_ASSERT to ensure uses are caught in tests.
No code should ever need to load them.

Attachments

(5 files)

Attached file Logs.txt

Steps to reproduce:

I exported the bookmarks from a profile in .json format. Then imported this in a whole new profile.
I tried it with Firefox 103.0.2 Flatpak and Firefox Snap( 104 RC, ESR 91.12.0esr-1). With Ubuntu 22.04 and Debian Stable.

If i delete the favicons.sqlite before exporting the bookmarks then this behavior dos not occur in the new profil when importing.

Actual results:

I have been using Firefox natively with Wayland with MOZ_ENABLE_WAYLAND=1.

Many windows are opened when importing.
In the windows is written:
"Open with..." and No Apps available
No app installed that can open "...ttps://www." You can finde more application in Softwae
Find More in Software

Under Settings(applications) appears now:
fake-favicon-uri - Open with systemhandler

In the logs, the following is one hundred and three times.
xdg-desktop-por: Failed to associate portal window with parent window
xdg-desktop-por: Unhandled parent window type

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Attached image 1.png
Attached image 2.jpg
Attached image 3.jpg

I have looked at the bookmarks.json with Libreoffice, because Gnome gedit has some problems with the file.
I have some of these "fake-favicon-uri" in the exported bookmarks. The problem with the many windows in my case does not occur if the favicons are deleted before exporting the bookmarks and then no "fake-favicon-uri" is present.

Deleted favicons.sqlite before exporting the bookmarks.json

{"guid":"iWaNHn1IiMiY","title":"o2 | Mobilfunknetz, Handytarife, Top-Smartphones & VDSL Internet","index":577,"dateAdded":1568024390000000,"lastModified":1612099071000000,"id":9992,"typeCode":1,"type":"text/x-moz-place","uri":"https://www.o2online.de/"}

With favicons.sqlite present and then exported the bookmarks.json

{"guid":"iWaNHn1IiMiY","title":"o2 | Mobilfunknetz, Handytarife, Top-Smartphones & VDSL Internet","index":577,"dateAdded":1568024390000000,"lastModified":1612099071000000,"id":9992,"typeCode":1,"iconUri":"fake-favicon-uri:https://www.o2online.de/","type":"text/x-moz-place","uri":"https://www.o2online.de/"}

Version: Firefox 103 → Firefox 104
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Version: Firefox 104 → Firefox 106
Summary: When importing bookmarks many windows are opened in Firefox Snap and Firefox Flatpak → When importing bookmarks many windows are opened in Firefox Snap and Firefox Flatpak - fake-favicon-uri
Version: Firefox 106 → Firefox 108

Today I imported bookmarks in Ubuntu 24.04 Firefox Snap again and the problem reappeared in a similar way, but it were new bookmarks.

about:memory
4,599 (100.0%) -- event-counts
└──4,599 (100.0%) -- window-objects
├──2,768 (60.19%) -- top(chrome://browser/content/browser.xhtml, id=1)/active
│ ├──2,766 (60.14%) -- window(chrome://browser/content/browser.xhtml)/dom
│ │ ├──2,757 (59.95%) ── event-listeners
│ │ └──────9 (00.20%) ── event-targets
│ └──────2 (00.04%) ── window(about:blank)/dom/event-targets
├────529 (11.50%) -- top(chrome://browser/content/places/places.xhtml, id=37)/active/window(chrome://browser/content/places/places.xhtml)/dom
│ ├──520 (11.31%) ── event-listeners
│ └────9 (00.20%) ── event-targets
├────148 (03.22%) ++ (5 tiny)
├─────76 (01.65%) -- top(none)/detached
│ ├──66 (01.44%) ── window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom/event-listeners [2]
│ └──10 (00.22%) ++ (2 tiny)
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=41)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=43)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=45)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=47)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=49)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=51)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=53)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=55)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
│ ├──46 (01.00%) ── event-listeners
│ └───3 (00.07%) ── event-targets
├─────49 (01.07%) -- top(chrome://mozapps/content/handling/permissionDialog.xhtml, id=57)/active/window(chrome://mozapps/content/handling/permissionDialog.xhtml)/dom
... and so on.

Attached image fake-uri-icons.jpg
Version: Firefox 108 → Firefox 125
Blocks: snap
Priority: -- → P3

You are probably getting this because fake-favicon-uri: is not a correct protocol handler somehow ?

Component: Widget: Gtk → Places
Product: Core → Toolkit

The severity field is not set for this bug.
:mak, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(mak)

First problem: I don't see why we'd try to open those uris, importing bookmarks is not supposed to load these icons, it should just store them. We must understand what is doing it, S3 until that is clarified, as this issue is not commonly reported.

Second problem:nsIOService::UsesExternalProtocolHandler has some special path to have place:, fake-favicon-uri: handled by external protocol hander at https://searchfox.org/mozilla-central/rev/4c8627a76e2e0a9b49c2b673424da478e08715ad/netwerk/base/nsIOService.cpp#907-913 but these protocols are only for internal use. That was implemented in Bug 1447341, though I don't see why we return true instead of false, nothing is going to open these protocols as they are fake anyway.
Gijs, do you remember anything about that decision?

Severity: -- → S3
Flags: needinfo?(mak) → needinfo?(gijskruitbosch+bugs)

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

Second problem:nsIOService::UsesExternalProtocolHandler has some special path to have place:, fake-favicon-uri: handled by external protocol hander at https://searchfox.org/mozilla-central/rev/4c8627a76e2e0a9b49c2b673424da478e08715ad/netwerk/base/nsIOService.cpp#907-913 but these protocols are only for internal use. That was implemented in Bug 1447341, though I don't see why we return true instead of false, nothing is going to open these protocols as they are fake anyway.
Gijs, do you remember anything about that decision?

I don't remember, and unfortunately there doesn't appear to be much context for it in the bug either. It doesn't look like that would have matched the status quo before the bug?

... however, by returning false, we would have entered this block, which would have tried to create the relevant protocol handler. Those don't exist, so we'd never cache them, and we'd keep re-entering that code, and then falling back to the default external handler below anyway.

So I imagine that it was a performance optimization to just go straight to using the default protocol handler. And even now, it looks like we'd hit this code instead, and I can't imagine that would be able to find a protocol handler for these schemes, either - so presumably the outcome would be the same anyway?

If we're still hitting this I'm kind of curious why we're hitting this for these handlers, if indeed we never load them: for then, why would we need the protocol handler at all?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(mak)

(In reply to :Gijs (he/him) from comment #11)

If we're still hitting this I'm kind of curious why we're hitting this for these handlers, if indeed we never load them: for then, why would we need the protocol handler at all?

We should NEVER hit these, as we should never need to load these protocols, the fact something is asking about the protocol is the bug to investigate here.

Though at that point we should probably add some MOZ_ASSERT to ensure we're not adding new accesses to these internal protocols. And my question is whether we should have an hard bailout where ::LookupProtocolHandler could possibly return a nullptr, so we don't even try the default handler.

Status: UNCONFIRMED → NEW
User Story: (updated)
Ever confirmed: true
Flags: needinfo?(mak)
Whiteboard: [sng]

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

Though at that point we should probably add some MOZ_ASSERT to ensure we're not adding new accesses to these internal protocols. And my question is whether we should have an hard bailout where ::LookupProtocolHandler could possibly return a nullptr, so we don't even try the default handler.

That sounds like a good idea to me - we'd have to audit all the existing consumers of course, to make sure they would be OK with this...

Whiteboard: [sng] → [sng][favicons-2024]
Points: --- → 3
Depends on: 1910324
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: