Bug 22300 – Use of NSCalibratedRGBColorSpace for CSS system colors causes layout tests to fail due to screen calibration

• Component: CSS
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

The following layout tests:

fast/forms/disabled-select-change-index.html
fast/forms/file-input-disabled.html
fast/forms/input-text-maxlength.html
fast/forms/input-text-paste-maxlength.html

always fail for me at home because the disabled text control color differs from the expected:

-            RenderButton {INPUT} at (0,0) size 78x18 [color=#808080] [bgcolor=#C0C0C0]
+            RenderButton {INPUT} at (0,0) size 78x18 [color=#6D6D6D] [bgcolor=#C0C0C0]

Deugging shows that for me, 
[[NSColor disabledControlTextColor] colorUsingColorSpaceName:NSCalibratedRGBColorSpace]

returns #6d6d6d, not #808080.

Maybe NSCalibratedRGBColorSpace is the wrong color space to use?

[Simon Fraser (smfr)]

This change was landed in https://bugs.webkit.org/show_bug.cgi?id=6129.

(In reply to comment #0)
> Maybe NSCalibratedRGBColorSpace is the wrong color space to use?

I think you're right.

[Pierre-Olivier Latour]

This failure is pretty easy to reproduce: just set your display profile to sRGB for instance.

Note that it won't reproduce if you run with --pixel as this temporarily changes the display profile to Generic RGB.

[Pierre-Olivier Latour]

I had a look at the webcore and webkit source, and it seems that:
1) CG colorspace are always device except for one case (in CreateCGColorIfDifferent())
2) AppKit colorspace can be device or calibrated and I don't see a clear pattern

[Pierre-Olivier Latour]

Created attachment 25329 [details]
Patch v1

The way to fix this issue and other potential ones is to ensure WebKit is always using Device RGB colorspace internally, which is mostly the case today.

Note that I replaced calls to +[NSColor whiteColor] as well since these return colors in calibrated colorspace.

I ran all layout tests with pixel tests enabled and disabled and no regressions were reported, except for platform/mac/fast/text/attributed-substring-from-range-001.html which needs to be updated.

Comment on attachment 25329 [details]
Patch v1

r=me

[Dean Jackson]

Committed r38671
	M	WebKit/mac/ChangeLog
	M	WebKit/mac/WebView/WebHTMLView.mm
	M	WebCore/ChangeLog
	M	WebCore/page/mac/AccessibilityObjectWrapper.mm
	M	WebCore/platform/graphics/mac/ColorMac.h
	M	WebCore/platform/graphics/mac/ColorMac.mm
	M	WebCore/rendering/RenderThemeMac.mm
	M	LayoutTests/platform/mac/fast/text/attributed-substring-from-range-001-expected.txt
	M	LayoutTests/ChangeLog