301776 - Can't see focus on tree or list items

Status: VERIFIED FIXED

(Firefox :: Disability Access, defect)

Description

[Aaron Leventhal]

Sometimes you literally can't see the focus, in the case of an unselected list
item, where we have no style rule for a focus outline.

Other times it's just too faint to really notice.

Comment 1

[Aaron Leventhal]

Attachment: 2px outline rules, different color depending on whether selected or not, have a rule for unselected list items

Comment 2

[Caleb]

The patch was checked in.

I think that the 2px outline is a bit over-the-top.

Maybe it should be set back to 1px, but leave the color as it is now (darker
than that yellow one)?

I'd like to CC Kevin to and hear his opinion.

Comment 3

[Gary van der Merwe]

This patch causes the favicons in bookmarks to be croped.

Comment 4

[daneel]

What on earth? Now the focus reticle in Firefox looks clunky, and is completely
different to every other application on my system.

Comment 5

[elfguy]

I also think its way overboard. At the most make it an option (off by default)
in the accessibility options, because no other application does that, ever.

Comment 6

[Aaron Leventhal]

Someone else on IRC complained about the 2px thickness of the outline.

Rather than get a lot of complaints, I think we should try to get an appearance
that is visible enough but looks okay before we fix. It doesn't have to do
exactly what the OS does, because honestly some 1px lightly colored dotted focus
outlines are just too hard to see.

We could either go 1px and dashed, at least for the selected focused appearance,
or we could just turn on the loud focus outlines after someone hits ctrl+arrow
or shift+arrow, so that in the normal case you just see things as they used to
appear.

Comment 7

[u118730]

Would it be possible to leave the border 1px dotted, but change its color to the
tree/list text color when focus leaves the selected element? That way the focus
reticle is at least as discernable as the text, and people who use non-standard
Firefox/Windows themes would have less of a problem if the background color on
tree/list items is different.

Comment 8

[Aaron Leventhal]

I'm open to other attempts at style rules. You can see from the original patch
how to modify things. Anyone want to play around and make it more attractive yet
visible enough, please go ahead, otherwise I'll probably get to it after the beta.

Comment 9

[:deanis74]

We shouldn't have this border for the beta.  It's fine for nightlies but we're
going to get a lot of flak for the chunky selection outline for an announced beta.

Comment 10

[Michael]

Attachment: 1px dashed outline

Here's my attempt, 1px dashed is more visable then 1px dotted but not as chunky
as 2px dotted.

Comment 11

[Michael]

(In reply to comment #6)
> We could either go 1px and dashed, at least for the selected focused appearance,
> or we could just turn on the loud focus outlines after someone hits ctrl+arrow
> or shift+arrow, so that in the normal case you just see things as they used to
> appear.

Sorry I see you've already suggested that. Either of those seems good to me.

Comment 12

[Phil Ringnalda (:philor)]

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

Comment 13

[u118730]

Is this bug still on the radar for beta? It has a checkin that not many approve
of, but it's not marked fixed, and it has no blocking flag. Is the checkin (that
nobody likes) going to remain for beta or will it be backed out/reworked?

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Using non system colors isn't a good solution. IMO ThreeDDarkShadow is a good
solution for outline-color. I can the such a color in other applications.
Additionally I agree to use a 1px outline. 2px is really too much and doesn't
look good. As style we should use dotted and not dashed. 

Comment 15

[:deanis74]

I'm, as usual, speaking only from Windows experience.  Why are we deviating from
how things look in common Windows controls, or trying to add non-standard
behaviors like changing the outline on Ctrl+Arrow?

Comment 16

[Aaron Leventhal]

It seems very unpopular so I guess we should just back it out.

The reason we put deviated? I have 20/20 vision myself and I just can't see the
old focus outline unless I'm squinting. A lot of people just can't see it.

But go ahead and back it out. We'll come up with a better solution to the
problem later. Right now I'm dealing with some hairy focus regressions and don't
have time to help back this out.

Comment 17

[u118730]

Attachment: screenshot of 1px dotted border with correct colors

I tried the following in userchrome.css and it produced the attached results. I
tested on the last official release (1.0.6), since it suffered from this bug
too and doesn't have the current patch applied like the nightlies do.

treechildren::-moz-tree-row(current, focus) {
  border: 1px dotted -moz-FieldText !important;
}
treechildren::-moz-tree-row(selected, current, focus) {
  border: 1px dotted #c0c0c0 !important;

listbox:focus > listitem[selected="false"][current="true"] {
  border: 1px dotted -moz-FieldText !important;
}

I'm not sure if -moz-FieldText is the right entity to use, but if we want the
focus ring to be the same color as the text it'll work. Also, I used a gray
color "c0c0c0" for the selected with focus for simplicity, but we'd also want a
named entity there too, like "Highlight" maybe? Of course, we wouldn't want the
"!important"s in there as well. This shouldn't be too hard, though. I don't
have the means to produce a proper patch, but I know the first two of the above
rules belong in /cvsroot/mozilla/toolkit/themes/winstripe/global/tree.css and
the third is goes in
/cvsroot/mozilla/toolkit/themes/winstripe/global/listbox.css. If someone wants
to produce a patch, most of the work would already be done. This assumes that
the proposed patch gets backed out first.

Comment 18

[Phil Ringnalda (:philor)]

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

Comment 19

[Phil Ringnalda (:philor)]

Note (from bug 302946) that if it doesn't get backed out or shrunk to 1px, that
the height in at least the bookmarks manager and bookmarks sidebar, if not all
tree rows, needs to be increased: apparently tree borders are part of the
height, so adding a couple of px of border cuts off the bottom of favicons.

Comment 20

[Caleb]

Attachment: untested 1px dotted border

This is an untested patch that should switch the border to 1px instead of 2px.

Comment 21

[Caleb]

Attachment: 1px dotted border (untested)

better diff this time

Comment 22

[:deanis74]

Requesting blocking based on comment 16 and comment 19.

Comment 23

[Phil Ringnalda (:philor)]

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

Comment 24

[Asa Dotzler [:asa]]

Aaron, can you back out this patch? It's one of the most visible UI
"regressions" we shipped in beta and it would be great if we could undo this
breakage. Thanks

Comment 25

[Asa Dotzler [:asa]]

Aaron, have you had a chance to get this backout together?

Comment 26

[Aaron Leventhal]

Comment on attachment 196123 [details] [diff] [review]
1px dotted border (untested)

This patch looks good to me for both listbox and tree.

The reason a simple backout wouldn't do is we previously had no style rule at
all for focused list items.

Comment 27

[Scott MacGregor]

Comment on attachment 196123 [details] [diff] [review]
1px dotted border (untested)

approving for the branch. would be great if you could get this in today.