[Webkit-unassigned] [Bug 50617] ValidityState's exposed functions should check if willValidate() is true before all

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50617


Kent Tamura <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #76205|review?                     |review-
               Flag|                            |




--- Comment #15 from Kent Tamura <tkent at chromium.org>  2010-12-11 03:59:01 PST ---
(From update of attachment 76205)
View in context: https://bugs.webkit.org/attachment.cgi?id=76205&action=review

> LayoutTests/fast/forms/ValidityState-patternMismatch.html:66
> -/><input id="empty-pattern" type="text" pattern="" value="Lorem Ipsum"
> -/><input id="disabled" pattern="[0-9][A-Z]{3}" value="00AA" disabled />
> +
> +<input id="simple" pattern="[0-9][A-Z]{3}" value="0AAA"/>
> +
> +<input id="no-pattern-and-no-value" />
> +
> +<input id="ip-address" type="text" pattern="\b(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\b" value="127.0.0.1" />
> +<input id="email-address" type="text" pattern="[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?" value="someone at somewhere.com" />
> +<input id="wrong-email-address" type="text" pattern="[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?" value="Wrong!"/>
> +
> +<input id="match-1" type="text" pattern="\b(word1|word2|word3)(?:\W+\w+){1,6}?\W+(word1|word2|word3)\b" value="word1 near word2" />
> +<input id="match-2" type="text" pattern=".{5,}" value="12345" />
> +<input id="match-3" type="text" pattern=".{5,}" value="*(@$!" />
> +<input id="match-4" type="text" pattern=".{5,}" value="*(@^$!" />
> +<input id="match-5" type="text" pattern="0|1|2|3|4|5|6|7|8|" value="3" />
> +<input id="match-6" type="text" pattern="^(foo).*" value="foo"/>
> +<input id="match-7" type="text" pattern="^(foo).*" value="fooo"/>
> +<input id="match-8" type="text" pattern="\w" value="f"/>
> +<input id="match-9" type="text" pattern="\." value="."/>
> +<input id="match-10" type="text" pattern="\)\(" value=")("/>
> +<input id="match-11" type="text" pattern="ab" value="ab"/>
> +<input id="match-12" type="text" pattern="^ab" value="ab"/>
> +<input id="match-13" type="text" pattern="^\(" value="("/>
> +<input id="match-14" type="text" pattern="...\)\(..." value="aaa)(bbb"/>
> +<input id="match-15" type="text" pattern="^" value=""/>
> +<input id="match-16" type="text" pattern="$" value=""/>
> +<input id="match-17" type="text" pattern="foobar" value=""/>
> +
> +<input id="wrong-gray-or-grey" type="text" pattern="gr[ae]y" value="Wrong!"/>
> +<input id="gray" type="text" pattern="gr[ae]y" value="gray"/>
> +<input id="grey" type="text" pattern="gr[ae]y" value="grey"/>
> +<input id="empty-gray-or-grey" type="text" pattern="gr[ae]y" value=""/>
> +
> +<input id="mismatch-1" type="text" pattern="((4\.[0-3])|(2\.[0-3]))" value="Something 4.0"/>
> +<input id="mismatch-2" type="text" pattern="\w" value="*"/>
> +<input id="mismatch-3" type="email" pattern="[0-9]" value="something"/>
> +<input id="mismatch-4" type="text" pattern=".{5,}" value="1234" />
> +<input id="mismatch-5" type="text" pattern=".{5,}" value="*)$!" />
> +<input id="mismatch-6" type="text" pattern=".{5,}" value="(^$!" />
> +<input id="mismatch-7" type="text" pattern="0|1|2|3|4|5|6|7|8|" value="a" />
> +<input id="mismatch-8" type="text" pattern="^(foo).*" value="a foo"/>
> +<input id="mismatch-9" type="text" pattern="\w" value="134"/>
> +<input id="mismatch-10" type="text" pattern="\." value="\."/>
> +<input id="mismatch-11" type="text" pattern="\)\(" value=") ("/>
> +<input id="mismatch-12" type="text" pattern="ab" value="a"/>
> +<input id="mismatch-13" type="text" pattern="ab" value="b"/>
> +<input id="mismatch-14" type="text" pattern="^ab" value="abc"/>
> +<input id="mismatch-15" type="text" pattern="^\(" value="(something"/>
> +<input id="mismatch-16" type="text" pattern="\)\\" value="something)\"/>
> +<input id="mismatch-17" type="text" pattern="...\)\([b]{3}" value="adf)(bbbTEST"/>
> +<input id="mismatch-18" type="text" pattern="foo\\" value="food"/>
> +<input id="mismatch-19" type="text" pattern="^" value="wrong"/>
> +<input id="mismatch-20" type="text" pattern="$" value="wrong"/>
> +
> +<input id="empty-pattern" type="text" pattern="" value="Lorem Ipsum"/>
> +
> +<input id="disabled" pattern="[0-9][A-Z]{3}" value="00AA" disabled />
> +

You don't need to change this block, do you?

> WebCore/ChangeLog:9
> +        ValidityState's exposed functions should check if willValidate() is true before all
> +        https://bugs.webkit.org/show_bug.cgi?id=50617
> +
> +        Added checking willValidate() to exposed functions.
> +

We should mention the submit button behavior change.

> WebCore/ChangeLog:12
> +        (WebCore::HTMLButtonElement::parseMappedAttribute): Added calling setNeedsWillValidateCheck().
> +        (WebCore::HTMLButtonElement::recalcWillValidate): Added.

It's obvious you added them.  You should write why you needed to add them.

> WebCore/ChangeLog:15
> +        (WebCore::SubmitInputType::supportsValidation): Removed.

You should write the reason.

> WebCore/html/HTMLButtonElement.cpp:188
> +    return !disabled() && !readOnly();

We should use HTMLFormControlElement::recalcWillValidate().
  return m_type == SUBMIT && HTMLFormControlElement::recalcWillValidate();

> WebCore/html/ValidityState.cpp:209
> +    if (!element->willValidate())
> +        return false;
> +
> +    return !m_customErrorMessage.isEmpty();

nit: We can write "return element->willValidate() && !m_customErrorMessage.isEmpty();"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.