[webkit-reviews] review requested: [Bug 6893] REGRESSION: Major bug with TinyMCE, no value submitted from textarea : [Attachment 6767] patch

[bugzilla-request-daemon at opendarwin.org]

Justin Garcia <justin.garcia at apple.com> has asked  for review:
Bug 6893: REGRESSION: Major bug with TinyMCE, no value submitted from textarea
http://bugzilla.opendarwin.org/show_bug.cgi?id=6893

Attachment 6767: patch
http://bugzilla.opendarwin.org/attachment.cgi?id=6767&action=edit

------- Additional Comments from Justin Garcia <justin.garcia at apple.com>
(In reply to comment #14)
> 
> Comment says that turning \r into \n is "just for good measure". But isn't it

> important to do it for form uploading so that all line breaks are the same?

No, when we're about to submit form data, we turn all 3 line endings into \r\n
as per the w3c spec.  I changed the comment to "we turn \r\ns into \ns so that
a line ending is always a single character, and we turn \rs into \ns to
simplify code that finds paragraph boundaries."

> +	   [string replaceOccurrencesOfString:@"\r" withString:@"\n"
> options:NSLiteralSearch range:range];
> +	   [string replaceOccurrencesOfString:@"\r\n" withString:@"\n"
> options:NSLiteralSearch range:range];
> 
> The above code will change \r\n into \n\n, right? We need to do the \r\n case

> first.

Oops, I don't know why I swapped the order, baad mistake.

> It's fragile to count on the fact that rangeToNormalize is still a good range

> -- if we're off by a character we'll get an Objective-C exception. Are we
> certain the text field can't change between where rangeToNormalize is set and

> where it's used?

I'm not certain.  Instead of saving rangeToNormalize, I just set a bool if the
replacementString contains \rs, and then I consider the entire textarea as
possibly having \rs in textDidChange.

I added two tests, one for this bug and another for <rdar://problem/3968059>
Textarea with hard-wrap: pre-filled text doesn't get hard-wrapped.