Open Bug 1301235 Opened 8 years ago Updated 2 years ago

Bookmark UI operations may hang the browser when there are cycles in folder shortcuts

Categories

(Toolkit :: Places, defect, P3)

defect

Tracking

()

People

(Reporter: tcsc, Unassigned)

References

Details

This is fairly obscure, but if you create a query cycle in your bookmarks, its very easy to cause a hang due to infinite recursion in nsNavHistoryContainerResultNode::GetSortType.

To do this, make a fresh profile, go to the "show all bookmarks" panel, and drag "All Bookmarks" on the sidebar onto the bookmarks toolbar folder. After doing this, trying to move these any further will cause the browser to hang. (let me know if this is unclear)

I'd accept that it's probably a bug that this is allowed in the first place, but it seems a little bit hard to prevent -- the steps above aren't the only way to do it.
I suspect it's not the only method that can get mad with recursion...
Priority: -- → P3
That's probably true, I didn't test for very long, this is just the first one I hit.
Mak,
  It wasn't clear to me on reading this that it's quite trivial for the user to perform an operation using the bookmarks UI that causes cycles and thus can cause the browser to hang. As such, it seems preventing the user from causing these cycles in the first place should be a higher priority than P3 - what do you think?
Flags: needinfo?(mak77)
it's a problem that exists from a long time, we try to prevent the most common actions already in the UI, and I have seen very rare reports.
Based on our current usage of priorities and resources, it's unlikely this can be a P2 (fix for next version) or P1 (fix for this version). P3 is the backlog and I can't predict an ETA on a fix.

that said, this is non-trivial, cause there is a hierarchy and doing an ancestor check on every bookmark change would be extremely expensive. The best we can do is UI checks, and those will always have some imperfection. Another alternative could be to change bookmarks table to an MPTT, but that's quite a challenge.
Flags: needinfo?(mak77)
(In reply to Marco Bonardo [::mak] from comment #4)
> it's a problem that exists from a long time

Thanks - I was checking you were aware it was available via the UI and not just theoretical via the API. I appreciate the constraints you mention.
Summary: nsNavHistoryContainerResultNode::GetSortType doesn't handle cycles in bookmarks → Bookmark UI operations may hang the browser when there are cycles in folder shortcuts
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.