Closed Bug 605125 Opened 9 years ago Closed 9 years ago

Add :-moz-ui-valid pseudo-class

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8

People

(Reporter: mounir, Assigned: mounir)

References

Details

(Keywords: dev-doc-complete, html5)

Attachments

(5 files, 2 obsolete files)

This is going to be very similar to bug 605124 but for :valid.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #488907 - Flags: review?(bzbarsky)
Comment on attachment 488907 [details] [diff] [review]
Part 1 - Add :-moz-ui-valid pseudo-class

r=me
Attachment #488907 - Flags: review?(bzbarsky) → review+
Attachment #489210 - Flags: review?(bzbarsky)
I've an idea of what :-moz-ui-valid should be but I prefer to be sure limi and Jonas agree before asking reviews.

The idea is to make :-moz-ui-valid apply when:
1. the element is valid
2. the element has it's value changed or the form has been submitted

When typing, we should dynamically change the style if the element was invalid on focus. Otherwise, we should keep the current style.

IOW, there are 3 situations:
1. you focus a field which hasn't :-moz-ui-invalid nor :-moz-ui-valid applied, in that case, one of them will apply on blur if the value has been changed ;
2. you focus a field which has :-moz-ui-valid applied, in that case one of the two pseudo-class will apply to reflect the new state on blur ;
3. you focus a field which has :-moz-ui-invalid applied, in that case the style will dynamically change will typing.

Let me know if you think we can go with that.
Comment on attachment 489210 [details] [diff] [review]
Part 2 - Make :-moz-ui-valid follow rules for :valid

Boris, feel free to ignore the review if it should take you too much time given that I didn't got a real feedback. However, I think my proposal should make sense and I will write the patches.
Attachment #489210 - Flags: review?(bzbarsky)
Lot of tests but very small changes in the code.
Attachment #489494 - Flags: review?(bzbarsky)
Comment on attachment 489210 [details] [diff] [review]
Part 2 - Make :-moz-ui-valid follow rules for :valid

r=me
Attachment #489210 - Flags: review?(bzbarsky) → review+
Comment on attachment 489494 [details] [diff] [review]
Part 3 - :-moz-ui-valid doesn't apply if the element hasn't been modified

r=me
Attachment #489494 - Flags: review?(bzbarsky) → review+
Comment on attachment 489503 [details] [diff] [review]
Part 4 - :-moz-ui-valid applies if the user tried to submit the form in an invalid state

r=me
Attachment #489503 - Flags: review?(bzbarsky) → review+
Attachment #488907 - Flags: approval2.0?
Attachment #489210 - Flags: approval2.0?
Attachment #489494 - Flags: approval2.0?
Attachment #489503 - Flags: approval2.0?
Attachment #489542 - Flags: approval2.0?
Comment on attachment 489542 [details] [diff] [review]
Part 5 - :-moz-ui-valid should not apply while typing if the element had no style when it was focused

>+      // If nor invalid UI nor valid UI is shown, we shouldn't show the valid
>+      // UI while typing.

  If neither ... nor ....

Maybe rename BF_DONT_SHOW_VALID_UI to BF_SUPPRESS_VALID_UI?

>+    // :-moz-ui-valid should never apply if when the element was focused,

Need comma before "when".  

>+    // 1. the element is valid, and

This "and" doesn't match the || in the code.  Which is correct?

>+    // 2. the element shouldn't show the invalid ui (and isn't valid).

I don't understand what this is trying to say.

It looks like whether we have NS_EVENT_STATE_MOZ_UI_VALID depends on the value of BF_CAN_SHOW_INVALID_UI.  When the latter changes, do we do a ContentStatesChanged for the former?

The multipart comment is really hard to read, as is the code that follows it.... (esp. because the second 1&2 pair doesn't map to an actual || in the code, as far as I can see).

>+   * Return if an element should show the valid UI.

s/if/whether/

ShouldShowValidUI might be better named as CanShowValidUI, no?
I tried to make things clearer by using the same name that I used for :-moz-ui-invalid and changed the comments.
Attachment #489542 - Attachment is obsolete: true
Attachment #489817 - Flags: review?(bzbarsky)
Attachment #489817 - Flags: approval2.0?
Attachment #489542 - Flags: review?(bzbarsky)
Attachment #489542 - Flags: approval2.0?
The comment for mCanShowValidUI looks like it needs updating.

I still don't understand the long -moz-ui-valid comments.  Please catch me on irc so we can talk about them?
With comments fixed.
Attachment #489817 - Attachment is obsolete: true
Attachment #490602 - Flags: review?(bzbarsky)
Attachment #490602 - Flags: approval2.0?
Attachment #489817 - Flags: review?(bzbarsky)
Attachment #489817 - Flags: approval2.0?
Comment on attachment 490602 [details] [diff] [review]
Part 5 - :-moz-ui-valid should not apply while typing if the element had no style when it was focused

>+    // :-moz-ui-valid applies if:

Needs "all of the following are true".

r=me with that fixed
Attachment #490602 - Flags: review?(bzbarsky) → review+
Depends on: 612730
Hard for me to test all these cases, but generally seems to be doing the right thing. Was asked for informal feedback, so here it is. Let's get it in a beta and get some feedback on it. :)
Depends on: 613249
Attachment #488907 - Flags: approval2.0? → approval2.0+
Attachment #489210 - Flags: approval2.0? → approval2.0+
Attachment #489494 - Flags: approval2.0? → approval2.0+
Attachment #489503 - Flags: approval2.0? → approval2.0+
Comment on attachment 490602 [details] [diff] [review]
Part 5 - :-moz-ui-valid should not apply while typing if the element had no style when it was focused

a=jst for this series of changes.
Attachment #490602 - Flags: approval2.0? → approval2.0+
Assigning to me for doc.
Assignee: mounir.lamouri → jswisher
Assignee: jswisher → mounir.lamouri
You need to log in before you can comment on or make changes to this bug.