51833 - Expanding tree folders sometimes causes lower-down folders to not paint

Status: VERIFIED DUPLICATE of bug 54049

(Core :: Networking, defect, P2)

Description

[Tapio Piironen]

I'll send two pics.
My system is redhat 6.2 with newest updates.

Comment 1

[Tapio Piironen]

Attachment: Before opening the directory.

Comment 2

[Tapio Piironen]

Attachment: And after

Comment 3

[Jeremy M. Dolan]

when reporting a bug, please specify build number.

Probable dupe of 51814.

Investigating...

Comment 4

[Jeremy M. Dolan]

confirmed... something is definatly going on with ftp. see also bug 51814, which
says during initial load, directories are missing... i didnt notice this
behaviour, myself.

note that the files dont actually disapear! if you click where they should be,
they highlight and are drawn again.

over to networking... good luck.

Comment 5

[Asa Dotzler [:asa]]

maybe a tree problem.

Comment 6

[Gagan]

->rjc

Comment 7

[Robert John Churchill]

Tree display problem, over to hyatt

Comment 8

[Peter Trudelle]

nsbeta3+, P3 for M18

Comment 9

[Brian Ryner (not reading)]

Reproduced on WinNT also, OS->All.

Comment 10

[Peter Trudelle]

->bryner

Comment 11

[Peter Trudelle]

I think this has to go to nsbeta3- tomorrow.

Comment 12

[John Morrison]

I'm not sure if this can go to b3-. FTP on Linux is undependable right now.

bryner: here's one manifestation:
1) go to ftp://ftp.netscape.com/pub/netscape6/
2) there are five folders. Starting with the bottom folder, quickly open
   each folder (which seems to kick off an asynch) load. When each folder
   loads, it winds up painting where it currently thinks it should be, without
   regard for the other folders. After that, the visual appearance is way out
   of synch with the logical model.

Comment 13

[Brian Ryner (not reading)]

Yes, I'd like to push this up to P2.  Can we do that?

Comment 14

[John Morrison]

We can cover it tomorrow with Peter, but I think this must be fixed. I can also
reproduce this on Windows on occasion. 

By the way, bryner, do you think that if the 'spinner' image had the same pixel
height as the other images, this problem would be less severe (if not entirely 
solved). As it stands right now, when the spinner kicks in, it causes all the 
row heights to increase by 2px. (There is this bug (bug 45341) which does point
out that all icons in trees should have a uniform height to avoid this dynamic
row-height resizing). 

Comment 15

[Brian Ryner (not reading)]

Yes, I noticed that too, and I think that might help.  I can't reproduce this on 
any other trees [than ftp], coincidentely, no other trees have a spinner image.

As for the actual cause, it seems that for some reason we are failing to lay out 
all of the items in the tree.  We seem to lay out items down to either the same 
number of rows the tree had prior to expanding the node, or down to the bottom 
of the children of the open node.  FTP is adding kids to the node as they are 
read (or so I hope), which means we need to relayout at least from that point to 
the bottom every time a child is added.  In other words, this could be a deeper 
issue with trees when a node is expanded and all the children aren't immediately 
available.

Comment 16

[Brian Ryner (not reading)]

Unfortunately, the spinning load icon isn't the culprit.  I tried commenting out 
that css rule and still see the disappearing directories.

Comment 17

[Peter Trudelle]

Bumping to P2 after discussion with QA lead, since this manifests itself 
insidiously as apparent data loss.  Users are unable to tell that the listing 
has been truncated, and will therefore have no reason to try to use a 
workaround.

Comment 18

[Brian Ryner (not reading)]

Unfortunately, no progress on this thusfar...

Comment 19

[Phil Peterson]

PDT agrees P2

Comment 20

[Brian Ryner (not reading)]

I just witnessed this same problem in mail when loading a list of folders from
the server.  Attempting to update summary to more accurately describe the problem.

Comment 21

[Peter Trudelle]

mass-adding rtm keyword to all open nsbeta3+ xptoolkit bugs

Comment 22

[Phil Peterson]

Bummer, but I don't think it's bad enough to hold PR3 for, especially if no fix 
is imminent. Marking nsbeta3-

Comment 23

[Brian Ryner (not reading)]

*** Bug 49252 has been marked as a duplicate of this bug. ***

Comment 24

[Peter Trudelle]

rtm+ need info.  Worth spending a bit more time on to come up with a simple/safe
fix.  Will have to '-' if that is not possible soon.

Comment 25

[Brian Ryner (not reading)]

cc'ing eric.. he's probably more on top of how to fix this than anyone right now.

Comment 26

[Brian Ryner (not reading)]

Here's some status based on discussions with evaughan and hyatt:

This problem is essentially a result of the tree widget constructing frames from
inside its Layout() method.  This causes the containing Box to not be aware of
the tree's new bounds, and not paint the entire area.

evaughan has written some code to move frame construction outside the Layout()
method.  This is a fairly major change (although it is, without question, the
Right Way to do it).  I've been running with it in my tree for awhile and have
noticed no adverse effects, and this code also apparently optimizes the region
that the tree paints during scrolling.  I think we should definitely evaluate
this change, but not without thorough testing.  I'll attach his patch to this bug.

We've also investigated some quick-fix options for the immediate problem.  None
have been found so far that don't cause a negative performance hit on scrolling.

Comment 27

[Brian Ryner (not reading)]

Attachment: eric's patch to fix the problem

Comment 28

[Eric Vaughan]

Attachment: This is the current patch for tree fixes. It touches nsIPresShell so make sure you do a depend build from the top.

Comment 29

[Peter Trudelle]

What's up with this bug?  It has been a week since the last patch was attached.
 Does it fix the problem, without regressing performance or anything else?  Any
reason it hasn't been reviewed?  May be too late for this already.

Comment 30

[John Morrison]

This looks (to me) like it is the same (or almost) patch that Eric has checked
into the trunk for bug 54049 "folder pane, thread pane and Buddy list often 
fails to refresh" -- it's the uber-tree-patch which is now rtm++ this evening.

As far as the original problem report for this bug, this problem does not 
appear in the mac/linux/win32 trunk builds in FTP directories, or in other 
tree widgets as far as I have seen. 

(There does appear to be a problem in getting new content displayed in the tree 
for an FTP listing, which is independent of the tree display glitches -- 
whether the breakdown occurs in the tree, or in RDF, or in Necko would best be 
investigated as a separate bug, one which won't make RTM).

Comment 31

[Peter Trudelle]

This does not look like the same patch, although there is a lot of overlap. Are
they intended to both go in?  Has one superceded the other?  What is the plan
here?  I must say, the very idea of a huge 'uber-patch' for 'tree fixes'
(plural) is quite scary at this stage in the release.

Comment 32

[Brian Ryner (not reading)]

Marking dup.  This is one and the same as bug 54049, and the fix attached to
that bug will fix this as well (and the patches attached to this bug are a
little dated).


*** This bug has been marked as a duplicate of 54049 ***

Comment 33

[John Morrison]

verified duplicate (and fixed [in general] -- they are a few ftp edge cases
already filed).