[webkit-reviews] review denied: [Bug 86058] [Forms] Move ValidityState methods implementation to another place : [Attachment 141310] Patch 7

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied yosin at chromium.org's request for
review:
Bug 86058: [Forms] Move ValidityState methods implementation to another place
https://bugs.webkit.org/show_bug.cgi?id=86058

Attachment 141310: Patch 7
https://bugs.webkit.org/attachment.cgi?id=141310&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=141310&action=review


> Source/WebCore/ChangeLog:91
> +	   * html/FormAssociatedElement.cpp:
> +	   (WebCore::FormAssociatedElement::formAttributeChanged):
> +	   (WebCore::FormAssociatedElement::customError):
> +	   (WebCore):
> +	   (WebCore::FormAssociatedElement::patternMismatch):
> +	   (WebCore::FormAssociatedElement::rangeOverflow):
> +	   (WebCore::FormAssociatedElement::rangeUnderflow):
> +	   (WebCore::FormAssociatedElement::stepMismatch):
> +	   (WebCore::FormAssociatedElement::tooLong):
> +	   (WebCore::FormAssociatedElement::typeMismatch):
> +	   (WebCore::FormAssociatedElement::valid):
> +	   (WebCore::FormAssociatedElement::valueMissing):
> +	   (WebCore::FormAssociatedElement::customValidationMessage):
> +	   (WebCore::FormAssociatedElement::validationMessage):
> +	   (WebCore::FormAssociatedElement::setCustomValidity):
> +	   * html/FormAssociatedElement.h:
> +	   (FormAssociatedElement):
> +	   * html/HTMLFormControlElement.cpp:
> +	   (WebCore::HTMLFormControlElement::setCustomValidity):
> +	   * html/HTMLFormControlElement.h:
> +	   (HTMLFormControlElement):
> +	   * html/HTMLInputElement.cpp:
> +	   (WebCore::HTMLInputElement::isValidValue):
> +	   (WebCore::HTMLInputElement::tooLong):
> +	   (WebCore):
> +	   (WebCore::HTMLInputElement::typeMismatch):
> +	   (WebCore::HTMLInputElement::valueMissing):
> +	   (WebCore::HTMLInputElement::patternMismatch):
> +	   (WebCore::HTMLInputElement::rangeUnderflow):
> +	   (WebCore::HTMLInputElement::rangeOverflow):
> +	   (WebCore::HTMLInputElement::validationMessage):
> +	   (WebCore::HTMLInputElement::minimum):
> +	   (WebCore::HTMLInputElement::maximum):
> +	   (WebCore::HTMLInputElement::stepMismatch):
> +	   (WebCore::HTMLInputElement::isInRange):
> +	   (WebCore::HTMLInputElement::isOutOfRange):
> +	   * html/HTMLInputElement.h:
> +	   (HTMLInputElement):
> +	   * html/HTMLObjectElement.h:
> +	   * html/HTMLSelectElement.cpp:
> +	   (WebCore::HTMLSelectElement::validationMessage):
> +	   (WebCore):
> +	   (WebCore::HTMLSelectElement::valueMissing):
> +	   * html/HTMLSelectElement.h:
> +	   (HTMLSelectElement):
> +	   * html/HTMLTextAreaElement.cpp:
> +	   (WebCore::HTMLTextAreaElement::validationMessage):
> +	   (WebCore):
> +	   (WebCore::HTMLTextAreaElement::valueMissing):
> +	   (WebCore::HTMLTextAreaElement::tooLong):
> +	   * html/HTMLTextAreaElement.h:
> +	   (HTMLTextAreaElement):
> +	   * html/InputType.cpp:
> +	   (WebCore::InputType::stepMismatch):
> +	   (WebCore):
> +	   (WebCore::InputType::alignValueForStep):
> +	   (WebCore::InputType::stepUpFromRenderer):
> +	   (WebCore::InputType::validationMessage):
> +	   * html/InputType.h:
> +	   (InputType):
> +	   * html/ValidityState.cpp:
> +	   (WebCore::ValidityState::validationMessage):
> +	   (WebCore::ValidityState::valueMissing):
> +	   (WebCore::ValidityState::typeMismatch):
> +	   (WebCore::ValidityState::patternMismatch):
> +	   (WebCore::ValidityState::tooLong):
> +	   (WebCore::ValidityState::rangeUnderflow):
> +	   (WebCore::ValidityState::rangeOverflow):
> +	   (WebCore::ValidityState::stepMismatch):
> +	   (WebCore::ValidityState::customError):
> +	   (WebCore::ValidityState::valid):
> +	   * html/ValidityState.h:
> +	   (ValidityState):

You had better write what you changed for each of functions.

> Source/WebCore/html/HTMLInputElement.h:51
>      bool tooLong(const String&, NeedsToCheckDirtyFlag) const;

I think we can make this private.

> Source/WebCore/html/HTMLInputElement.h:60
> +    // valueMissing() ignores the specified string value for CHECKBOX and
RADIO.
> +    virtual bool valueMissing() const OVERRIDE;

This comment is stale.	We should remove it.

> Source/WebCore/html/HTMLTextAreaElement.h:55
>      bool valueMissing(const String& value) const { return
isRequiredFormControl() && !disabled() && !readOnly() && value.isEmpty(); }
>      bool tooLong(const String&, NeedsToCheckDirtyFlag) const;

We should make them private.