[webkit-reviews] review denied: [Bug 44883] <input> maxLength is clamped to 524288 : [Attachment 109255] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied Antaryami Pandia
<xqb748 at motorola.com>'s request for review:
Bug 44883: <input> maxLength is clamped to 524288
https://bugs.webkit.org/show_bug.cgi?id=44883

Attachment 109255: Patch
https://bugs.webkit.org/attachment.cgi?id=109255&action=review

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=109255&action=review


What's the purpose of this patch? Do you want to remove the internal hard
limit, 524288, or want not to expose the internal hard limit via
HTMLInputELement::maxLength?


> Source/WebCore/html/TextFieldInputType.cpp:304
> -    return limitLength(proposedValue.removeCharacters(isASCIILineBreak),
HTMLInputElement::maximumLength);
> +    return limitLength(proposedValue.removeCharacters(isASCIILineBreak),
element()->maxLength());

No, this is wrong. We should not clamp the value by maxLength() here.

> LayoutTests/fast/forms/input-appearance-maxlength.html:18
> -if (input.value != "12345") {
> +if (input.value == "12345") {
>      testPassed('Maxlength shouldn\'t work for default value.\n');

This is wrong. We must not change this.

> LayoutTests/fast/forms/input-text-maxlength.html:20
> -<input type="text" size="5" value="12x&#x305;&#x332;45" maxlength="4"
id="input7">
> -<input type="text" size="5" maxlength="4" value="12x&#x305;&#x332;45"
id="input8">
> -<input type="text" id="j" size="5" maxlength="4" value="123">
> -<input type="text" id="i" size="5" maxlength="4" value="123">
> +<input type="text" size="5" value="12x&#x305;&#x332;45" maxlength="10"
id="input7">
> +<input type="text" size="5" maxlength="10" value="12x&#x305;&#x332;45"
id="input8">
> +<input type="text" id="j" size="5" maxlength="10" value="123">
> +<input type="text" id="i" size="5" maxlength="10" value="123">

We should not change this.

> LayoutTests/fast/forms/input-text-maxlength.html:38
> -shouldBe('domValueOf("input1")', '"12345"');
> +shouldBe('domValueOf("input1")', '"1234"');

We should not change this file.

> LayoutTests/fast/forms/input-text-paste-maxlength.html:17
> -<input type="text" id="i" size="5" maxlength="4">
> +<input type="text" id="i" size="5" maxlength="20">

We should not change this file.

> LayoutTests/fast/forms/script-tests/ValidityState-tooLong-input.js:13
> -shouldBe('input.value.length', '5');
> +shouldBe('input.value.length', '3');

We should not change this file.