Closed Bug 553992 Opened 10 years ago Closed 9 years ago

No focus rings for buttons in download manager, places window and the update dialog

Categories

(Toolkit :: Themes, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: whimboo, Assigned: stefanh)

References

Details

(Keywords: access, regression, Whiteboard: [inbound])

Attachments

(4 files, 1 obsolete file)

Attached image screenshot
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a4pre) Gecko/20100321 Minefield/3.7a4pre ID:20100321063324

Since the landing of bug 480178 the buttons at the bottom do not have any focus ring anymore. You cannot see if a button is in a hover or active state. I have no idea how that is exposed to screen readers. I'll cc Marco. Eventually this is only a problem on OS X.
Component: Application Update → Themes
Keywords: access
QA Contact: application.update → themes
Henrik, is there the same bug for the Download manager, etc.?
As you point that out, yes it is. Seems to be a widget issue? I can even not see the focus ring for Namoroka builds. Looks like it's older.
Keywords: regression
I was going to wait for bug 418521 before adding focus rings to this button style. It's a theme issue.
Keywords: regression
For screen reader users this usually does not have any impact, since they rely on internal structures/states rather than pure visual indications to communicate states like "focused". However, visually impaired and sighted keyboard navigation users will suffer from this.
Duplicate of this bug: 557853
Attached patch Patch v1.0 (obsolete) — Splinter Review
I haven't yet tested the update dialog and I'll be away for a few days, but this patch should fix buttons in the update dialog, download manager and the places window (all buttons that use the toolbarbutton rules in shared.inc). I use different border color depending on aqua/graphite, but that might be an overkill. The corners where particulary tricky in graphite, so I ended up with just making the borders a bit more transparent.
Assignee: nobody → stefanh
Summary: No focus rings for buttons in the update dialog → No focus rings for buttons in download manager, places window and the update dialog
Comment on attachment 544605 [details] [diff] [review]
Patch v1.0

For screenshots, see attachment #545091 [details]. Do I btw need ui-r? If so, would you mind suggesting a name?
Attachment #544605 - Flags: review?(dao)
Status: NEW → ASSIGNED
Comment on attachment 544605 [details] [diff] [review]
Patch v1.0

Why aren't you just using focusRingShadow instead of toolbarbuttonFocusRingShadow?
(In reply to comment #9)
> Comment on attachment 544605 [details] [diff] [review] [review]
> Patch v1.0
> 
> Why aren't you just using focusRingShadow instead of
> toolbarbuttonFocusRingShadow?

I don't think focusRingShadow matches the look we want. I belive my toolbarbuttonFocusRingShadow matches the toolbar focus rings in the OS (e.g "Show All" in system preferences) much better.
> I don't think focusRingShadow matches the look we want. I belive my
> toolbarbuttonFocusRingShadow matches the toolbar focus rings in the OS (e.g
> "Show All" in system preferences) much better.

Could you provide screenshots that show this difference? I don't have a Mac to test this.
Will this Bug also fix Bug 587813 or is this different?
(In reply to comment #12)
> Will this Bug also fix Bug 587813 or is this different?

That looks like a different issue, more like some kind of re-paint problem.
Attached image Screenshots
(In reply to comment #11)
> > I don't think focusRingShadow matches the look we want. I belive my
> > toolbarbuttonFocusRingShadow matches the toolbar focus rings in the OS (e.g
> > "Show All" in system preferences) much better.
> 
> Could you provide screenshots that show this difference? I don't have a Mac
> to test this.

From left to right: toolbarbuttonFocusRingShadow - native - focusRingShadow.

The difference is not huge. But, especially for the places buttons, the focusRingShadow is a bit thicker and sligtly more "fluffy" (zoom in and it becomes even more visible).
In fact, the difference is very minor. Given this and since apparently this is a temporary solution until we get bug 672050, I'd prefer if you just reused focusedRingShadow.
Attached patch V1.1Splinter Review
(In reply to comment #15)
> In fact, the difference is very minor. Given this and since apparently this
> is a temporary solution until we get bug 672050, I'd prefer if you just
> reused focusedRingShadow.

Sure.
Attachment #544605 - Attachment is obsolete: true
Attachment #548260 - Flags: review?(dao)
Attachment #544605 - Flags: review?(dao)
Comment on attachment 548260 [details] [diff] [review]
V1.1

>--- a/browser/themes/pinstripe/browser/places/organizer.css
>+++ b/browser/themes/pinstripe/browser/places/organizer.css

>+#placesToolbar:-moz-system-metric(mac-graphite-theme) > toolbarbutton:-moz-focusring {
>+  border-color: @toolbarbuttonFocusedBorderColorGraphite@;
>+}

Please move :-moz-system-metric(mac-graphite-theme) to the toolbarbutton.

>--- a/toolkit/themes/pinstripe/mozapps/update/updates.css
>+++ b/toolkit/themes/pinstripe/mozapps/update/updates.css

>+.wizard-buttons:-moz-system-metric(mac-graphite-theme) button:-moz-focusring {
>+  border-color: @toolbarbuttonFocusedBorderColorGraphite@;
>+}

And to the button here.
Attachment #548260 - Flags: review?(dao) → review+
Pushed to inbound with comment #17 addressed:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a7da303efe28
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/a7da303efe28
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.