This is part of re-factoring of HTMLInputElement class for Steppable input type. The objective of this change is 1. Limit scope of Steppable input type related methods, maximumString, minimumString and stepString. 2. Dispatching by virtual method rather than type checking.
Created attachment 141119 [details] Patch 1
Created attachment 141122 [details] Patch 2
Created attachment 141124 [details] Patch 3
Created attachment 141125 [details] Patch 4
Created attachment 141126 [details] Patch 5
Created attachment 141127 [details] Patch 6
Created attachment 141310 [details] Patch 7
Comment on attachment 141310 [details] Patch 7 It seems cr-linux is too busy. Could you review changes for style and design? Thanks in advance.
Comment on attachment 141310 [details] Patch 7 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.
Created attachment 141337 [details] Patch 8
Created attachment 141339 [details] Patch 9
Created attachment 141343 [details] Patch 10
Comment on attachment 141343 [details] Patch 10 Thanks for review. I updated this patch as you suggested. Failure of EWS-WIN doesn't relate to my patch, I copy/paste error message at botton. Thanks in advance. ..\WebProcess\WebCoreSupport\win\WebPopupMenuWin.cpp(122) : error C2664: 'WebCore::TextRun::TextRun(const UChar *,int,float,float,WebCore::TextRun::ExpansionBehavior,WebCore::TextDirection,bool,bool,WebCore::TextRun::RoundingHacks)' : cannot convert parameter 6 from 'WebCore::TextRun::ExpansionBehaviorFlags' to 'WebCore::TextDirection' 7> Conversion to enumeration type requires an explicit cast (static_cast, C-style cast or function-style cast)
Comment on attachment 141343 [details] Patch 10 View in context: https://bugs.webkit.org/attachment.cgi?id=141343&action=review > Source/WebCore/html/InputType.h:160 > + virtual bool stepMismatch(const String&) const; We don't need to make this virtual, right?
Comment on attachment 141343 [details] Patch 10 View in context: https://bugs.webkit.org/attachment.cgi?id=141343&action=review > Source/WebCore/html/FormAssociatedElement.h:64 > + virtual bool customError() const; We don't need to make this virtual. > Source/WebCore/html/FormAssociatedElement.h:70 > + virtual bool patternMismatch() const; > + virtual bool rangeOverflow() const; > + virtual bool rangeUnderflow() const; > + virtual bool stepMismatch() const; > + virtual bool tooLong() const; > + virtual bool typeMismatch() const; We should add a comment that an override function must call willValidate(). > Source/WebCore/html/FormAssociatedElement.h:71 > + virtual bool valid() const; We don't need to make this virtual. > Source/WebCore/html/FormAssociatedElement.h:72 > + virtual bool valueMissing() const; We should add a comment that an override function must call willValidate().
Created attachment 141366 [details] Patch 11
Comment on attachment 141366 [details] Patch 11 Thanks for review. I updated comments in FormAssociateElement.h as you suggested. Local build was succeeded and changes at this time is comment only. So, EWS build should be passed. Please review this patch. Thanks in advance!
Comment on attachment 141366 [details] Patch 11 ok
Comment on attachment 141366 [details] Patch 11 Clearing flags on attachment: 141366 Committed r116752: <http://trac.webkit.org/changeset/116752>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by 86201
(In reply to comment #19) > (From update of attachment 141366 [details]) > Clearing flags on attachment: 141366 > > Committed r116752: <http://trac.webkit.org/changeset/116752> roll out because of Chromium-mac build errors. In file included from third_party/WebKit/Source/WebCore/html/HTMLFormControlElementWithState.h:27: third_party/WebKit/Source/WebCore/html/HTMLFormControlElement.h:98:12:error: 'WebCore::HTMLFormControlElement::validationMessage' hides overloaded virtual function [-Werror,-Woverloaded-virtual] String validationMessage(); ^ third_party/WebKit/Source/WebCore/html/FormAssociatedElement.h:75:20: note: hidden overloaded virtual function 'WebCore::FormAssociatedElement::validationMessage' declared here virtual String validationMessage() const; ^
Created attachment 141627 [details] Patch 12 - Fix virtual/override issues
Comment on attachment 141627 [details] Patch 12 - Fix virtual/override issues Build locally with Chromium-Mac/clang to make sure virtual/override declaration correct. Changes are in HTMLFormControlElement.{cpp,h}, HTMLObjectElement.h. Please review again. Thanks in advance.
Comment on attachment 141627 [details] Patch 12 - Fix virtual/override issues Clearing flags on attachment: 141627 Committed r116915: <http://trac.webkit.org/changeset/116915>