Bug 45376

Summary: Apply :invalid CSS class to <input type=number> with an unacceptable value
Product: WebKit Reporter: Kent Tamura <tkent>
Component: FormsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, adele, aestes, darin, dglazkov, eric, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 43973    
Bug Blocks:    
Attachments:
Description Flags
Patch (EWS failure expected) none

Kent Tamura
Reported 2010-09-08 02:31:28 PDT
Apply :invalid CSS class to <input type=number> with an unacceptable value
Attachments
Patch (EWS failure expected) (10.40 KB, patch)
2010-09-08 02:38 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Kent Tamura
Comment 1 2010-09-08 02:38:39 PDT
Created attachment 66861 [details] Patch (EWS failure expected)
Dimitri Glazkov (Google)
Comment 2 2010-09-08 07:53:31 PDT
Comment on attachment 66861 [details] Patch (EWS failure expected) View in context: https://bugs.webkit.org/attachment.cgi?id=66861&action=prettypatch Ok. > WebCore/dom/Element.h:284 > + virtual bool hasUnacceptableValue() const { return false; } Darin Adler taught me it would be better to have bodies of virtuals in the cpp files, even inlines. But for consistency with the other methods, I don't think it makes sense to ask you to redo this. > WebCore/html/HTMLInputElement.cpp:831 > + if (inputType() == NUMBER) { One day we will rework HTMLInputElement to eliminate all this inputType() == FOO cruft. But not today. > WebCore/rendering/RenderTextControlSingleLine.cpp:178 > + static_cast<HTMLInputElement*>(input)->setNeedsStyleRecalc(); And we'll remove this cruft too :)
Darin Adler
Comment 3 2010-09-08 08:20:45 PDT
(In reply to comment #2) > > WebCore/html/HTMLInputElement.cpp:831 > > + if (inputType() == NUMBER) { > One day we will rework HTMLInputElement to eliminate all this inputType() == FOO cruft. But not today. If we can afford one additional memory allocation per input element, and I think we can, all we need to do is to create a class for the input type and replace all the switch and if statements with virtual function calls on that object. Should be simple refactoring!
Kent Tamura
Comment 4 2010-09-08 20:28:19 PDT
(In reply to comment #3) > > > + if (inputType() == NUMBER) { > > One day we will rework HTMLInputElement to eliminate all this inputType() == FOO cruft. But not today. > > If we can afford one additional memory allocation per input element, and I think we can, all we need to do is to create a class for the input type and replace all the switch and if statements with virtual function calls on that object. Should be simple refactoring! I agree! I have wanted to do so since I started to work on Forms. I guess virtual method call is not slower than switch-case, and the object size would be smaller.
Kent Tamura
Comment 5 2010-09-09 22:06:55 PDT
Comment on attachment 66861 [details] Patch (EWS failure expected) Clearing flags on attachment: 66861 Committed r67166: <http://trac.webkit.org/changeset/67166>
Kent Tamura
Comment 6 2010-09-09 22:07:03 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 7 2010-09-09 22:12:59 PDT
http://trac.webkit.org/changeset/67166 might have broken Qt Linux Release minimal
Andy Estes
Comment 8 2010-09-24 16:06:09 PDT
It looks like this fix caused a regression related to input elements with placeholder text. See <https://bugs.webkit.org/show_bug.cgi?id=45940> for details.
Note You need to log in before you can comment on or make changes to this bug.