Places tree view sometimes improperly restores multiple selection

NEW
Unassigned

Status

()

Firefox
Bookmarks & History
9 years ago
8 years ago

People

(Reporter: adw, Unassigned)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
See http://groups.google.com/group/mozilla.dev.apps.firefox/browse_thread/thread/4ae951db5e145593 for context.

STR (trunk required for these steps, but bug exists on branch, too):
1. Open the Places library window and type in a search string that returns more than one bookmark.
2. Select multiple bookmarks in the right content pane.
3. Tag the bookmarks all at once using the tag controls in the bottom of the right pane.

Actual results:
The bookmarks lose their selection after being tagged.

Expected results:
The bookmarks' selection should not be affected.

If you select just a single bookmark, it works as expected.  Only multiple selection is broken.  If you don't use search terms and just select a folder in step 1 instead, it works as expected with multiple selection.  I think this is because nsNavHistoryQueryResultNode::Refresh is not called in that case (see below).

The problem is in method PlacesTreeView.prototype._refreshVisibleSection, the "bookmark-nodes in queries case" path under the "restore selection" comment: the newElements array contains nodes, but the value of PlacesUtils.getConcreteItemId(newElements[j]) is undefined for every single j.  index is therefore always -1, so selection.rangedSelect(index, index, true) is never called.

With the STR above, the path to _refreshVisibleSection is something like this:
- nsNavHistoryQueryResultNode::OnItemAdded
- nsNavHistoryQueryResultNode::Refresh
- view's invalidateContainer
- _refreshVisibleSection

Comment 1

9 years ago
Seems to duplicate bug 279433 .
(Reporter)

Comment 2

9 years ago
I don't think so.  Just glancing at it, the problem with that bug looks like the contents of the tree are entirely changed, so from that (developer) perspective, it makes sense that the selection is lost.  But thanks for bringing it up.  It would definitely be useful to persist selection across searches.
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.