Replace Color::getHSL() with sRGBToHSL to ensure it at least gives somewhat sensible results for ExtendedColors and reduce code duplication
Created attachment 399845 [details] Patch
Created attachment 399847 [details] Patch
Comment on attachment 399847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399847&action=review > Source/WebCore/editing/ReplaceSelectionCommand.cpp:550 > + return lightness(color.value().toSRGBAComponentsLossy()); Ideally we'd be able to compute lightness for any colorspace without converting to sRGB. Which makes me think that lightness() should stay as a function on Color. This will be especially true when we have LAB color where lightness is just one of the components. > Source/WebCore/editing/cocoa/DataDetection.mm:628 > + auto underlineColor = Color(makeRGBAFromHSLA(hue, saturation, overrideLightness, overrideAlphaMultiplier * alpha)); Make sure the hue is correctly round-tripped here. > Source/WebCore/page/FrameView.cpp:368 > + if (lightness(backgroundColor.toSRGBAComponentsLossy()) <= .5f && backgroundColor.isVisible()) Yeah, this would be much better with backgroundColor.getHSL.lightness().
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 399847 [details] > Patch > > Source/WebCore/page/FrameView.cpp:368 > > + if (lightness(backgroundColor.toSRGBAComponentsLossy()) <= .5f && backgroundColor.isVisible()) > > Yeah, this would be much better with backgroundColor.getHSL.lightness(). Would you be ok with backgroundColor.lightness()?
(In reply to Sam Weinig from comment #4) > (In reply to Simon Fraser (smfr) from comment #3) > > Comment on attachment 399847 [details] > > Patch > > > > Source/WebCore/page/FrameView.cpp:368 > > > + if (lightness(backgroundColor.toSRGBAComponentsLossy()) <= .5f && backgroundColor.isVisible()) > > > > Yeah, this would be much better with backgroundColor.getHSL.lightness(). > > Would you be ok with backgroundColor.lightness()? That's actually what I mean to write!
Comment on attachment 399847 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=399847&action=review >> Source/WebCore/editing/cocoa/DataDetection.mm:628 >> + auto underlineColor = Color(makeRGBAFromHSLA(hue, saturation, overrideLightness, overrideAlphaMultiplier * alpha)); > > Make sure the hue is correctly round-tripped here. I think this is wrong now. Going to make an API test with round-tripping to see.
Created attachment 399897 [details] Patch
Committed r261967: <https://trac.webkit.org/changeset/261967> All reviewed patches have been landed. Closing bug and clearing flags on attachment 399897 [details].
<rdar://problem/63467460>