475273 - Double Clicking a Folder in Right Pane causes Left Pane Tree items to expand and collapse

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P2)

Description

[Leman Bennett [Omega]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090124 Shiretoko/3.1b3pre Firefox/3.0.5 Ubiquity/0.1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090124 Shiretoko/3.1b3pre

Double clicking on a folder in the right pane causes every main branch item in the tree of the left pane to expand and collapse every branch before it reaches its destination. 

Reproducible: Always

Steps to Reproduce:
1. Open the Library.
2. Click on "All Bookmarks" in the left pane.
3. In the Right Pane, double click on "Unsorted Bookmarks".
Actual Results:  
The tree will expand and collapse items before reaching the Unsorted Bookmarks folder.

Expected Results:  
Open Unsorted Bookmarks immediately.

This will happen with any folder double clicked in the right pane. For best effect, double click a folder further down the tree in the right pane.

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

Confirmed, setting to New

Comment 3

[Alice0775 White]

I found regression range as follows.

Works OK:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090107 Minefield/3.2a1pre ID:20090107033209

Works NG:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090113 Minefield/3.2a1pre ID:20090113035246

Changesets of the above regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-01-12+03%3A16%3A15&enddate=2009-01-13+03%3A52%3A46

Comment 4

[Marco Bonardo [:mak]]

regwindow on 1.9.1:
WFM:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090113 Shiretoko/3.1b3pre
NOT:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090114 Shiretoko/3.1b3pre

Regwindow:
http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=7b4d7f11fefc&tochange=fe71edbb2925

crossing the two regwindows the only common push that could have regressed a panel feels like:
Bug 472302 - get rid of fragile timeouts in textbox binding

cc-ing Dao, in case he as any idea about that.

Comment 5

[Marco Bonardo [:mak]]

correct mozilla-central regwindow based on nightlies changesets:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ca9d3c35fe47&tochange=9dbded90af2a

Comment 6

[Marco Bonardo [:mak]]

asking blocking, this is not only quite visible and annoying, but perf killing with many bookmarks

Comment 7

[Dão Gottwald [:dao]]

So why is updateCollectionTitle called so often? 36 times over here, 35 times with title == null and once with "Unfiled Bookmarks".

Comment 8

[Dão Gottwald [:dao]]

Attachment: proposed patch

Comment 9

[Marco Bonardo [:mak]]

even if that's probably a fix we would like to take, the search box is still flickering. I think we need to better look at how the tree view acts, what i see is that opening and closing a node causes a onselect call, while that should not happen. Indeed when opening a flat container in the right pane we walk the tree open and close nodes to find the item to be selected, leaving needed nodes open, that's causing a lot of onselect calls, but the selection should not change.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 358817 [details] [diff] [review]
proposed patch

moving to a separate bug

Comment 11

[Marco Bonardo [:mak]]

from what i see setting selectEventsSuppressed = false; causes an onselect call, the idl does not talk about this, but the code is doing it.
http://mxr.mozilla.org/mozilla-central/source/layout/xul/base/src/tree/src/nsTreeSelection.cpp#611
So we receive potentially useless calls at onPlaceSelected (ie called even if the selection did not really change)

Comment 12

[Marco Bonardo [:mak]]

Attachment: patch v1.0

Taking for now.
I don't think the changes to textbox timers have caused any real regression, but probably have made this much more visible.

places.js change could be enough, hwv for heavy idented bookmarks i think it could make sense null the viewer while recursively looking for nodes, and reopen later only the needed ones.

This could potentially help (solve?) also bug 434845.

Mano, who should i ask a review for the added comment to the layout idl?

Comment 13

[Dietrich Ayala (:dietrich)]

blocking for poor user experience and t-snappiness.

Comment 15

[Asaf Romano (gone)]

Comment on attachment 358831 [details] [diff] [review]
patch v1.0

This optimization should be disabled in multiple-selection tress (or just when multiple items are selected). Otherwise r=mano.

Comment 16

[Marco Bonardo [:mak]]

(In reply to comment #15)
> (From update of attachment 358831 [details] [diff] [review])
> This optimization should be disabled in multiple-selection tress (or just when
> multiple items are selected). Otherwise r=mano.

what's the risk with multiple selections? their folders will be opened and then items selected. Or are you talking about onPlaceSelected? that's called by left pane that has selection=single.

Comment 17

[Asaf Romano (gone)]

I misunderstood the context, r=mano.

Comment 18

[Dietrich Ayala (:dietrich)]

http://hg.mozilla.org/mozilla-central/rev/ddcb3ca67b5e

Comment 19

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/352a209c9e79

Comment 20

[Aakash Desai [:aakashd]]

verified FIXED on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3 ID:20090305133223

Comment 21

[Aakash Desai [:aakashd]]

verified FIXED on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090326 Minefield/3.6a1pre

Comment 22

[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