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

Status: RESOLVED FIXED

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

Description

[Mounir Lamouri (:mounir)]

And even if maxlength is set, if the value is clearly wrong, we shouldn't get the text length.

Comment 1

[Mounir Lamouri (:mounir)]

This should probably block considering the possible perf win.

Comment 2

[Benjamin Smedberg]

I'll take a patch, but I don't think it blocks.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I think this needs to block.  To be clear, this is a performance regression from 3.6 due to the new validation code....

Comment 4

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

Comment 6

[Mounir Lamouri (:mounir)]

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

Comment 7

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/043d9e0a62d9