58437 - Fix the ftp and file trees; add a min-height for <treerow>

Status: VERIFIED FIXED

(SeaMonkey :: Themes, defect, P2)

Description

[John Morrison]

Currently, in the skin for directory.xul, which is the tree for ftp://
and file:// listings, the icons for folders, loading and files have
different heights.

This leads to the rows dynamically resizing on certain conditions as the 
loading icon (the spinning arrows) kicks in during the load. Per hyatt,
this puts a lot of stress on the tree code and can lead to situations 
where the child documents of folders are not displayed. 

In the interest of making this tree more stable, we need to add a rule
to modern/blue/classic directory.css that will force the treerows to 
have a min-height of 19px [this is the height that the rows will attain
after the spinning icon kicks in].

This is a very low-risk fix, which can only have an affect on the ftp://
and file:// trees since this CSS is not loaded by any other XUL file in 
the product. [As a side-benefit, it even looks better, although the point
of this bug is to get more stable behaviour at very low cost/risk].

I will attach a patch that makes this change (essentially it's a one-liner
times three skins).

Nominating RTM -- definite end-user benefit, very low risk.

cc: ben as skins owner, 
cc: evaughan since he has a limbo bug that affects the same directory.css 
(but his change is not in any way in conflict with this change).

Comment 1

[John Morrison]

Attachment: patch; add 'treerow{min-height:19px}' to directory.css (times three)

Comment 2

[Peter Trudelle]

Marking rtm need info since this is apparent data loss with no obvious
workaround, and the fix is so safe and limited.

Comment 3

[Brian Ryner (not reading)]

r=bryner on jrgm's patch.  hyatt, sr?

Comment 4

[Jason Eager]

This needs a SR immediately, if it is going to get in.

cc:ing waterson.

Comment 5

[selmer (gone)]

The last line of the patch appears to put a } after the line

	\ No newline at end of file

can that possibly be correct?

Can this change ever have any effect on any tree other than the one you're fixing?

Comment 6

[John Morrison]

Attachment: patch; no newline adjustment

Comment 7

[John Morrison]

Actually the patch was showing the difference between '}' and '}\n', but 
looks like an addition. (Sorry, my editor insists on adding the newline, so
I switched to notepad to avoid the correction).

This file, directory.css, is pulled in by only one file: directory.xul, which
implements the tree for ftp:// and file:// directory listings. So, no, this 
cannot affect any other tree. This is a very localized, low-risk, solid benefit
fix.

Comment 8

[David Hyatt]

a=hyatt

Comment 9

[Brian Ryner (not reading)]

checked in on trunk, leaving bug open until decision is made for rtm++

Comment 10

[Phil Peterson]

PDT marking [rtm++]

Comment 11

[lchiang]

jrgm: will you be available to test this patch / fix on the branch builds from
Saturday morning (once patch is checked in) to make sure other tree areas don't
have regressions and that ftp trees work fine?  Thanks!

Comment 12

[John Morrison]

Sure, I can come in and do that. Just to be clear: this patch can not affect
any other tree, except for the single file "directory.xul" which includes 
this .css file:
      http://lxr.mozilla.org/seamonkey/search?string=directory.css
      http://lxr.mozilla.org/seamonkey/search?string=directory.xul
which means only the ftp:// and file:// directory viewers.

Nonetheless, I will be looking at the various trees for general sanity 
checking. (By the way, I have already made this .css change in a copy of 
the branch chrome mac/linux/win32 in both skins, and it works fine).

Comment 13

[Brian Ryner (not reading)]

checked in on branch

Comment 14

[John Morrison]

verified with 2000110409 MN6 branch mac/linux/win32 -- row heights have a 
min-height of 19px, avoiding the dynamic resize of the rows. Previously 
verified on trunk.