Bug 38203 – Move number parsing code out of HTMLInputElement.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Yael]

Move HTMLInputElement::parseToDoubleForNumberType and HTMLInputElement::serializeForNumberType to HTMLElement, so that other elements can reuse this code.
This was suggested in https://bugs.webkit.org/show_bug.cgi?id=38140#c12.
A patch is coming soon.

[Yael]

Created attachment 54432 [details]
Patch.

This patch is simply moving the number handling methods from HTMLInputElement to HTMLElement.

[Darin Adler]

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.

[Yael]

Created attachment 54493 [details]
Patch

Moved HTMLInputElement::parseToDoubleForNumberType to String::toDoubleStrict .

[Adam Roben (:aroben)]

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)

[Yael]

(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!

[Yael]

Created attachment 54620 [details]
Update the JavaScriptCore.def for the new function.

[Darin Adler]

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.

[Yael]

(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?

[Kent Tamura]

(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.

[Yael]

(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.

[Yael]

Created attachment 54750 [details]
Patch. Based on comment #9.

Moved the code to HTMLparser as suggested in comment #9.

[Yael]

Committed r58635: <http://trac.webkit.org/changeset/58635>