[Webkit-unassigned] [Bug 54443] Textarea maxlength doesn't account for newlines

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=54443


Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #85904|review?                     |review-
               Flag|                            |




--- Comment #27 from Ryosuke Niwa <rniwa at webkit.org>  2011-03-15 21:22:06 PST ---
(From update of attachment 85904)
View in context: https://bugs.webkit.org/attachment.cgi?id=85904&action=review

> LayoutTests/ChangeLog:5
> +        Textarea maxlength doesn't account for newlines

"doesn't" should be "doesn't"

> LayoutTests/fast/forms/script-tests/textarea-maxlength.js:52
> +    return textArea;

This line isn't necessary as textAre is declared in the global scope.

> LayoutTests/fast/forms/script-tests/textarea-maxlength.js:93
> +textArea = createFocusedTextAreaWithMaxLength3();

No need to update textArea manually here.

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

Given we already call createFocusedTextAreaWithMaxLength3, the comment is redundant.  Please get rid of it.

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

Ditto about the comment.

> Source/WebCore/ChangeLog:5
> +        Textarea maxlength doesn't account for newlines

Same comment about "'"

> Source/WebCore/ChangeLog:8
> +        When a user presses a return key, TypingCommand::insertLineBreak() is called. So before append a new line, check if we can add the new line.

This seems to be excessively long.  Please split the line at the end of the first sentence so that each line is ~150 columns.

> Source/WebCore/editing/TypingCommand.cpp:412
> +    // Check if we can insert new line break with \n(line feed) string.
> +    ExceptionCode ec = 0;
> +    RefPtr<BeforeTextInsertedEvent> event = BeforeTextInsertedEvent::create(String("\n"));
> +    endingSelection().rootEditableElement()->dispatchEvent(event, ec);
> +    if (!event->text().length())
> +        return;
> +

Now that I think about it, it seems odd that we don't do the same thing for insertParagraphSeparator (i.e. execCommand('InsertParagraph', false, null))

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.