Bug 171745 – WebKit should default to using sRGB with NSColor conversion instead of device color space

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Beth Dakin]

WebKit should maintain colorspaces that it understands on NSColors

We need this in order to get the highlight ring on the TouchBar color picker to work. 

rdar://problem/28314183

[Beth Dakin]

Created attachment 309201 [details]
Patch

[Tim Horton]

Comment on attachment 309201 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=309201&action=review

> Source/WebCore/platform/graphics/mac/ColorMac.mm:64
> +    if (color.colorSpace == [NSColorSpace sRGBColorSpace])

Please use isEqual.

[Simon Fraser (smfr)]

Comment on attachment 309201 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=309201&action=review

> Source/WebCore/platform/graphics/mac/ColorMac.mm:64
> +    if (color.colorSpace == [NSColorSpace sRGBColorSpace])

This should use isEqual(To?): right?

> Source/WebCore/platform/graphics/mac/ColorMac.mm:67
> +        rgbColor = [color colorUsingColorSpaceName:NSDeviceRGBColorSpace];

Is this equivalent to NSCalibratedRGBColorSpace and which is sRGB?

[Beth Dakin]

(In reply to Simon Fraser (smfr) from comment #3)
> Comment on attachment 309201 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309201&action=review
> 
> > Source/WebCore/platform/graphics/mac/ColorMac.mm:64
> > +    if (color.colorSpace == [NSColorSpace sRGBColorSpace])
> 
> This should use isEqual(To?): right?
> 

Yup, fixing.

> > Source/WebCore/platform/graphics/mac/ColorMac.mm:67
> > +        rgbColor = [color colorUsingColorSpaceName:NSDeviceRGBColorSpace];
> 
> Is this equivalent to NSCalibratedRGBColorSpace and which is sRGB?

I'm not sure. This is what we have always done in this function before this patch.

[Simon Fraser (smfr)]

Comment on attachment 309201 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=309201&action=review

>>> Source/WebCore/platform/graphics/mac/ColorMac.mm:67
>>> +        rgbColor = [color colorUsingColorSpaceName:NSDeviceRGBColorSpace];
>> 
>> Is this equivalent to NSCalibratedRGBColorSpace and which is sRGB?
> 
> I'm not sure. This is what we have always done in this function before this patch.

Let's use colorUsingColorSpace:[NSColorSpace sRGBColorSpace]

[Beth Dakin]

Created attachment 309206 [details]
Patch

[Build Bot]

Comment on attachment 309206 [details]
Patch

Attachment 309206 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3681199

New failing tests:
editing/mac/attributed-string/comment-cdata-section.html
editing/mac/attributed-string/font-style-variant-effect.html
editing/mac/attributed-string/font-weight.html
editing/mac/attributed-string/letter-spacing.html
editing/mac/attributed-string/vertical-align.html
editing/mac/attributed-string/text-decorations.html
editing/mac/attributed-string/font-size.html
editing/mac/attributed-string/anchor-element.html
editing/mac/attributed-string/basic.html

[Build Bot]

Created attachment 309221 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Tim Horton]

I think those can be safely rebaselined.

[Beth Dakin]

Created attachment 309236 [details]
Patch with new test results

For the bots to try.

[Build Bot]

Comment on attachment 309236 [details]
Patch with new test results

Attachment 309236 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/3681987

New failing tests:
editing/mac/attributed-string/font-style-variant-effect.html

[Build Bot]

Created attachment 309246 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Beth Dakin]

https://trac.webkit.org/changeset/216449/webkit