503636 - Library details pane not updated when selecting two nodes in left pane with same URI

Status: RESOLVED INCOMPLETE

(Firefox :: Bookmarks & History, defect)

Description

[Alice0775 White]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090708 Firefox/3.5.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090708 Firefox/3.5.1pre ID:20090708044703

When I go through a certain procedure, 
Name in the Detail Pane of the Library does not be updated and remained editable.

Reproducible: Always

Steps to Reproduce:
1. STart Firefox3.5 with new profile
2. Open Library (Ctrl + Shift + B)
3. Click "Bookmarks Menu" in the Left Pane
4. Create New Bookmark with Location "place:type=6&sort=1" Named "New Bookmark"
5. Close the Library
6. Open Library (Ctrl + Shift + B) again
7. Expand "Bookmarks Menu" in the Left Pane by tree-twisty click.
8. Click the created "New Bookmark" in the Left Pane
9. Click "Tags" in the Left Pane


Actual Results:  
Name is not updated.
Name is editable in the Detail Pane.


Expected Results:  
Name should be updated.
Name should not be editable.

Comment 1

[Alice0775 White]

I thinkI think that I had better compare node than I compare uri.

diff -b -U 8 orig/places.js new/places.js
--- orig/places.js	2009-07-08 06:07:34.000000000 +0900
+++ new/places.js	2009-07-11 13:21:42.577977600 +0900
@@ -177,17 +177,17 @@
    * @param   resetSearchBox
    *          true if the search box should also be reset, false otherwise.
    *          The search box should be reset when a new folder in the left
    *          pane is selected; the search scope and text need to be cleared in
    *          preparation for the new folder.  Note that if the user manually
    *          resets the search box, either by clicking its reset button or by
    *          deleting its text, this will be false.
    */
-  _cachedLeftPaneSelectedURI: null,
+  _cachedLeftPaneSelectedNode: null,
   onPlaceSelected: function PO_onPlaceSelected(resetSearchBox) {
     // Don't change the right-hand pane contents when there's no selection.
     if (!this._places.hasSelection)
       return;
 
     var node = this._places.selectedNode;
     var queries = asQuery(node).getQueries({});
 
@@ -222,19 +222,19 @@
       PlacesSearchBox.updateCollectionTitle(folderTitle);
 
     // When we invalidate a container we use suppressSelectionEvent, when it is
     // unset a select event is fired, in many cases the selection did not really
     // change, so we should check for it, and return early in such a case. Note
     // that we cannot return any earlier than this point, because when
     // !resetSearchBox, we need to update location and hide the UI as above,
     // even though the selection has not changed.
-    if (node.uri == this._cachedLeftPaneSelectedURI)
+    if (node == this._cachedLeftPaneSelectedNode)
       return;
-    this._cachedLeftPaneSelectedURI = node.uri;
+    this._cachedLeftPaneSelectedNode = node;
 
     // At this point, resetSearchBox is true, because the left pane selection
     // has changed; otherwise we would have returned earlier.
 
     PlacesSearchBox.searchFilter.reset();
     this._setSearchScopeForNode(node);
     if (this._places.treeBoxObject.focused)
       this._fillDetailsPane([node]);

Comment 2

[Alice0775 White]

Works(the name is updated correctly):
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8a4847a6194c
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090319 Shiretoko/3.5b4pre ID:20090319045129

Broken:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090320 Shiretoko/3.5b4pre ID:20090320052032
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6fa887ad479f

Push log:
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=8a4847a6194c&tochange=6fa887ad479f
Regression caused by defb09f7b933	Bug 451151 - Search box In Places Organizer (Library) not behaving correctly

Comment 3

[Drew Willcoxon :adw]

Confirming, but see bug 451151 comment 15 about the problem of using _cachedLeftPaneSelectedNode.  Maybe we could check the node's parent or viewIndex in addition to the URI.

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

viewIndex should change with live updating of the left pane, does not sound like a unique way to identify an element.

Comment 5

[Gervase Markham [:gerv]]

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

Comment 6

[Alice0775 White]

I can still reproduce str of comment #0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9pre) Gecko/20110107 Firefox/4.0b9pre ID:20110107030356