176987 - It is possible to open NC:BookmarksRoot group bookmark

Status: RESOLVED WORKSFORME

(Firefox :: Bookmarks & History, defect)

Description

[Jeff Thieleke]

It is possible to open up all of the root level bookmarks in tabs using the
middle mouse button.  Depending on the number of bookmarks, CPU speed, and
network speed, this can be rather disasterous...

I don't know why this is happening, and I can't provide exact step to reproduce,
other than randomly middle clicking on a root level bookmark will usually
demonstrate it sooner or later.  I typically see this bug occur about once a day.

Even if the bug isn't reproducible, Phoenix should never attempt to open
NC:BookmarkRoot as a group bookmark, so the forthcoming patch is still valid.

Comment 1

[Jeff Thieleke]

Attachment: Patch to prevent opening NC:BookmarksRoot in tabs and limit the total number of tabs opened

This patch addresses the NC:BookmarksRoot issue, and also adds a new preference
("browser.tabs.maxFolderLoadCount") so the user can limit the maximum number of
group bookmarks to open in one operation.  This seems like a good idea, since
an errant middle click on a folder containing many bookmarks could also be
disasterous.

Comment 2

[Asa Dotzler [:asa]]

Comment on attachment 104317 [details] [diff] [review]
Patch to prevent opening NC:BookmarksRoot in tabs and limit the total number of tabs opened

pch, can you take a look at this patch please? thanks.

Comment 3

[Pierre Chanial]

Comment on attachment 104317 [details] [diff] [review]
Patch to prevent opening NC:BookmarksRoot in tabs and limit the total number of tabs opened

Sorry for the delay. This patch is technically fine with me. But some thoughts:
- for note, tab extensions has another approach by limiting the number of total
tabs. Is it a better approach?
- wouldn't it better to popup a warning?
- how can you load the bookmark root, right now?

Comment 4

[Jeff Thieleke]

- for note, tab extensions has another approach by limiting the number of total
tabs. Is it a better approach?

I'm not really that familiar with the extension's feature.  I think the pref for
the standard browser is a reasonable way to prevent a user from accidently
loading a large number of tabs, though.


- wouldn't it better to popup a warning?

That sounds reasonable to me.  I don't really like adding even more modal
dialogs, but if there would be a "Never Ask Me Again" feature, that would be ok.


- how can you load the bookmark root, right now?

I'm not really sure, but I know I could reproduce it as I describe above. 
Before I applied this patch, I was seeing this bug at least once a day while
middle-clicking on a bookmark.

Comment 5

[Blake Ross]

Heh, this is really bad.  I middle-clicked on my bookmarks folder and opened 250
bookmarks in tabs and had to shutdown Firebird.  We have to fix this for 1.0.  
I think for 1.0 a great fix is to just make sure that middle click on the
bookmarks folder doesn't do anything.  The menuitem is already disabled.

I don't think we should add a "max tabs" limit right now as this patch does,
because then we'll just have broken UI in places.  I think the above is okay for
1.0.

Comment 6

[Mike Connor [:mconnor]]

taking QA contact, sorry about the bugspam

Comment 7

[Ben Goodger (use ben at mozilla dot org for email)]

-> 0.9

Comment 8

[Mike Connor [:mconnor]]

this is fixed now, or seems to be.  I can't replicate this anymore, and haven't
been able to for quite some time.  Resolving WORKSFORME, someone reopen if they
can still make bad things happen here.

Comment 9

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change