[webkit-reviews] review granted: [Bug 58043] Unify the way we generate HTML for an image in the Clipboard : [Attachment 103408] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 58043: Unify the way we generate HTML for an image in the Clipboard
https://bugs.webkit.org/show_bug.cgi?id=58043

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=103408&action=review


Yay! More code sharing between the ports.

> LayoutTests/editing/pasteboard/drag-and-drop-image-contenteditable.html:13
> +

Nit: why do we have these blank lines after </head>?

> LayoutTests/editing/pasteboard/drag-and-drop-image-contenteditable.html:22
> +jsTestIsAsync = true;

Nit: missing var.

> LayoutTests/editing/pasteboard/drag-and-drop-image-contenteditable.html:36
> +function runTest() {

Nit: maybe a blank line before function runTest()?

>
LayoutTests/editing/pasteboard/drag-and-drop-inputimage-contenteditable.html:13

> +

Nit: space.

>
LayoutTests/editing/pasteboard/drag-and-drop-inputimage-contenteditable.html:22

> +jsTestIsAsync = true;

Nit: missing var.

>
LayoutTests/editing/pasteboard/drag-and-drop-inputimage-contenteditable.html:37

> +function runTest() {

Nit: blank line here.

>
LayoutTests/editing/pasteboard/drag-and-drop-objectimage-contenteditable.html:1
3
> +

Nit: ditto.

>
LayoutTests/editing/pasteboard/drag-and-drop-objectimage-contenteditable.html:2
2
> +jsTestIsAsync = true;

Nit: ditto.

>
LayoutTests/editing/pasteboard/drag-and-drop-objectimage-contenteditable.html:3
7
> +function runTest() {

Nit: ditto.

> Source/WebCore/editing/MarkupAccumulator.cpp:126
> +

Missing break.

> Source/WebCore/editing/MarkupAccumulator.cpp:391
> +	   appendQuotedURLAttributeValue(out, resolveURLIfNeeded(element,
attribute.value()));

Maybe we can merge appendQuotedURLAttributeValue and resolveURLIfNeeded in a
follow up patch?