[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 #85907|review?                     |review-
               Flag|                            |




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

> LayoutTests/ChangeLog:9
> +        (createFocusedTextAreaWithMaxLength3): Added to make sure consecutive insertbreaks work correctly for textarea maxlength.

Please split this into two lines.  It's way too long to fit in one line.

> Source/WebCore/editing/TypingCommand.cpp:50
> +// Helper function to check if we can insert new line feed with \n(line feed) string.

This comment seems to repeat what code does.  In general, we'd like to make code self-explanatory and add as little comment as possible.

> Source/WebCore/editing/TypingCommand.cpp:51
> +static bool canAppendNewLineFeed(TypingCommand& typingCommand)

It's uncommon for a function to take a reference to an EditCommand like this because EditCommand is ref-counted.

> Source/WebCore/editing/TypingCommand.cpp:416
> +    if (!canAppendNewLineFeed(*this))
> +        return;

I'd pass typingCommand.endingSelection() or the root editable element instead.

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