[webkit-reviews] review denied: [Bug 54443] Textarea maxlength doesn't account for newlines : [Attachment 85609] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Naoki Takano
<takano.naoki at gmail.com>'s request for review:
Bug 54443: Textarea maxlength doesn't account for newlines
https://bugs.webkit.org/show_bug.cgi?id=54443

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=85609&action=review

> LayoutTests/fast/forms/script-tests/textarea-maxlength.js:96
> +document.execCommand('insertLineBreak'); // This linebreak should be ignored
because limitation up to 3 characters.

You didn't realize about the bug I spotted below because your previous 3 calls
have instantiated an open TypingCommand.

> Source/WebCore/editing/TypingCommand.cpp:227
> +	       // Check if we can insert new line break with \n(line feed)
string.
> +	       ExceptionCode ec = 0;
> +	       RefPtr<BeforeTextInsertedEvent> evt =
BeforeTextInsertedEvent::create(String("\n"));
> +	       root->dispatchEvent(evt, ec);
> +
> +	       if (evt->text().length()) {
> +		  
lastTypingCommand->setShouldRetainAutocorrectionIndicator(options &
RetainAutocorrectionIndicator);
> +		   lastTypingCommand->insertLineBreak();
> +	       }

I don't think this is the right place to put this code.  I don't see any reason
this fix should be restricted to the line breaks inserted after the user had
typed something else (i.e. there is an open typing command).  r- because of
this.