Bug 13328 – Code cleanup in image pasteboard code

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Andrew Wellington]

The soon-to-be-attached patch addresses some comments from aroben and jgarcia relating to the fix for http://bugs.webkit.org/show_bug.cgi?id=12959

- Consolidate Pasteboard::writeImage(const HitTestResult& result) and Pasteboard::writeImage(Node* imageNode, const KURL& url)
- Create frameContainsCopyableImageDocument in Editor.cpp to create more readable code in Editor::canCopy and Editor::copy

[Andrew Wellington]

Created attachment 14007 [details]
Proposed patch

[Darin Adler]

Comment on attachment 14007 [details]
Proposed patch

+    if (frame->document() && frame->document()->isImageDocument()) {
+        Document* doc = frame->document();

If you're going to have a local variable here, then I suggest declaring it outside the if statement.

Further, I think this function would read better with early exits and a couple more local variables.

It also might be good to make the function return the image element instead of a boolean, allowing us to share slightly more code.

Otherwise looks great. I'm not going to say review+ right now, but I'll probably look back later today.

[Andrew Wellington]

Comment on attachment 14007 [details]
Proposed patch

r20846 by Adam Roben addresses this issue

[Adam Roben (:aroben)]

(In reply to comment #3)
> (From update of attachment 14007 [details] [edit])
> r20846 by Adam Roben addresses this issue
> 

Sorry, I didn't see that this bug had been filed! At least now we know we were thinking along the same lines :-)