Bug 38203

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:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Bug Depends on:

Bug Blocks:

38140

Attachments:

Description	Flags
Patch.	darin: review-, darin: commit-queue-
Patch	none
Update the JavaScriptCore.def for the new function.	none
Patch. Based on comment #9.	darin: review+

Yael

Reported 2010-04-27 10:27:36 PDT

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.

Attachments
Patch. (6.24 KB, patch) 2010-04-27 11:00 PDT, Yael	darin: review- darin: commit-queue-	Details Formatted Diff Diff
Patch (9.47 KB, patch) 2010-04-27 18:29 PDT, Yael	no flags	Details Formatted Diff Diff
Update the JavaScriptCore.def for the new function. (10.19 KB, patch) 2010-04-28 14:23 PDT, Yael	no flags	Details Formatted Diff Diff
Patch. Based on comment #9. (9.49 KB, patch) 2010-04-29 16:08 PDT, Yael	darin: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Yael

Comment 1 2010-04-27 11:00:17 PDT

Created attachment 54432 [details] Patch. This patch is simply moving the number handling methods from HTMLInputElement to HTMLElement.

Darin Adler

Comment 2 2010-04-27 12:42:02 PDT

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

Comment 3 2010-04-27 18:29:44 PDT

Created attachment 54493 [details] Patch Moved HTMLInputElement::parseToDoubleForNumberType to String::toDoubleStrict .

Adam Roben (:aroben)

Comment 4 2010-04-28 13:47:17 PDT

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

Comment 5 2010-04-28 14:22:22 PDT

(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

Comment 6 2010-04-28 14:23:49 PDT

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

Darin Adler

Comment 7 2010-04-28 16:14:49 PDT

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

Comment 8 2010-04-28 17:23:28 PDT

(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

Comment 9 2010-04-29 07:16:36 PDT

(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

Comment 10 2010-04-29 14:35:34 PDT

(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

Comment 11 2010-04-29 16:08:13 PDT

Created attachment 54750 [details] Patch. Based on comment #9. Moved the code to HTMLparser as suggested in comment #9.

Yael

Comment 12 2010-05-01 05:11:51 PDT

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

Note You need to log in before you can comment on or make changes to this bug.