[Webkit-unassigned] [Bug 74775] WebKit editing throws exception when monochrome color dragged onto text

Wed Dec 21 11:25:28 PST 2011

https://bugs.webkit.org/show_bug.cgi?id=74775

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #120196|review?                     |review-
               Flag|                            |

--- Comment #9 from Ryosuke Niwa <rniwa at webkit.org>  2011-12-21 11:25:27 PST ---
(From update of attachment 120196)
View in context: https://bugs.webkit.org/attachment.cgi?id=120196&action=review

r- due to various nits.

> ChangeLog:3
> +        Add manual test covering failure to accept grayscale color drags to contentEditable regions.

This line should repeat what the bug summary.

> ManualTests/drag-color-to-contenteditable.html:3
> +<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
> +        "http://www.w3.org/TR/html4/strict.dtd">
> +<html lang="en">

Can we use HTML5 style DOCTYPE?
<!DOCTYPE html>
<html>

> ManualTests/drag-color-to-contenteditable.html:5
> +<head>
> +</head>

We don't need head.

> ManualTests/drag-color-to-contenteditable.html:11
> +<li>Open a color panel in some host app that facilitates doing so, such as TextEdit.app.</li>

I don't understand what you mean by "that facilitates doing so". I think it's better to say "Open a color panel in some app such as TextEdit.app."

> Source/WebCore/ChangeLog:4
> +        Handle non-RGB colorspace colors in the Mac platform drag manager. Fixes NSException thrown 
> +        when dragging monochrome colors to contentEditable regions.

This line should repeat what the bug summary. You can add a long description below "Reviewed by" line followed by a blank line.

> Source/WebCore/platform/mac/DragDataMac.mm:121
> +    if ([[color colorSpace] colorSpaceModel] != NSRGBColorSpaceModel) {
> +        color = [color colorUsingColorSpaceName:NSCalibratedRGBColorSpace];
> +    }

WebKit style is not to wrap single statement with { and }.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.