Closed
Bug 566045
Opened 15 years ago
Closed 14 years ago
Default style for invalid form elements with :invalid
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla2.0b5
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: mounir, Assigned: mounir)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, html5)
Attachments
(3 files, 4 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
limi
:
ui-review+
sicking
:
approval2.0+
|
Details | Diff | Splinter Review |
A default style for :valid is probably not needed but we may want to set a default style for :invalid.
Assignee | ||
Comment 1•14 years ago
|
||
So, this is the mockup from limi: https://bugzilla.mozilla.org/attachment.cgi?id=451079
I suppose we do not want any default style for :valid (probably too intrusive, right ?) and for :invalid, according to the mockup it sounds like -moz-box-shadow.
However, -moz-box-shadow do not apply on most form elements. It is working for <fieldset> and <input type='file'> but it is not working for <textarea>, <button>, <select>, <keygen>, <input type='radio'> and <input type='checkbox'>.
So, Boris and David, do you think we can/want to fix that ? (it's not working on webkit too, at least for <input> and opera has a complete support except rendering issues for <input type='radio'> and <input type='checkbox'>)
Otherwise, what could replace box-shadow ? outline is the most similar CSS property I know but it's not as beautiful (and it's an understatement ;)).
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•14 years ago
|
||
You can use this preview to check box-shadow support on form elements (some form elements in the file are barred from constraint validation so we don't need to have a support for all of them).
Yeah, I agree that we don't want styling for :valid. Would love to see box-shadow fixed for all form controls though. I suspect checkbox and radios will be harder than the other ones since we don't render those ourselves.
Comment 4•14 years ago
|
||
So is box-shadow just not working on natively themed stuff? Why not?
Assignee | ||
Comment 5•14 years ago
|
||
I'm using a svg filter to make all elements looking red.
This way is probably a bit too intrusive and not really beautiful. In addition, it will probably not work really well if the background color or the text color is red (we may not see the text anymore).
The good point is it will work for any kind of elements.
Attachment #457808 -
Attachment is obsolete: true
Assignee | ||
Comment 6•14 years ago
|
||
This is a simulation of a box-shadow using svg filters: it's creating a copy of the element which is completely red then it blurs it and show it behind the original element. It should work for any kind of elements too I think.
Assignee | ||
Comment 7•14 years ago
|
||
It would be great to have feedbacks from Jonas, limi (UI) and roc (technical aspects).
I suppose the box-shadow simulation is the best way to go?
I'm wondering how we can have this filter available from forms.css...
Roc, do we need to optimized some code as you told on IRC?
In addition, I see two tweaks we can do:
- input[type='file'] -> we could use -moz-box-shodow as it is not a system widget
- :focus -> we should have a special behavior because there is a focus-ring on some platform which may be confusing.
We should probably file a bug about allowing box-shadow on all form controls, even if we won't need it right now.
Sorry. I missed bug 581222 in the dependency field.
Comment on attachment 459848 [details]
Simulating a box-shadow with svg filters
This is beautiful! You rock!!
Would be great if we could trigger this type of rendering by putting something like this in forms.css:
:invalid, :-moz-invalid-submit-button {
-moz-invalid-form-control-glow: 1;
}
Definitely want input from roc regarding performance though.
Even cooler would be if we could implement box-shadow on form controls this way. Then we could simply put
:invalid, :-moz-invalid-submit-button {
box-shadow: ...;
}
However it would mean having to support multiple shadows, offsets, etc, so probably too much work for FF4 I'm guessing.
Actually, maybe something as simple as:
:invalid, :-moz-invalid-submit-button {
filter: url(chrome://global/content/formglow.svg#glow;
}
could work as implementation for now. Definitely would need rocs feedback on performance though.
I'm not sure what works in terms of accessing a filter from forms.css, but it looks like we have
-moz-binding: url("chrome://global/content/platformHTMLBindings.xml#inputFields");
for inputs, so you can probably put something in the same place.
The problem of the red glow not showing up on a red background is pretty hard to solve with any approach. I'm not sure what we should do there. Maybe just make it red and hope that authors with red backgrounds using HTML5 forms apply an author style?
Mounir, you should test a large widget in the invalid state (basically a textarea), scrolling around inside and outside that widget, etc. Get some profiles and see how much time we're spending in SVG filter computations.
Another option is to use box-shadow, revert 476062 and reimplement box-shadow to look good on native-themed widgets. For native-themed widgets, we could extract the alpha-channel of the widget and say that that's the box shape, and get equivalent results to what you see here, possibly with some special-casing for widgets that don't normally draw themed backgrounds, like text inputs, so that if the author creates a text input with a transparent background, we don't do something crazy.
I suspect with the SVG filter approach, a transparent text input does look pretty crazy right now.
Yeah, I actually think that fixing box-shadow is the right way to go here for performance, and for authors since the first thing authors will think of to disable our glow is to set box-shadow:none. Let's move that discussion to bug 581222.
Roc, the only concern I have is what we have time for in the FF4 timeframe. I suspect someone will have to lend Mounir a hand.
Assignee | ||
Comment 15•14 years ago
|
||
(In reply to comment #12)
> The problem of the red glow not showing up on a red background is pretty hard
> to solve with any approach. I'm not sure what we should do there. Maybe just
> make it red and hope that authors with red backgrounds using HTML5 forms apply
> an author style?
Yes, we can't have a style that would work everywhere for everyone. If someone is using too much red in his/her website, :invalid may be his/her friend :)
> Mounir, you should test a large widget in the invalid state (basically a
> textarea), scrolling around inside and outside that widget, etc. Get some
> profiles and see how much time we're spending in SVG filter computations.
I guess, if you are trying to have -mox-box-shadow working (comment 13), the benchmark isn't needed. I will see if I can do that, at least for curiosity.
> Another option is to use box-shadow, revert 476062 and reimplement box-shadow
> to look good on native-themed widgets. For native-themed widgets, we could
> extract the alpha-channel of the widget and say that that's the box shape, and
> get equivalent results to what you see here, possibly with some special-casing
> for widgets that don't normally draw themed backgrounds, like text inputs, so
> that if the author creates a text input with a transparent background, we don't
> do something crazy.
>
> I suspect with the SVG filter approach, a transparent text input does look
> pretty crazy right now.
The shadow should be shown inside and outside in that case. But isn't a transparent background already something crazy?
(In reply to comment #14)
> Roc, the only concern I have is what we have time for in the FF4 timeframe. I
> suspect someone will have to lend Mounir a hand.
Roc, do you think someone can take care of fixing box-shadow or I should jump on that?
I'm working on it today.
Comment 17•14 years ago
|
||
CC shorlander for theme advice... We'll eventually want to tweak the exact shadow style/color to look right (and match the platform glow, in the case of OS X?). I'm not sure if there's a preferred approach (SVG vs box-shadow) from a front-end point of view, perhaps we should file a Toolkit::Themes bug for adjusting the exact style, and just use this bug for getting the basic functionality?
Assignee | ||
Updated•14 years ago
|
Summary: CSS pseudo-classes :valid / :invalid defaults → Default style for invalid form elements with :invalid.
Assignee | ||
Updated•14 years ago
|
Summary: Default style for invalid form elements with :invalid. → Default style for invalid form elements with :invalid
Assignee | ||
Comment 18•14 years ago
|
||
The default style is:
-moz-box-shadow 0 0 2px 1px red;
I'm going to attach some screenshots for the ui-review.
David, the patch may sound big but it's because of the tests changes. The real change is only 3 lines of CSS.
Attachment #468619 -
Flags: ui-review?(limi)
Attachment #468619 -
Flags: review?(dbaron)
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → ?
Assignee | ||
Comment 19•14 years ago
|
||
Assignee | ||
Comment 20•14 years ago
|
||
If you have a build with roc's patch, this page should show you a preview of the style.
Attachment #459847 -
Attachment is obsolete: true
Attachment #459848 -
Attachment is obsolete: true
Assignee | ||
Comment 21•14 years ago
|
||
Actually, there is no :invalid style for <output> at the moment (it's not doable per the specifications) even if the screenshot/preview page show one.
My patch is on trunk since yesterday.
Comment 23•14 years ago
|
||
Comment on attachment 468619 [details] [diff] [review]
Patch v1
I think if we're going to do this, we need to change -moz-box-shadow into box-shadow. I filed bug 590039 on doing that.
I guess the <output> bit in the screenshot actually isn't part of the patch?
r=dbaron (though I didn't look too closely at the test changes)
Attachment #468619 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 24•14 years ago
|
||
(In reply to comment #22)
> My patch is on trunk since yesterday.
Indeed, I should have been more explicit: a nightly or a recent build from trunk/try-server should work.
Assignee | ||
Comment 25•14 years ago
|
||
(In reply to comment #23)
> Comment on attachment 468619 [details] [diff] [review]
> Patch v1
>
> I think if we're going to do this, we need to change -moz-box-shadow into
> box-shadow. I filed bug 590039 on doing that.
This patch needs to land this week. Do you think there is a chance bug 590039 lands quickly? Otherwise, we could land this patch and have bug 590039 landing in the next beta?
> I guess the <output> bit in the screenshot actually isn't part of the patch?
Right.
Comment 26•14 years ago
|
||
I think we're ok landing this first.
Comment 27•14 years ago
|
||
According to blizzard, all the HTML5 form bits are opportunistic; we're not going to block on them.
blocking2.0: ? → -
Assignee | ||
Comment 28•14 years ago
|
||
r=dbaron
I've updated the patch because some reftests tests were failing with the default :invalid style.
Attachment #468619 -
Attachment is obsolete: true
Attachment #468619 -
Flags: ui-review?(limi)
Assignee | ||
Updated•14 years ago
|
Attachment #468760 -
Flags: ui-review?(limi)
Assignee | ||
Updated•14 years ago
|
Attachment #468760 -
Flags: approval2.0?
Comment on attachment 468760 [details] [diff] [review]
Patch v1.1
a=sicking assuming you get all the necessary reviews. Though I'm not sure if limi needs to ui-review this given that it matches his draft exactly...
Attachment #468760 -
Flags: approval2.0? → approval2.0+
Comment 30•14 years ago
|
||
Comment on attachment 468760 [details] [diff] [review]
Patch v1.1
Looks good to me if the screenshot PNG is indeed representative of this patch.
Attachment #468760 -
Flags: ui-review?(limi) → ui-review+
Assignee | ||
Comment 31•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
Comment 32•14 years ago
|
||
Were there any a11y experts consulted here? I am not one, but I suppose that for someone with total color blindness the red glow might be almost indistinguishable from the (blue) glow for :focus on Mac...
Comment 33•14 years ago
|
||
(In reply to comment #32)
> Were there any a11y experts consulted here? I am not one, but I suppose that
> for someone with total color blindness the red glow might be almost
> indistinguishable from the (blue) glow for :focus on Mac...
It's not a requirement to get the forms right, it's an peripheral state change that will have explanatory text if you submit the form with errors anyway. So consider it as "additional signal", not "main error indicator". :)
Assignee | ||
Comment 34•14 years ago
|
||
(In reply to comment #32)
> Were there any a11y experts consulted here? I am not one, but I suppose that
> for someone with total color blindness the red glow might be almost
> indistinguishable from the (blue) glow for :focus on Mac...
We are probably going to tweak the invalid glow when there is a focus ring because even if you are not color blind that is quite ugly. See bug 561636 for more information about what is planned.
Assignee | ||
Updated•14 years ago
|
Flags: in-testsuite? → in-testsuite+
Assignee | ||
Comment 35•14 years ago
|
||
Adding dev-doc-needed flag so authors now they can override the default style with:
:invalid {
-moz-box-shadow: none;
box-shadow: none;
}
Keywords: dev-doc-needed
Comment 36•14 years ago
|
||
:invalid is already documented; was there more that needed to be covered?
https://developer.mozilla.org/en/CSS/%3ainvalid
Assignee | ||
Comment 37•14 years ago
|
||
(In reply to comment #36)
> :invalid is already documented; was there more that needed to be covered?
>
> https://developer.mozilla.org/en/CSS/%3ainvalid
Yes, this bug add a default style for :invalid and authors should know that. It looks like the current documentation do not mention this default style.
Comment 38•14 years ago
|
||
Ah! Right!
I've added text to this effect to the :invalid page.
Keywords: dev-doc-needed → dev-doc-complete
I adjusted the docs since bug 590039 is now fixed. I also added mention of the -moz-submit-invalid since I suspect people that want to remove the glow will look at the :invalid docs.
Comment 40•14 years ago
|
||
Question, as I don't see it addressed here and the dev-doc doesn't mention it.
Checking a form with the latest nightly (rev/8a6a5cf00da7), I noticed the input[type="submit"] displaying a red glow, as if it was *invalid*.
Why ? -Bug ?
That button has no 'required' attribute in the source code. From the html5 spec:
http://dev.w3.org/html5/spec/Overview.html#submit-button-state
I understand that the 'required attribute does not apply.
Form is here:
http://l-c-n.com/contact/
Assignee | ||
Comment 41•14 years ago
|
||
That's because we have introduced a pseudo-class for submit controls when the form is invalid, see: bug 580575 and bug 582277.
The pseudo-class is :-moz-submit-invalid and, indeed, it looks like :invalid default style.
Comment 42•14 years ago
|
||
(In reply to comment #41)
> That's because we have introduced a pseudo-class for submit controls when the
> form is invalid, see: bug 580575 and bug 582277.
Hmm, ok - I can eventually see the logic here (and I missed those 2 bugs, sorry about that).
> The pseudo-class is :-moz-submit-invalid and, indeed, it looks like :invalid
> default style.
Yes, that's first thing I found - firebug betrays all secrets :-).
(still not clear on marking fields as invalid before the user has even interacted with them -when the form first loads- but that is something to take up with the whatwg I think. Need to ponder this a little more)
You need to log in
before you can comment on or make changes to this bug.
Description
•