1995696 - Don't populate Places menus if they create a recursive structure

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, task, P1)

Description

[Marco Bonardo [:mak]]

To avoid issues with software that navigates through menus (e.g. accessiblity or system search) it would be better to not have infinite hierarchies, that folder shortcuts can easily create. The user may inadvertently create these shortcuts with drag and drop.
In Bug 1631239 we'll introduce a maintenance task to remove recursive structures, but we can start avoiding to populate menus when they end up creating this infinite hierarchies.
Bug 1979283 will also implement a limit on the widget side.

Comment 1

[Marco Bonardo [:mak]]

Attachment: Bug 1995696 - Don't populate Places menus if they create a recursive structure

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9cbe62581b7f
https://hg.mozilla.org/integration/autoland/rev/acef1c203ca9
Don't populate Places menus if they create a recursive structure r=places-reviewers,Standard8

Comment 3

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: Infinite bookmarks hierarchies in menus may cause third party software scanning menus loop indefinitely
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: Can create a bookmark with url "place:parent=toolbar_____" on the toolbar, or in a subfolder on the toolbar... Or similarly, "place:parent=menu________" in the bookmarks menu. Instead of creating an infinite loop it will instead show an (empty) panel with the patch.
• Risk associated with taking this patch: low
• Explanation of risk level: The patch concept is simple, walking up parentNodes and checking bookmark GUIDs.
• String changes made/needed: no
• Is Android affected?: no

Comment 4

[Marco Bonardo [:mak]]

Attachment: Bug 1995696 - Don't populate Places menus if they create a recursive structure

Original Revision: https://phabricator.services.mozilla.com/D269599

Comment 5

[Norisz Fay [:noriszfay]]

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

Comment 6

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/9ef460791a44
https://hg.mozilla.org/releases/mozilla-beta/rev/6371ba17ede2

Comment 7

[Peter Magyari (Desktop QA)]

Hi, I have reproduced the issue using Beta 145.0b5 (20251022093404) and attempted to verify the fix using Nightly 146.0a1 (20251024094538).
Creating a bookmark on the toolbar or a subfolder on the toolbar with the URL "place:parent=toolbar_____" no longer creates an infinite loop of bookmarks, however, creating a bookmark with the URL "place:parent=menu________" in the bookmarks menu still does.

SS: https://imgur.com/PdXQTvZ

Comment 8

[Marco Bonardo [:mak]]

(In reply to Peter Magyari (Desktop QA) from comment #7)

however, creating a bookmark with the URL "place:parent=menu________" in the bookmarks menu still does.

That looks like the Sidebar, this fix is only for menu views. (e.g. the native menubar on MacOS, the menubar visible on Windows when pressing ALT)

Comment 9

[Peter Magyari (Desktop QA)]

You are right, I have also verified it using Beta 145.0b7 (20251027114755) on Windows 10, MacOS 15 and Ubuntu 24.04, I'm setting the flags accordingly. Thanks!

Comment 10

[Ryan VanderMeulen [:RyanVM]]

We probably want this on ESR140 also?

Comment 11

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: Possible hang if a third party software walks the application native menus
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: yes
• Steps to reproduce for manual QE testing: See previous requests
• Risk associated with taking this patch: low
• Explanation of risk level: the patch is pretty simple and no issues reported so far.
• String changes made/needed: no
• Is Android affected?: no

Comment 12

[Marco Bonardo [:mak]]

Attachment: Bug 1995696 - Don't populate Places menus if they create a recursive structure

Original Revision: https://phabricator.services.mozilla.com/D269599

Comment 13

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/b867a9aa764c

Comment 14

[Peter Magyari (Desktop QA)]

Verified fixed on 140.5.0esr (20251103105753)