Bug 8104 – REGRESSION (NativeTextField): New text fields should not allow pasting newlines

• Status: VERIFIED
• Resolution: FIXED
• Component: Forms

New text fields allow you to paste text containing newlines, and then their height changes to accommodate the multi-line text.

Old behavior was to paste the text only up to the first newline.

[Adele Peterson]

*** Bug 8107 has been marked as a duplicate of this bug. ***

[Adele Peterson]

ok- i'm working on this now.

[Adele Peterson]

Created attachment 7429 [details]
initial patch

posting an initial patch...

I ran into a problem while testing a simpler version of this patch.  If you select all at cnn.com, and then go to paste that content into the text field, we ended up pasting in a newline at the end.  I realized that if we did a test rendering of the plain-text conversion first, and then sent the BeforeTextInserted event (which causes the truncation).

I don't like doing another test rendering in ReplaceSelectionCommand though.  So I'm trying to find a better solution.  Feel free to jump in w/ ideas :)

[Darin Adler]

(In reply to comment #3)
> I don't like doing another test rendering in ReplaceSelectionCommand though. 
> So I'm trying to find a better solution.  Feel free to jump in w/ ideas :)

Well, this version seems fine to me; we shouldn't worry unduly about the extra text rendering for now. But if you are doing this, you should move more lines inside the if statement. Specifically, these three don't need to be done twice if you aren't in the plain text case:

+    range->selectNodeContents(holder.get(), ec);
+    ASSERT(ec == 0);
+    text = plainText(range.get());

The use of min/max here is a bit confusing because of the special case for -1. You should probably just write it out like this:

        if (newLinePosition >= 0 && (truncateLength < 0 || truncateLength > newLinePosition))
            truncateLength = newLinePosition;
 
I think that's a little clearer. Another possibility would be to write a local function that is a version of min that handles the -1 value as a special case.

    static int minPosition(int p1, int p2)
    {
        if (p1 < 0)
            return p2;
        if (p2 < 0)
            return p1;
        return min(p1, p2);
    }

        truncateLength = minPosition(truncateLength, newLinePosition);

[Darin Adler]

(In reply to comment #4)
> I think that's a little clearer. Another possibility would be to write a local
> function that is a version of min that handles the -1 value as a special case.

Or you could initialize truncateLength to text.length() to simplify the code a bit. You're stuck with the "-1" for the find result, but not for your own code. You could even cover find with something that returns the string length instead of -1 when not finding anything.

[Maciej Stachowiak]

These are all text field regressions so they should all be P1.

[Adele Peterson]

With further testing, and talking w/ Justin, I don't think we actually need to change how ReplaceSelectionCommand works for this fix.

Justin reviewed a simpler patch that just does the truncation in HTMLInputElement.