[HTML5][Forms] :valid/:invalid/:optional/:required CSS selectors should be applied lively

[Kent Tamura]

:valid, :invalid, :optional and :required CSS selectors should be applied immediately when an element's
  * value,
  * constraints, or
  * willValidate
state is changed.  In the current WebKit, some of them apply selectors immediately and some of them don't.

[Kent Tamura]

Created attachment 38853 [details]
Proposed patch

[Kent Tamura]

Created attachment 38854 [details]
Proposed patch (rev.2)

[Eric Seidel (no email)]

Comment on attachment 38854 [details]
Proposed patch (rev.2)

I probably would have used a function to make the test output more readable:
+PASS document.defaultView.getComputedStyle(el, null).getPropertyValue("background-color") is "rgb(255, 0, 0)"

function backgroundOf(el)
{
    return document.defaultView.getComputedStyle(el, null).getPropertyValue("background-color");
}

PASS backgroundOf(el) is "rgb(255, 0, 0)"

or maybe even:

PASS backgroundOf(el) is "red"

Slightly sad to see custom edited html files instead of just using .js files and the standard TEMPLATE.html, but these are fine. :)

Those comments above are just nits, not requiring any change.  I'd need to stare at the c++ part of this more to make a real review decision...

[Kent Tamura]

Created attachment 39168 [details]
Proposed patch (rev.3)

Follows the Eric's advice.
 - Separate test HTMLs and JavaScript code
 - Visible text changes in the tests

[Kent Tamura]

Note that we don"t need to update the patch rev.3 for the resoruces -> script-tests change because the tests uses not only .js but also .css.

[Eric Seidel (no email)]

Comment on attachment 39168 [details]
Proposed patch (rev.3)

Looks fine to me.  Sad this has been sitting in the queue for 2 weeks w/o comment. :(

50     if (textArea->willValidate())
 51         // Need to sync the value in order to apply :valid/:invalid selectors.
 52         textArea->updateValue();

Needs {} since the comment makes it multi-line.  There are 2 instances of that.

If updateValue() isn't const anymore, why is this const_cast needed?
 297     const_cast<HTMLTextAreaElement*>(this)->updateValue();

I'm not sure I'm the best person to review this change, but the others have been silent.  It looks OK to me and looks non-harmful.  I'll r+ it, but it can't be auto-committed as is, so you'll have to commit this yourself, find someone to commit it, or post a new patch with modifications for the commit-queue.

[Kent Tamura]

Created attachment 40029 [details]
Proposed patch (rev.4)


> Needs {} since the comment makes it multi-line.  There are 2 instances of that.

Fixed 2 instances, and removed {} for single statement at another place.

> If updateValue() isn't const anymore, why is this const_cast needed?
>  297     const_cast<HTMLTextAreaElement*>(this)->updateValue();

In order to nuke "const" of "this".  This method is const and updateValue() is not const.

[Kent Tamura]

Created attachment 40215 [details]
Proposed patch (rev.5)

Resolved a patch conflict with today's WebKit source.

[Kent Tamura]

Created attachment 40439 [details]
Proposed patch (rev.6)

- Resolved a conflict with r48959.
- Added more explanation to ChangeLog.

[Kent Tamura]

Created attachment 40487 [details]
Proposed patch (rev.7)

Oops, ChangeLog in the previous patch was old.

[Eric Seidel (no email)]

Comment on attachment 39168 [details]
Proposed patch (rev.3)

Clearing r+ on this obsolete patch.

[Eric Seidel (no email)]

Comment on attachment 40487 [details]
Proposed patch (rev.7)

Looks OK.

[WebKit Commit Bot]

Comment on attachment 40487 [details]
Proposed patch (rev.7)

Clearing flags on attachment: 40487

Committed r49105: <http://trac.webkit.org/changeset/49105>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.