431140 - dropping a bookmark or folder in the library window causes flickering

Status: VERIFIED FIXED

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

Description

[Alice0775 White]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-us; rv:1.9pre) Gecko/2008042705 Minefield/3.0pre Firefox/2.0.0.14
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-us; rv:1.9pre) Gecko/2008042705 Minefield/3.0pre

Bookmark Item or Folder in Liblary window,the Liblary window  flickers two or tree times.

Reproducible: Sometimes

Steps to Reproduce:
1.Open Library
2.Drag & Drop Bookmark Folder or Item in the Liblary window
3.
Actual Results:  
The Liblary window flickers two or tree times.

Expected Results:  
Do Not flicker

Comment 1

[Marco Bonardo [:mak]]

confirmed, asking blocking, a probable recent regression

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Seems to be due to the auto-resizing. Seeing it on OSX as well.

Comment 3

[Alice0775 White]

Regression window:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042506 Minefield/3.0pre -- No flicker

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042511 Minefield/3.0pre  -- flicker

Bonsai:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1209124020&maxdate=1209145199

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Suspect bug 430213, but maybe bug 426000, either of which can be backed out if that's the more expedient fix.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This flicker can be seen for several seconds when copying the "All bookmarks" folder to "Unfiled Bookmarks", depending on how much bookmarks you have stored.

Bug 423212 is a bit outside of the regression window but EndUpdateBatch looks like a candidate. It seems that for each update the library window is re-drawn.

Comment 6

[Alice0775 White]

I found another step.

1.Open Library
2.Select a Bookmark item in right upper pane.
3.Press more button(expand deteil pane)
4.Focus Tags input area in deteil pane
5.Focus Keyword input area
6.Focus Tags input area again


1.Open Library
2.Select a Bookmark item in right upper pane.
3.Input something text in the Tags area. 
5.Click tree/listbox or Name etc.

Comment 7

[Marco Bonardo [:mak]]

i see the flicker only if the Tags folder is expanded in the left pane, can someone help confirming this? 
feels like we are visiting tag containers on bookmark select.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Marco, I can confirm this at least on Windows. Having the tags folder open and bookmarking an URL a tag container is selected for less a second. Afterwards the target folder is selected again. Removing a bookmark shows the same behavior but much faster. AFAIR I also had open the tags folder yesterday while testing on OS X.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

This definitely needs to block, and I think all the problems shown in this screencast are related:

http://www.screencast.com/users/beltzner/folders/Jing/media/b1043bbb-3f76-422a-a48d-35b7d1a8f2b7

 - flickering on the renaming of a folder
 - flickering on the creation of a folder
 - conniption fit when deleting a tree of subfolders
 - conniption fit when undoing a delete of a tree of subfolders

The "create new folder" creating two new folders is, I think, a different bug.

Comment 10

[Marco Bonardo [:mak]]

nice video mike, not too funny though :\
Some flickering is due to this bug (if you close Tags you should exclude them), some is due to details pane shrinking (that is bug 431173 waiting for ui-review).

So could you try with patch from bug 431173 and Tags container "closed", to check if the behaviour is what you're looking for?

Comment 11

[Tracy Walker [:tracy]]

I think this pervasive flickering in the library is much more than a minor issue.

Comment 12

[Dietrich Ayala (:dietrich)]

(In reply to comment #5)

> Bug 423212 is a bit outside of the regression window but EndUpdateBatch looks
> like a candidate. It seems that for each update the library window is re-drawn.
> 

it was a very localized batch, almost always contained within another batch (so a no-op).

to be sure, i reverted that patch, and was able to reproduce the flickering.

Comment 13

[Dietrich Ayala (:dietrich)]

oops, removing blocking on bug 430213, since that's inconclusive still.

Comment 14

[Marco Bonardo [:mak]]

i can confirm the regression range, so most likely is really bug 430213.

from first tests is the call to fillDetailsPane in onPlaceselected

Comment 15

[Marco Bonardo [:mak]]

Attachment: patch

this is quite cryptic, but works, we still have some selection event fired in refreshVisibleSection, so i've suppressed them all, then we had a toggle open behaviour that it's quite obscure to me (what's the point in closing an open one and opening a closed one?), plus a fix to update the details pane only if a tree is focused (without this if you edit a folder name in the right pane and you click away you could get details pane filled with left pane selection).

Following this is quite difficult, so if someone want to test this patch in a custom build it would be really useful (also to avoid regressions or liveupdate problems).

Mano, can you elaborate on this please, since you know the treeView code much better than me?

Comment 16

[Asaf Romano (gone)]

Comment on attachment 318781 [details] [diff] [review]
patch

r=mano, thanks!

Comment 17

[Mike Connor [:mconnor]]

Comment on attachment 318781 [details] [diff] [review]
patch

a=mconnor on behalf of 1.9 drivers

Comment 18

[Dietrich Ayala (:dietrich)]

Checking in browser/components/places/content/places.js;
/cvsroot/mozilla/browser/components/places/content/places.js,v  <--  places.js
new revision: 1.164; previous revision: 1.163
done
Checking in browser/components/places/content/treeView.js;
/cvsroot/mozilla/browser/components/places/content/treeView.js,v  <--  treeView.js
new revision: 1.69; previous revision: 1.68
done

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050209 Minefield/3.0pre and the today's nightly build 2008050206 under Windows.

Comment 20

[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