392802 - File listings very hard to read

Status: VERIFIED FIXED

(Firefox :: General, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

The default toolkit styling for file listings now uses a fairly light gray text color and fairly low opacity on the icons for all files except the one you're hovering.  This gives the whole listing a very washed-out, hard-to-read appearance.

I'd recommend ditching the washed-out opacity and text color styling and sticking with whatever color matches the background color we're using.

Comment 1

[Dão Gottwald [:dao]]

Attachment: remove opacity effect (checked in)

Comment 2

[Asaf Romano (gone)]

Comment on attachment 278485 [details] [diff] [review]
remove opacity effect (checked in)

needs ui-r (likely post-facto given current AFKs :-/).

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 278485 [details] [diff] [review]
remove opacity effect (checked in)

looks like Toolkit could use a Themes component ...

Comment 4

[Boris Zbarsky [:bzbarsky]]

I put this in Core so I could set the right blocker flags on it (which have now been nuked by the product change).

And Firefox is definitely the wrong product for this, since this bug affects Seamonkey as well.  I do think the component where I filed it was the right one.  Is the problem that it doesn't have a ui-review flag?  Do we need to add one?

Comment 5

[Dão Gottwald [:dao]]

According to <https://bugzilla.mozilla.org/describecomponents.cgi?product=Core>, it wasn't the right one, and yes, there was no ui-review flag.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I hate our component (dis)organization.  In any case, next time you do that please restore the nearest-equivalent blocking flags, ok?

Comment 7

[Dão Gottwald [:dao]]

Sure.

Comment 8

[Reed Loden [:reed]]

Checking in toolkit/themes/pinstripe/global/dirListing/dirListing.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/dirListing/dirListing.css,v  <--  dirListing.css
new revision: 1.4; previous revision: 1.3
done
Checking in toolkit/themes/winstripe/global/dirListing/dirListing.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/dirListing/dirListing.css,v  <--  dirListing.css
new revision: 1.4; previous revision: 1.3
done

Comment 9

[Dão Gottwald [:dao]]

Boris, you might want to reopen this bug if you still think the gray text shouldn't be gray. In that case, please also add the uiwanted keyword back.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Uh...  It for sure shouldn't be.  Which part of "hard to read" was unclear?

Comment 11

[Dão Gottwald [:dao]]

Attachment: possible solution

Comment 12

[Boris Zbarsky [:bzbarsky]]

I guess I'm not sure why we're trying to highlight the currently-hovered item.  If we _are_ then I do think that reducing its readability slightly (as the attached screenshot shows) is better than reducing the readability of the whole listing.

Comment 13

[Dão Gottwald [:dao]]

(In reply to comment #12)
> I guess I'm not sure why we're trying to highlight the currently-hovered item.

Because it helps to visually connect the cells that belong to a row.

Comment 14

[Boris Zbarsky [:bzbarsky]]

You could get that by having thin row borders, no?  Possibly even just on the hovered row if you want.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Or better yet, something like:

  tr:hover { outline: whatever }

since that won't affect layout either.

Comment 16

[Dão Gottwald [:dao]]

Yes, border doesn't work on tr, but outline would.

Comment 17

[Boris Zbarsky [:bzbarsky]]

> border doesn't work on tr

It does in the collapsed border model.  But I do think the outline solution is better.

Comment 18

[Dão Gottwald [:dao]]

Attachment: possible solution #2

looks better I think, thus making the previous one obsolete.

Comment 19

[Mike Connor [:mconnor]]

moving out bugs that don't need to block b1

Comment 20

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

Comment on attachment 281492 [details]
possible solution #2

Looks good. I wonder if we even need to do the link highlighting, as opposed to instead just highlighting the background of the entire row much like a file explorer.

Comment 21

[Dão Gottwald [:dao]]

Attachment: patch

Since I need to touch the paddings anyway, this also fixes some outstanding RTL issues. For perfect RTL support, I'd need the CSS3 values 'start' and 'end' for the text-align property.

Comment 22

[Asaf Romano (gone)]

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

r=mano

Comment 23

[Reed Loden [:reed]]

Checking in netwerk/streamconv/converters/nsIndexedToHTML.cpp;
/cvsroot/mozilla/netwerk/streamconv/converters/nsIndexedToHTML.cpp,v  <--  nsIndexedToHTML.cpp
new revision: 1.88; previous revision: 1.87
done
Checking in toolkit/themes/pinstripe/global/dirListing/dirListing.css;
/cvsroot/mozilla/toolkit/themes/pinstripe/global/dirListing/dirListing.css,v  <--  dirListing.css
new revision: 1.5; previous revision: 1.4
done
Checking in toolkit/themes/winstripe/global/dirListing/dirListing.css;
/cvsroot/mozilla/toolkit/themes/winstripe/global/dirListing/dirListing.css,v  <--  dirListing.css
new revision: 1.6; previous revision: 1.5
done

Comment 24

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010805 Minefield/3.0b3pre ID:2008010805