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

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 6893: REGRESSION: Major bug with TinyMCE, no value submitted from textarea
http://bugzilla.opendarwin.org/show_bug.cgi?id=6893

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

------- Additional Comments from Darin Adler <darin at apple.com>
In new code, please use String instead of DOMString. DOMString is a synonym
allowed for backward compatibility, but should not be used in anything new.

Since HTMLTextAreaElementImpl::attach and HTMLTextAreaElementImpl::detach no
longer do any special work and just call the base class, we should remove them
altogether.

Why change notification to aNotification in textDidChange?

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?

+	 [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.

Why add the blank lines after calling textDidChangeInTextArea?

Shouldn't we do the "rangeToNormalize" logic before calling
widget->textChanged()?

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?

Now that newlineSet is not a character set any more, maybe we can just use
plain old "find" calls instead of an NSScanner, but I suppose it's not
important if this code is going away.

Setting review- because of the "\r\n" after "\r" issue and normalizing before
calling textChanged() on the widget.