[webkit-reviews] review granted: [Bug 7579] TinyMCE: Implement execCommand(insertImage, ...) : [Attachment 6931] patch

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 7579: TinyMCE: Implement execCommand(insertImage, ...)
http://bugzilla.opendarwin.org/show_bug.cgi?id=7579

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

------- Additional Comments from Darin Adler <darin at apple.com>
Is it correct to just append the value between two single quote characters?
Don't we have to do some escaping to handle special characters?

Do we need a local variable for baseURL? If it's just "", why not just pass ""
to the function?

Is "" the right thing to use for the base URL? How did you test that in other
browsers?

I guess the local variable for selectReplacement is a way to document what the
false is, but I'd probably just use "false" for brevity.

A note about the "user interface" variant: To implement that properly we'll
have to involve the WebView's delegate as we do for window.alert -- we never
want WebKit to put up a sheet or a dialog box without giving the application
control.

Overall looks great to me. I'll set review+ despite the special character
issue, but that's worth considering.