Closed Bug 612752 Opened 9 years ago Closed 9 years ago

Focus rings and default style for :-moz-ui-invalid should be re-thought

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b9
Tracking Status
blocking2.0 --- final+

People

(Reporter: mounir, Assigned: mounir)

Details

(Keywords: polish)

Attachments

(2 files, 1 obsolete file)

The focus ring _and_ the red box shadow is shown which isn't really nice.

Possible solutions:
1. Remove the default style when the element has the focus ;
2. Remove the focus ring when the element has :-moz-ui-invalid applying (and make the box shadow bigger?) ;
3. Change the default style so it still works with the focus ring.

I think that 1. wouldn't be really nice for UX, 2. would have to be reverted by authors which might be annoying and 3. isn't easy.
One of the possibilities is to have a lighter box-shadow when the element is focused. Like this:
data:text/html,<style>:invalid.proposal { box-shadow: 0 0 2px 2px rgba(255,0,0,0.4); }</style><input class='proposal' type='email'><br><br><input type='email'>

The first input has a lighter box-shadow but not the second one. I think the focus ring is more visible but I'm not sure it will solve possible a11y issues.

What do you think?
Should that block? It might be an issue for accessibility (color blind people) and it's a very bad UI. I think we should try to found a solution for Firefox 4.
blocking2.0: --- → ?
Attached file Test that (obsolete) —
Jonas and Alexander, could you try that with a nightly build and give me your feedbacks.
Assuming we just want this fainter "ring" when an element has focus (rather than any time it is invalid) then it looks good to me.
Attached file Test that
Exactly. This new test reflect better.
Attachment #494434 - Attachment is obsolete: true
Hmm, maybe we should try to have this different style only when there is a focus ring but as far as I can test, GTK themes without focus ring have :-moz-focusring applying so I don't see what this is for. I will check when I will write the patch.
Attached patch Patch v1Splinter Review
Alexander, you can test that with the attached test case.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #494522 - Flags: ui-review?(limi)
Attachment #494522 - Flags: review?(dbaron)
Whiteboard: [needs-review]
Comment on attachment 494522 [details] [diff] [review]
Patch v1

Works for me!
Attachment #494522 - Flags: ui-review?(limi) → ui-review+
Comment on attachment 494522 [details] [diff] [review]
Patch v1

r=dbaron.  Sorry for the delay getting to this.
Attachment #494522 - Flags: review?(dbaron) → review+
Whiteboard: [needs-review] → [needs-landing][needs-try]
Whiteboard: [needs-landing][needs-try] → [needs-landing][passed-try]
Keywords: polish
http://hg.mozilla.org/mozilla-central/rev/af69450fedb9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs-landing][passed-try]
Flags: in-testsuite+
Target Milestone: --- → mozilla2.0b9
You need to log in before you can comment on or make changes to this bug.