[webkit-reviews] review denied: [Bug 40044] resolve urls in text/html clipboard data : [Attachment 57626] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied Tony Chang (Google)
<tony at chromium.org>'s request for review:
Bug 40044: resolve urls in text/html clipboard data
https://bugs.webkit.org/show_bug.cgi?id=40044

Attachment 57626: Patch
https://bugs.webkit.org/attachment.cgi?id=57626&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
LayoutTests/editing/pasteboard/copy-resolves-urls.html:16
 +	s.setBaseAndExtent(test, 0, test, 4);
Nit: a somewhat more readable way to do this:
s.selectAllChildren(test);

LayoutTests/http/tests/misc/copy-resolves-urls.html:16
 +	s.setBaseAndExtent(test, 0, test, 4);
ditto.

LayoutTests/ChangeLog:11
 +	    * editing/pasteboard/paste-noscript.html:  Updated to no longer
throw a JS exception so the results are the same
nice!

Here are the behaviors I saw with a quick test in different browsers:
1. IE: always keeps relative URLs relative
2. Safari 5.0 Mac / Chrome Windows / FF Windows: Depends on where you copy
from. If you copy from within a contentEditable, relative URLs stay relative,
otherwise they are resolved.
3. FF Mac: Always resolve URLs.

I wouldn't mind someone double-checking to make sure I didn't get that wrong.

> The other tricky bit about the Mac impl is that it only resolves URLs if the
page you're pasting into is a different URL from where you copied from.  I
don't think we can do that with text/html data because it doesn't keep track of
the base URL.  This makes testing a bit tricky (see the http test case).

Are you sure about this? I didn't see this in Safari 5.0 Mac. As in, copy-paste
from one contentEditable to another contentEditable kept things relative even
if they were on different pages.

Anyways, behavior (2) seems optimal. I think we should do that. Basically,
s/AbsoluteURLs/AbsoluteNonEditableURLs/. Otherwise, the code changes look good
to me.

Also, FWIW, I like the idea of never resolving file URLs.