If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

File listings very hard to read

VERIFIED FIXED in Firefox 3 beta3

Status

()

Firefox
General
P2
normal
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: bz, Assigned: dao)

Tracking

(Blocks: 1 bug, {access, regression})

Trunk
Firefox 3 beta3
access, regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

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.
Flags: blocking1.9?
(Assignee)

Updated

10 years ago
Keywords: uiwanted
(Assignee)

Comment 1

10 years ago
Created attachment 278485 [details] [diff] [review]
remove opacity effect (checked in)
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #278485 - Flags: review?(mano)
Comment on attachment 278485 [details] [diff] [review]
remove opacity effect (checked in)

needs ui-r (likely post-facto given current AFKs :-/).
Attachment #278485 - Flags: review?(mano) → review+
(Assignee)

Updated

10 years ago
Assignee: dao → nobody
Status: ASSIGNED → NEW
Component: Themes → Tabbed Browser
Flags: blocking1.9?
Product: Core → Firefox
QA Contact: themes → tabbed.browser
(Assignee)

Updated

10 years ago
Component: Tabbed Browser → General
QA Contact: tabbed.browser → general
(Assignee)

Comment 3

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

looks like Toolkit could use a Themes component ...
Attachment #278485 - Flags: ui-review?(mconnor)
(Assignee)

Updated

10 years ago
Assignee: nobody → dao
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
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?
(Assignee)

Comment 5

10 years ago
According to <https://bugzilla.mozilla.org/describecomponents.cgi?product=Core>, it wasn't the right one, and yes, there was no ui-review flag.
I hate our component (dis)organization.  In any case, next time you do that please restore the nearest-equivalent blocking flags, ok?
Flags: blocking-firefox3?
(Assignee)

Comment 7

10 years ago
Sure.
Blocks: 392803
(Assignee)

Updated

10 years ago
Attachment #278485 - Flags: ui-review?(mconnor) → ui-review?(beltzner)
Flags: blocking-firefox3? → blocking-firefox3+
Attachment #278485 - Flags: ui-review?(beltzner) → ui-review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed, uiwanted
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M9
(Assignee)

Comment 9

10 years ago
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.
Uh...  It for sure shouldn't be.  Which part of "hard to read" was unclear?
Status: RESOLVED → REOPENED
Keywords: access, uiwanted
Resolution: FIXED → ---
(Assignee)

Updated

10 years ago
Attachment #278485 - Attachment description: remove opacity effect → remove opacity effect (checked in)
(Assignee)

Comment 11

10 years ago
Created attachment 281488 [details]
possible solution
Attachment #281488 - Flags: ui-review?(beltzner)
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.
(Assignee)

Comment 13

10 years ago
(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.
You could get that by having thin row borders, no?  Possibly even just on the hovered row if you want.
Or better yet, something like:

  tr:hover { outline: whatever }

since that won't affect layout either.
(Assignee)

Comment 16

10 years ago
Yes, border doesn't work on tr, but outline would.
> border doesn't work on tr

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

Comment 18

10 years ago
Created attachment 281492 [details]
possible solution #2

looks better I think, thus making the previous one obsolete.
Attachment #281488 - Attachment is obsolete: true
Attachment #281492 - Flags: ui-review?(beltzner)
Attachment #281488 - Flags: ui-review?(beltzner)
moving out bugs that don't need to block b1
Target Milestone: Firefox 3 M9 → Firefox 3 M10

Updated

10 years ago
Status: REOPENED → ASSIGNED
Priority: -- → P2
(Assignee)

Updated

10 years ago
OS: Linux → All
Hardware: PC → All
Whiteboard: [needs review beltzner]
Target Milestone: Firefox 3 M10 → Firefox 3 M11
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.
Attachment #281492 - Flags: ui-review?(beltzner) → ui-review+
(Assignee)

Updated

10 years ago
Whiteboard: [needs review beltzner]
(Assignee)

Comment 21

10 years ago
Created attachment 295482 [details] [diff] [review]
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.
Attachment #295482 - Flags: superreview?(bzbarsky)
Attachment #295482 - Flags: review?(mano)
Attachment #295482 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 295482 [details] [diff] [review]
patch

r=mano
Attachment #295482 - Flags: review?(mano) → review+
(Assignee)

Updated

10 years ago
Keywords: uiwanted → checkin-needed
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
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3pre) Gecko/2008010805 Minefield/3.0b3pre ID:2008010805
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.