55899 - ftp and file picker columns get out of synch when scrolling or opening folders

Status: VERIFIED FIXED

(Core Graveyard :: Skinability, defect, P3)

Description

[Eric Vaughan]

To repoduce:

1) open the browser and go to ftp://ftp.mozilla.org
2) Open a bunch of directories
3) Scroll up and down

Then columns don't line up with the contents. They are just scattered all over 
the place. 

To reproduce in file picker

1) One unix select open file
2) Scroll up and down the columns will not align

This is high visibility and I already have a simple CSS fix.  Attached is the 
patch. This fix is a simple 9 line change directory.css.

(Directory in in 4 places. That's why the patch looks redundant.)

Comment 1

[Eric Vaughan]

Attachment: patch

Comment 2

[Peter Trudelle]

rtm need info: this minimal css fix has already been done for thread pane, would
be good to get it in for FTP if possible.  Probably best to test on trunk first.

Comment 3

[David Hyatt]

a=hyatt

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

r=ben

Comment 5

[Peter Trudelle]

Why isn't this marked rtm+?  It seems like it was ready a day ago.  Has it been
thoroughly tested?  cc self, jrgm

Comment 6

[John Morrison]

This was checked into the trunk Monday afternoon. In this evening's trunk
builds, mac/linux/win32, this is working well and reliably so. It fixes the
scroll problem in both ftp:// and file:// listings of directory.xul (and 
improves the default column sizing as a bonus!). No regressions noted. 

So, with r=ben, a=hyatt, tested on trunk on three platforms, marking [rtm+] 
for PDT approval.

Comment 7

[Eric Vaughan]

This fix is extremely simple. Its a little CSS that is only invoked when ftp is 
hit of the file picker is hit. It is VERY LOW risk in my opinion.

Comment 8

[selmer (gone)]

rtm++

Comment 9

[Eric Vaughan]

Fixed in branch.

Comment 10

[lchiang]

blake - can I assign this to someone to verify on the branch since jrgm has 
verified on the trunk?

Comment 11

[Blake Ross]

I regularly build the branch...I'll verify this asap tomorrow, since I find 
that 2:30 am isn't optimal time for verifications ;)

Comment 12

[Blake Ross]

vrfy fixed in new win2k trunk build, but NOT working in new win2k branch build. 

I also just filed related bug 58346 which doesn't work in either the branch or 
trunk, but that'll probably require a different fix (it's not a directory 
structured tree).

Why is this bug in skinability, anyways?

Comment 13

[John Morrison]

If I didn't know better, I would say that something was up with CVS. I "saw"
these checkins to the branch, and had already checked that it was working
for branch builds in both skins. But now, it says that this checkin to 
directory.css never happened for the classic skin. (It does work in the modern 
skin).

Comment 14

[lchiang]

That's strange.  I'll cc: leaf and granrose in case they know what may be happening.

This is reopened and the branch is closed for checkins until Monday. I will
change the rtm++ to rtm+ for re-evaluation for candidate limbo on Monday morning.

Comment 15

[John Morrison]

Sorry. I should have been clearer. I don't really think CVS magically reverted 
this change. (I just have such a clear, but erroneous, recollection that I 
had seen this checkin on the branch that I am stunned to see that that is 
not the case.)

Comment 16

[selmer (gone)]

PDT marking [rtm++]. This bug is now out of limbo and approved for checkin to
the branch. Please check in ASAP.

Comment 17

[Eric Vaughan]

Why was this bug reopened? The fix is in the branch and trunk. Bug 58346 is a
DIFFERENT bug. The fix is not in the tree code but the XUL that describes the
tree. You MUST give each column a width.
 

Comment 18

[Blake Ross]

Eric, I reopened this because it still doesn't work for me in a branch build.  
Can anyone confirm or deny this?

I realize that 58346 will require a different fix; I said that earlier.

Comment 19

[John Morrison]

Eric, the classic skin directory.css does not include the patch to set the 
width: for the treecols. (As I said earlier, I am puzzled as to where this 
patch went, because I was sure I saw it go into the branch, but ... check 
the URL on this bug. I can also confirm the columns get out-of-synch when using
10/29 win32 Classic skin).

Comment 20

[Eric Vaughan]

Patch reappled to classic skin.

Comment 21

[Blake Ross]

Yes, classic-only.  Sorry, should have mentioned that...

and that's very strange wrt the patch already having gone in.

Comment 22

[Eric Vaughan]

fixed

Comment 23

[lchiang]

blake - can you verify this in the branch builds again?

Comment 24

[John Morrison]

verified on the branch 11/01 mac/linux/win32 builds (really this time). 
(and blake has verified trunk previously comments above).

(Side-note: I think blue on the trunk doesn't have this css change, but
that's outside the scope of this bug, and I don't know if there is really
any support for maintaining blue going forward).