Closed Bug 2002102 Opened 11 months ago Closed 13 days ago

Limit the number of bookmarks shown in the macOS main menu to prevent non-permanent hangs

Categories

(Firefox :: Bookmarks & History, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
161 Branch
Tracking Status
relnote-firefox --- 161+
firefox161 --- fixed

People

(Reporter: spohl, Assigned: spohl)

References

Details

(Keywords: blocked-ux)

Attachments

(1 external file)

Starting in macOS Tahoe, macOS is indexing the menu items in the main menu for Spotlight. While we have fixed the case of permanent hangs due to cycles in the bookmarks structure (see bug 1979283), non-permanent hangs due to thousands (or even tens of thousands) of bookmarks can still happen (see bug 1979283 comment 63). While discussing this with the team, Nika suggested that we limit the number of bookmarks that we add to the main menu and instead have a menu item along the lines of "Open Bookmark Manager for more bookmarks...". The macOS Spotlight team thought that this was worth exploring. Marco, what do you think?

This sounds like a question for Product/UX, rather than engineering.
My personal opinion is that bookmarks in menus make zero sense, apart from summary menus like Recent Bookmarks. They are better served by the sidebar.

I think we have 2 paths here:

  1. the bookmarks menu becomes just a copy of the Bookmarks section of the hamburger menu (basically Recent Bookmarks), users interested in a full fledged bookmarks menu can add the bookmarks button to their toolbar.
  2. we limit the number of bookmarks when it's very large, and forward the rest to the Sidebar (that feels closer to their original intent than the Library window), as suggested. Though it feels like this would only work with a flat list of bookmarks, we do have a hierarchy, so I could have 80k bookmarks spread into folders so that each level is small, but the overall tree is huge. And it's not trivial to know which paths bring to many bookmarks... what would we do in this case? is MacOS only indexing the first level?

I suspect solution 1) is the best to fix the problem long term, but it definitely needs frontend Product/UX approval.

It may also be sensible to speak with a11y to ensure the bookmarks remain accessible even after any change made here, as they are likely a major consumer of this menu.

I am not advocating for keeping the bookmarks in the main menu, but in order to make an informed decision I wanted to point out that one "feature" that we would be losing if we removed them is that the bookmarks would no longer get indexed or show up in Spotlight search results.

True, but I still don't see how to limit a tree hierarchy in a clean and understandable way. Also bookmarks cut-off by the limit would not be indexed, but their position in the hierarchy is not representative of their importance.

We could eval Recently Visited Bookmarks instead of Recent Bookmarks (unless in permanent pb mode where we can only do Recent Bookmarks), then some meaningful bookmarks would still be indexed.

Keywords: blocked-ux
Duplicate of this bug: 2005154

Looking at bug 2005154, we could artificially limit queries returning flat lists (so not a structure of bookmarks) to a reasonable amount, such that one would get bored of scrolling a menu anyway. Something around 3000?
That would prevent cases where the user wrongly copied over large queries, like full history, while not affecting normal bookmark hierarchies.
A user with an abnormal number of actual bookmarks (tens of thousands) may still hit this bug, of course.

To do that, the menu constructor may easily add to the query a maxResults param if the place uri doesn't represent a folder or a folder shortcut.
We'd not be able to detect "overflow" this way for a "show more" entry. If we need that, then it requires an additional disk access.

Using Spotlight on MacOS Tahoe reliably hangs Firefox 147.0b9 dev edition for me. My wife seems to have similar problems with latest release version of Firefox. This is very disruptive. Is this the bug to follow? I found another but it was closed.

(In reply to tofu from comment #6)

Using Spotlight on MacOS Tahoe reliably hangs Firefox 147.0b9 dev edition for me. My wife seems to have similar problems with latest release version of Firefox. This is very disruptive. Is this the bug to follow? I found another but it was closed.

It depends, do you have a bookmark folder/query containing many thousands entries in the native menubar bookmarks menu?
O maybe, like in Bug 2005154, you may have added one of the history folders to bookmarks?
Checking your bookmarks structure for obvious cases of many entries in a single query would help figuring it out, and then in most cases those "links" can be removed safely, we can help with figuring out what those are and whether they are safe to remove.

My idea from comment 5 is probably still the best we can do short term.

Flags: needinfo?(tofu)

Indeed my bookmarks history was in my "Other bookmarks" folder (though I don't think I've ever touched that). I've deleted it and will see if that helps, thanks! I have about 950 "Other" bookmarks FWIW. It's strange that multiple people have this erroneous history bookmark.

Flags: needinfo?(tofu)

Confirmed, removing "History" from "Other bookmarks" made Firefox usable again, thanks!

Is there a way to distinguish when a menu is being inspected by the OS, rather than the user? In that case we could refuse to open nodes representing queries.

Flags: needinfo?(spohl.mozilla.bugs)

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

Is there a way to distinguish when a menu is being inspected by the OS, rather than the user? In that case we could refuse to open nodes representing queries.

I don't believe that there is, unfortunately.

Flags: needinfo?(spohl.mozilla.bugs)

Before 149.0 launches, I'll point out that this bug has been somewhat improved via the 148.0 release. Many users on Reddit stated that 148.0 sped up the platform in general, so I presume that whatever it was that changed things affected this bug (in a good way) as well.

To be clear, most of the problems (which I thoroughly described in another Bugzilla report here) still persist, the one thing that's changed being that rather than the browser hanging for two minutes when starting up and/or when hitting command-space bar, it hangs now for "only" 30 seconds or so. While that's still bad, and while multi-tasking between Firefox and other applications is still painful (especially when doing a lot of continuous back and forth, as it hangs for about 5 seconds each way), the improvement is just enough to stop me from exploring migrating to another browser (which I'd started just before 148.0 was released).

That all being so, and if someone would be willing to clarify, is there effectively nothing that can be done on Firefox's end, and the only recourse is to hope that Apple one day addresses the issue on their end? If it's the former, is there such a thing as bug bounties whereby we can fund somebody to tackle a bug? Cause for what I presume to be a tiny, tiny minority, Firefox is really painful to use in this state. For all I know I might even be the only one willing to fund such a thing, but I'd at least be willing to consider the prospect supposing it even were one.

I can't clearly tell if the remaining slowdowns are still the same issue, as the fixes that landed either solved it, or left it as it was. I'd expect the 2 minutes to persist or disappear, not become 30s.

We can still do what I suggested in comment 5, it would be a stop-gap solution for the bookmarks problem, if that is still the problem.

Do you have a really large bookmarks folder containing thousands and thousands of entries? Basically the same question as in comment 7.

Flags: needinfo?(allan)

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

Do you have a really large bookmarks folder containing thousands and thousands of entries? Basically the same question as in comment 7.

Confirmed. I've got over 20,000 bookmarks (accumulated over many years), and tried deleting the one folder holding the majority of them. That got it down to 4,200, but when I re-tested things the problems persisted unchanged (the 30-second hang with spinning beach ball when activating search via hitting command-space bar, as well as the multi-second lag when multi-tasking back and forth between Firefox and other apps).

I deleted a few more folders and got the collection down to ~1,900 bookmarks, and while the multi-second lag when multi-tasking seemed to be gone, the 30-second command-space bar hang still existed but had dropped substantially down to two seconds. I deleted some more, got it down to ~1,200 bookmarks, and the command-space bar hang seemed to have disappeared entirely.

I'm not sure if your suggestion from comment 5 about "artificially limit[ing] queries" to "Something around 3000" correlates to 3000 bookmarks, but if it does, and supposing this would be a stop-gap solution (as you suggest) that would work, then perhaps 1,000 would be a better idea?

Flags: needinfo?(allan)

I inadvertently came across a partial workaround for this, which may or may not be useful for the bug-fixing end.

When I tested out the removal of varying amounts of bookmarks yesterday the browser froze when I tried command-Z to bring all the folders back. I had of course backed up all my bookmarks prior to the tests, so I restarted the browser, deleted all remaining bookmarks, then imported the entire lot. Afterwards, the multi-second lag when multi-tasking was gone. However, upon testing out the command-space bar search functionality a few hours later the spinning beach ball hang again lasted for about 30 seconds or so, and to make matters worse the multi-second lag when multi-tasking returned.

When I loaded up the browser this morning all previous problems were present. I tried repeating the delete-all-bookmarks followed by import-all-bookmarks process, and lo and behold I got the same positive effect as yesterday of no multi-second lag when multi-tasking. It's a crude workaround for me but, as I literally never use the search functionality, from now on I'm gonna be exporting then deleting then importing all bookmarks upon every browser restart every morning. Yes, it'll be a bit of a PITA, but I'd much rather do that than move to a different browser or feel like I'm running modern software on a computer from 2005.

That's not an acceptable workaround, we would certainly like to address this, so you don't have to do it.
My suggestion to comment 5 would artificially limit certain type of queries, but if you have 20000 bookmarks in a folder it wouldn't help as a folder is not a query.

Could you please capture a performance profile while the slowdown issue happens? So we can check we're still looking at the same issue.
There's instructions on https://profiler.firefox.com/ you may also take one profile when searching, and one when switching app, if you wish.

Flags: needinfo?(allan)
Duplicate of this bug: 2067306

Quick update: Bug 2067306 turned out to be another case of this, with the same three queries in "Other Bookmarks" as bug 2005154: place:type=3&sort=4 (history by date), place:transition=7&sort=4 (downloads) and place:type=6&sort=1 (tags). Deleting them took Spotlight's requests to Firefox from 2.5 to 5 s down to 70 to 80 ms in the same process (bug 2067306 comment 15). That's three reports where people found these Library queries in "Other Bookmarks" without remembering putting them there.

The reporter's logs and profiles (bug 2067306 comment 9) also show more of the mechanism:

  1. The trigger is Spotlight. When it connects, intelligencecontextd sends Firefox a request with a 0.75 s timeout and UIIntelligenceSupport walks the whole menubar with -[NSMenu _simulateOpening:]. Firefox needed about 2.5 s per pass, every request timed out, and each timeout was followed by a retry about 1.8 s later. So every Spotlight session cost two full passes.
  2. The cost doesn't build up over time. A profile 3.6 minutes after startup and one after 3 hours look the same.
  3. Every pass rebuilds the query subtrees from scratch. PlacesMenu._onPopupHidden sets containerOpen = false on anything that isn't a folder or folder shortcut, so the next popupshowing reopens the query and _rebuildPopup recreates every item. In the profiles, the date containers under the history query are new DOM nodes on every pass, each new item becomes a new native menu item with a fresh favicon load, and one hang window had about 14,000 image decodes. Accessibility was active in the parent process as well, which made removing the old items noticeably more expensive.

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

That's not an acceptable workaround, we would certainly like to address this, so you don't have to do it.
My suggestion to comment 5 would artificially limit certain type of queries, but if you have 20000 bookmarks in a folder it wouldn't help as a folder is not a query.

Could you please capture a performance profile while the slowdown issue happens? So we can check we're still looking at the same issue.

Sorry for having not gotten back to you about this, which isn't to say the problem has disappeared. I can at least say that the workaround has improved.

My initial workaround, mentioned in my previous message, was to implement a "delete-all-bookmarks followed by import-all-bookmarks process" each time I used Firefox. Well, after I'd done that a couple of times or so something seems to have gotten "reset" (for lack of a better explanation) as I no longer have to go through that process. Instead, upon starting up my Mac and opening up Firefox for the first time, 99.9% of the time it starts with the freeze/lag. After the freeze/lag has completed I then quit Firefox, start it up again, and it's freeze/lag free. At some point during the day (sometimes even twice), for which I can't identify a specific cause or pattern, the freeze/lag will occur again. I again repeat the quit Firefox / start up Firefox process, and the problem is once again (temporarily) gone. Otherwise, opening up Spotlight with command-spacebar still enacts the bug.

Anyway, while I've been rather slack to respond with a performance profile, I was hoping that the update to macOS Golden Gate 27 would fix things. It did not.

Otherwise, now that the report of this bug has resurfaced I'll get to creating that performance profile in the next few days. I just need to clean up an excess of open tabs I've got in tab groups (and move them to bookmarks -- yikes) and find a good time when I can do the profiling.

Thanks for not closing the bug report.

Flags: needinfo?(allan)

(In reply to Stephen A Pohl [:spohl] from comment #18)

That's three reports where people found these Library queries in "Other Bookmarks" without remembering putting them there.

Unfortunately it's easy to do a mis-dragndrop. Another reason may be maintenance detecting a broken database and trying to save things, but it's less likely.

  1. Every pass rebuilds the query subtrees from scratch. PlacesMenu._onPopupHidden sets containerOpen = false on anything that isn't a folder or folder shortcut, so the next popupshowing reopens the query and _rebuildPopup recreates every item. In the profiles, the date containers under the history query are new DOM nodes on every pass, each new item becomes a new native menu item with a fresh favicon load, and one hang window had about 14,000 image decodes. Accessibility was active in the parent process as well, which made removing the old items noticeably more expensive.

If anyone is willing to do a vibe coded patch implementing comment 5 I'm happy to review it. I don't know what the limit should be, but it makes sense to have some limit, and maybe when we limit we could add a final more... menuitem that opens the appropriate sidebar. We could also make so that it only happens when the view is in the native menubar.

Thanks, looking into it...

Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED

Another data point on macOS 27.0.1 (26A434), Firefox 157.0.1:
same two queries in "Other Bookmarks" (place:type=3&sort=4 and
place:transition=7&sort=4), not knowingly added. Only ~290 bookmarks
total. Each Spotlight open/close hung the parent main thread for
several seconds (~328 popupshowing in a 34s profile, mostly
nsMenuItemIconX favicon rebuilds). Deleting the two queries fixed it.
The same profile also had 4 duplicate "Recent Tags" queries in
different locales (zh-TW / ja), likely merged via Sync.

Duplicate of this bug: 2080066
Pushed by spohl@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/6f77b3edb2b0 https://hg.mozilla.org/integration/autoland/rev/ff516b4b7395 Add a configurable result limit to native macOS Places query menus. r=mak,fluent-reviewers,places-reviewers,bolsson

Is it possible to test out this commit in a release, be it now or in the imminent future? Such as Beta or Nightly? I ask because I'm not exactly sure how to decipher which release that commit has gone out to, if any at all.

It will be available in the next Nightly, roughly 12-24h after the patch landed in comment 25.

Status: ASSIGNED → RESOLVED
Closed: 13 days ago
Resolution: --- → FIXED
Target Milestone: --- → 161 Branch

Did you want to nominate this for the Fx161 relnotes? If so, set the relnote-firefox flag to "?"
https://wiki.mozilla.org/Release_Management/Release_Notes_Nomination

Possible wording:

Fixed Firefox hanging on macOS 26 (Tahoe) when opening Spotlight with a large number of bookmarks.

Thanks

Flags: needinfo?(spohl.mozilla.bugs)

Release Note Request (optional, but appreciated)
[Why is this notable]: This prevents a possible hang on macOS 26+.
[Affects Firefox for Android]: no
[Suggested wording]: Fixed Firefox hanging on macOS 26+ when opening Spotlight with a large number of bookmarks.
[Links (documentation, blog post, etc)]: n/a

relnote-firefox: --- → ?
Flags: needinfo?(spohl.mozilla.bugs)

Added to nightly notes, thanks.

Blocks: 2086594
QA Whiteboard: [qa-triage-done-c162/b161]
See Also: → 2091501
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: