Changeset 247792 in webkit

Timestamp:

Jul 24, 2019, 3:36:13 PM (7 years ago)

Author:

timothy_horton@apple.com

Message:

Daring Fireball long press highlights are unnecessarily inflated due to false illegibility
​https://bugs.webkit.org/show_bug.cgi?id=200064

Reviewed by Geoff Garen.

Source/WebCore:

If we consider text illegible on the given estimated background color,
we bail from doing a tightly fitted selection-only TextIndicator and
instead just paint the page without modification into the indicator,
causing ugly overlap and an excessively inflated indicator.

Change the mechanism we use to determine illegibility to be based on
a standard, instead of a constant chosen by hand 13 years ago.

Test: fast/text-indicator/text-indicator-with-low-contrast-text.html

• platform/graphics/ColorUtilities.cpp:

(WebCore::luminance):
Fix a typo.

(WebCore::contrastRatio):
Add a function that computes the contrast ratio given two colors using
the formula from WCAG.

• platform/graphics/ColorUtilities.h:
• rendering/TextPaintStyle.cpp:

(WebCore::textColorIsLegibleAgainstBackgroundColor):
Make use of WCAG's minimum legible contrast ratio instead of an
arbitrary color difference cutoff for determining whether we consider
text legible. It seems sensible and also considers the text on DF readable
(which it seems to be to me!).

• testing/Internals.cpp:

(WebCore::Internals::TextIndicatorInfo::TextIndicatorInfo):

• testing/Internals.h:
• testing/Internals.idl:

Expose all of the text rects to Internals, not just the bounding rect.
Expose some more TextIndicator options to Internals so that we can
turn on the legibility mechanism.

LayoutTests:

• fast/text-indicator/text-indicator-with-low-contrast-text-expected.txt: Added.
• fast/text-indicator/text-indicator-with-low-contrast-text.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text-indicator/text-indicator-with-low-contrast-text-expected.txt (added)
• LayoutTests/fast/text-indicator/text-indicator-with-low-contrast-text.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/ColorUtilities.cpp (modified) (1 diff)
• Source/WebCore/platform/graphics/ColorUtilities.h (modified) (1 diff)
• Source/WebCore/rendering/TextPaintStyle.cpp (modified) (2 diffs)
• Source/WebCore/testing/Internals.cpp (modified) (1 diff)
• Source/WebCore/testing/Internals.h (modified) (3 diffs)
• Source/WebCore/testing/Internals.idl (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-07-24  Tim Horton  <timothy_horton@apple.com>
+
+        Daring Fireball long press highlights are unnecessarily inflated due to false illegibility
+        https://bugs.webkit.org/show_bug.cgi?id=200064
+
+        Reviewed by Geoff Garen.
+
+        * fast/text-indicator/text-indicator-with-low-contrast-text-expected.txt: Added.
+        * fast/text-indicator/text-indicator-with-low-contrast-text.html: Added.
+
 2019-07-24  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-24  Tim Horton  <timothy_horton@apple.com>
+
+        Daring Fireball long press highlights are unnecessarily inflated due to false illegibility
+        https://bugs.webkit.org/show_bug.cgi?id=200064
+
+        Reviewed by Geoff Garen.
+
+        If we consider text illegible on the given estimated background color,
+        we bail from doing a tightly fitted selection-only TextIndicator and
+        instead just paint the page without modification into the indicator,
+        causing ugly overlap and an excessively inflated indicator.
+
+        Change the mechanism we use to determine illegibility to be based on
+        a standard, instead of a constant chosen by hand 13 years ago.
+
+        Test: fast/text-indicator/text-indicator-with-low-contrast-text.html
+
+        * platform/graphics/ColorUtilities.cpp:
+        (WebCore::luminance):
+        Fix a typo.
+
+        (WebCore::contrastRatio):
+        Add a function that computes the contrast ratio given two colors using
+        the formula from WCAG.
+
+        * platform/graphics/ColorUtilities.h:
+        * rendering/TextPaintStyle.cpp:
+        (WebCore::textColorIsLegibleAgainstBackgroundColor):
+        Make use of WCAG's minimum legible contrast ratio instead of an
+        arbitrary color difference cutoff for determining whether we consider
+        text legible. It seems sensible and also considers the text on DF readable
+        (which it seems to be to me!).
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::TextIndicatorInfo::TextIndicatorInfo):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+        Expose all of the text rects to Internals, not just the bounding rect.
+        Expose some more TextIndicator options to Internals so that we can
+        turn on the legibility mechanism.
+
 2019-07-24  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebCore/platform/graphics/ColorUtilities.cpp

@@ -105 +105 @@
 }
 
-float luminance(const FloatComponents& sRGBCompontents)
+float luminance(const FloatComponents& sRGBComponents)
 {
     // Values from https://www.w3.org/TR/2008/REC-WCAG20-20081211/#relativeluminancedef
-    return 0.2126f * sRGBToLinearColorComponentForLuminance(sRGBCompontents.components[0])
-        + 0.7152f * sRGBToLinearColorComponentForLuminance(sRGBCompontents.components[1])
-        + 0.0722f * sRGBToLinearColorComponentForLuminance(sRGBCompontents.components[2]);
+    return 0.2126f * sRGBToLinearColorComponentForLuminance(sRGBComponents.components[0])
+        + 0.7152f * sRGBToLinearColorComponentForLuminance(sRGBComponents.components[1])
+        + 0.0722f * sRGBToLinearColorComponentForLuminance(sRGBComponents.components[2]);
+}
+
+float contrastRatio(const FloatComponents& componentsA, const FloatComponents& componentsB)
+{
+    // Uses the WCAG 2.0 definition of contrast ratio.
+    // https://www.w3.org/TR/WCAG20/#contrast-ratiodef
+    float lighterLuminance = luminance(componentsA);
+    float darkerLuminance = luminance(componentsB);
+
+    if (lighterLuminance < darkerLuminance)
+        std::swap(lighterLuminance, darkerLuminance);
+
+    return (lighterLuminance + 0.05) / (darkerLuminance + 0.05);
 }
 

trunk/Source/WebCore/platform/graphics/ColorUtilities.h

@@ -163 +163 @@
 
 float luminance(const FloatComponents& sRGBCompontents);
+float contrastRatio(const FloatComponents&, const FloatComponents&);
 
 class ColorMatrix {

trunk/Source/WebCore/rendering/TextPaintStyle.cpp

@@ -27 +27 @@
 #include "TextPaintStyle.h"
 
+#include "ColorUtilities.h"
 #include "FocusController.h"
 #include "Frame.h"
@@ -61 +62 @@
 bool textColorIsLegibleAgainstBackgroundColor(const Color& textColor, const Color& backgroundColor)
 {
-    // Semi-arbitrarily chose 65025 (255^2) value here after a few tests.
-    return differenceSquared(textColor, backgroundColor) > 65025;
+    // Uses the WCAG 2.0 definition of legibility: a contrast ratio of 4.5:1 or greater.
+    // https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast
+    return contrastRatio(textColor, backgroundColor) > 4.5;
 }
 

trunk/Source/WebCore/testing/Internals.cpp

@@ -5126 +5126 @@
 Internals::TextIndicatorInfo::TextIndicatorInfo(const WebCore::TextIndicatorData& data)
     : textBoundingRectInRootViewCoordinates(DOMRect::create(data.textBoundingRectInRootViewCoordinates))
+    , textRectsInBoundingRectCoordinates(DOMRectList::create(data.textRectsInBoundingRectCoordinates))
 {
 }

trunk/Source/WebCore/testing/Internals.h

@@ -838 +838 @@
     struct TextIndicatorInfo {
         RefPtr<DOMRectReadOnly> textBoundingRectInRootViewCoordinates;
+        RefPtr<DOMRectList> textRectsInBoundingRectCoordinates;
         
         TextIndicatorInfo();
@@ -846 +847 @@
     struct TextIndicatorOptions {
         bool useBoundingRectAndPaintAllContentForComplexRanges { false };
+        bool computeEstimatedBackgroundColor { false };
+        bool respectTextColor { false };
         
         WebCore::TextIndicatorOptions core()
@@ -852 +855 @@
             if (useBoundingRectAndPaintAllContentForComplexRanges)
                 options = options | TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges;
+            if (computeEstimatedBackgroundColor)
+                options = options | TextIndicatorOptionComputeEstimatedBackgroundColor;
+            if (respectTextColor)
+                options = options | TextIndicatorOptionRespectTextColor;
             return options;
         }

trunk/Source/WebCore/testing/Internals.idl

@@ -165 +165 @@
 ] dictionary TextIndicatorInfo {
     DOMRectReadOnly textBoundingRectInRootViewCoordinates;
+    DOMRectList textRectsInBoundingRectCoordinates;
 };
 
@@ -172 +173 @@
 ] dictionary TextIndicatorOptions {
     boolean useBoundingRectAndPaintAllContentForComplexRanges = false;
+    boolean computeEstimatedBackgroundColor = false;
+    boolean respectTextColor = false;
 };