Closed Bug 596496 Opened 12 years ago Closed 12 years ago

IsTooLong (nsHTMLInputELement and nsHTMLTextAreaElement) shouldn't compare maxlength and textlength if @maxlength isn't set

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b7
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: mounir, Assigned: mounir)

References

Details

(Keywords: perf)

Attachments

(1 file)

And even if maxlength is set, if the value is clearly wrong, we shouldn't get the text length.
This should probably block considering the possible perf win.
blocking2.0: --- → ?
I'll take a patch, but I don't think it blocks.
blocking2.0: ? → -
Whiteboard: [mounir-g2.0]
I think this needs to block.  To be clear, this is a performance regression from 3.6 due to the new validation code....
blocking2.0: - → ?
Attached patch Patch v1Splinter Review
Tested with this testcase: https://bugzilla.mozilla.org/attachment.cgi?id=476164

For 2000/2000, I moved from 10 seconds to 5 seconds. But it's a debug build so it might vary on a release.

(By the way, in the patch, I changed the check from maxLength < 0 to maxLength == -1 because I think it's safer considering GetMaxLength() should return -1 if there is a problem. If that's not the case, by changing the condition, that will break a lot of tests which is better so we do not hide a deeper problem too long...)
Attachment #477816 - Flags: review?(bzbarsky)
Whiteboard: [mounir-g2.0]
blocking2.0: ? → betaN+
Status: NEW → ASSIGNED
Comment on attachment 477816 [details] [diff] [review]
Patch v1

s/comapre/compare/ in the checkin comment and looks good.  Though it'd be really nice if we could just get the text length quickly instead of what we have to do now....
Attachment #477816 - Flags: review?(bzbarsky) → review+
(In reply to comment #5)
> Comment on attachment 477816 [details] [diff] [review]
> Patch v1
> 
> s/comapre/compare/ in the checkin comment and looks good.  Though it'd be
> really nice if we could just get the text length quickly instead of what we
> have to do now....

The text length is used only for maxlength in nsHTMLInputElement.cpp so I doesn't sound to be critical. To get the text length quickly, we can optimize string's Length() method or cache the text length when setting the value, right? I guess we can also try to have faster GetValue() or even optimize for cases where length=0 (there is bug 585100 for that).
Pushed:
http://hg.mozilla.org/mozilla-central/rev/043d9e0a62d9
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Target Milestone: mozilla2.0b8 → mozilla2.0b7
You need to log in before you can comment on or make changes to this bug.