Summary: | Move number parsing code out of HTMLInputElement. | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yael <yael> | ||||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aroben, tkent | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 38140 | ||||||||||||
Attachments: |
|
Description
Yael
2010-04-27 10:27:36 PDT
Created attachment 54432 [details]
Patch.
This patch is simply moving the number handling methods from HTMLInputElement to HTMLElement.
Comment on attachment 54432 [details]
Patch.
If we're going to move these, I suggest we make them non-member functions. I see no reason for these to be members of any DOM class.
Created attachment 54493 [details]
Patch
Moved HTMLInputElement::parseToDoubleForNumberType to String::toDoubleStrict .
Here are the errors on Windows: 13>WebCore.lib(HTMLElementsAllInOne.obj) : error LNK2019: unresolved external symbol "public: double __thiscall WebCore::String::toDoubleStrict(bool *)const " (?toDoubleStrict@String@WebCore@@QBENPA_N@Z) referenced in function "public: bool __thiscall WebCore::HTMLInputElement::stepMismatch(void)const " (?stepMismatch@HTMLInputElement@WebCore@@QBE_NXZ) 13>WebCore.lib(ValidityState.obj) : error LNK2001: unresolved external symbol "public: double __thiscall WebCore::String::toDoubleStrict(bool *)const " (?toDoubleStrict@String@WebCore@@QBENPA_N@Z) 13>WebCore.lib(StepRange.obj) : error LNK2001: unresolved external symbol "public: double __thiscall WebCore::String::toDoubleStrict(bool *)const " (?toDoubleStrict@String@WebCore@@QBENPA_N@Z) (In reply to comment #4) > Here are the errors on Windows: > > 13>WebCore.lib(HTMLElementsAllInOne.obj) : error LNK2019: unresolved external > symbol "public: double __thiscall WebCore::String::toDoubleStrict(bool *)const > " (?toDoubleStrict@String@WebCore@@QBENPA_N@Z) referenced in function "public: > bool __thiscall WebCore::HTMLInputElement::stepMismatch(void)const " > (?stepMismatch@HTMLInputElement@WebCore@@QBE_NXZ) > 13>WebCore.lib(ValidityState.obj) : error LNK2001: unresolved external symbol > "public: double __thiscall WebCore::String::toDoubleStrict(bool *)const " > (?toDoubleStrict@String@WebCore@@QBENPA_N@Z) > 13>WebCore.lib(StepRange.obj) : error LNK2001: unresolved external symbol > "public: double __thiscall WebCore::String::toDoubleStrict(bool *)const " > (?toDoubleStrict@String@WebCore@@QBENPA_N@Z) Thank you for your help, Adam! Created attachment 54620 [details]
Update the JavaScriptCore.def for the new function.
Comment on attachment 54620 [details]
Update the JavaScriptCore.def for the new function.
I don't think that parsing a number in a way that is right for HTML is something that necessarily belongs in WTF. The particular strictness here is tied to HTML semantics. I think that having this be somewhere inside the WebCore project makes sense. I just don't think it should be a member function of a class. Not of any class, including String.
(In reply to comment #7) > (From update of attachment 54620 [details]) > I don't think that parsing a number in a way that is right for HTML is > something that necessarily belongs in WTF. The particular strictness here is > tied to HTML semantics. I think that having this be somewhere inside the > WebCore project makes sense. I just don't think it should be a member function > of a class. Not of any class, including String. Thank you for the review. I am thinking that the best place would probably be in HTMLElement, but not as a member. Would you agree? (In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 54620 [details] [details]) > > I don't think that parsing a number in a way that is right for HTML is > > something that necessarily belongs in WTF. The particular strictness here is > > tied to HTML semantics. I think that having this be somewhere inside the > > WebCore project makes sense. I just don't think it should be a member function > > of a class. Not of any class, including String. > > Thank you for the review. > I am thinking that the best place would probably be in HTMLElement, but not as > a member. > Would you agree? Moving to HTMLParser might be another choice, but it would be curious because we should move serializeForNumberType(double) too. (In reply to comment #9) > Moving to HTMLParser might be another choice, but it would be curious because > we should move serializeForNumberType(double) too. Thanks Kent, I should move both methods to the HTMLParser. Created attachment 54750 [details] Patch. Based on comment #9. Moved the code to HTMLparser as suggested in comment #9. Committed r58635: <http://trac.webkit.org/changeset/58635> |