⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 248347 in webkit


Ignore:
Timestamp:
Aug 6, 2019, 11:52:25 PM (7 years ago)
Author:
Kocsen Chung
Message:

Cherry-pick r247792. rdar://problem/54017900

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.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@247792 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-608.1-branch
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608.1-branch/LayoutTests/ChangeLog

    r248345 r248347  
     12019-08-06  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r247792. rdar://problem/54017900
     4
     5    Daring Fireball long press highlights are unnecessarily inflated due to false illegibility
     6    https://bugs.webkit.org/show_bug.cgi?id=200064
     7   
     8    Reviewed by Geoff Garen.
     9   
     10    Source/WebCore:
     11   
     12    If we consider text illegible on the given estimated background color,
     13    we bail from doing a tightly fitted selection-only TextIndicator and
     14    instead just paint the page without modification into the indicator,
     15    causing ugly overlap and an excessively inflated indicator.
     16   
     17    Change the mechanism we use to determine illegibility to be based on
     18    a standard, instead of a constant chosen by hand 13 years ago.
     19   
     20    Test: fast/text-indicator/text-indicator-with-low-contrast-text.html
     21   
     22    * platform/graphics/ColorUtilities.cpp:
     23    (WebCore::luminance):
     24    Fix a typo.
     25   
     26    (WebCore::contrastRatio):
     27    Add a function that computes the contrast ratio given two colors using
     28    the formula from WCAG.
     29   
     30    * platform/graphics/ColorUtilities.h:
     31    * rendering/TextPaintStyle.cpp:
     32    (WebCore::textColorIsLegibleAgainstBackgroundColor):
     33    Make use of WCAG's minimum legible contrast ratio instead of an
     34    arbitrary color difference cutoff for determining whether we consider
     35    text legible. It seems sensible and also considers the text on DF readable
     36    (which it seems to be to me!).
     37   
     38    * testing/Internals.cpp:
     39    (WebCore::Internals::TextIndicatorInfo::TextIndicatorInfo):
     40    * testing/Internals.h:
     41    * testing/Internals.idl:
     42    Expose all of the text rects to Internals, not just the bounding rect.
     43    Expose some more TextIndicator options to Internals so that we can
     44    turn on the legibility mechanism.
     45   
     46    LayoutTests:
     47   
     48    * fast/text-indicator/text-indicator-with-low-contrast-text-expected.txt: Added.
     49    * fast/text-indicator/text-indicator-with-low-contrast-text.html: Added.
     50   
     51   
     52    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247792 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     53
     54    2019-07-24  Tim Horton  <timothy_horton@apple.com>
     55
     56            Daring Fireball long press highlights are unnecessarily inflated due to false illegibility
     57            https://bugs.webkit.org/show_bug.cgi?id=200064
     58
     59            Reviewed by Geoff Garen.
     60
     61            * fast/text-indicator/text-indicator-with-low-contrast-text-expected.txt: Added.
     62            * fast/text-indicator/text-indicator-with-low-contrast-text.html: Added.
     63
    1642019-08-06  Kocsen Chung  <kocsen_chung@apple.com>
    265

  • branches/safari-608.1-branch/Source/WebCore/ChangeLog

    r248346 r248347  
     12019-08-06  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r247792. rdar://problem/54017900
     4
     5    Daring Fireball long press highlights are unnecessarily inflated due to false illegibility
     6    https://bugs.webkit.org/show_bug.cgi?id=200064
     7   
     8    Reviewed by Geoff Garen.
     9   
     10    Source/WebCore:
     11   
     12    If we consider text illegible on the given estimated background color,
     13    we bail from doing a tightly fitted selection-only TextIndicator and
     14    instead just paint the page without modification into the indicator,
     15    causing ugly overlap and an excessively inflated indicator.
     16   
     17    Change the mechanism we use to determine illegibility to be based on
     18    a standard, instead of a constant chosen by hand 13 years ago.
     19   
     20    Test: fast/text-indicator/text-indicator-with-low-contrast-text.html
     21   
     22    * platform/graphics/ColorUtilities.cpp:
     23    (WebCore::luminance):
     24    Fix a typo.
     25   
     26    (WebCore::contrastRatio):
     27    Add a function that computes the contrast ratio given two colors using
     28    the formula from WCAG.
     29   
     30    * platform/graphics/ColorUtilities.h:
     31    * rendering/TextPaintStyle.cpp:
     32    (WebCore::textColorIsLegibleAgainstBackgroundColor):
     33    Make use of WCAG's minimum legible contrast ratio instead of an
     34    arbitrary color difference cutoff for determining whether we consider
     35    text legible. It seems sensible and also considers the text on DF readable
     36    (which it seems to be to me!).
     37   
     38    * testing/Internals.cpp:
     39    (WebCore::Internals::TextIndicatorInfo::TextIndicatorInfo):
     40    * testing/Internals.h:
     41    * testing/Internals.idl:
     42    Expose all of the text rects to Internals, not just the bounding rect.
     43    Expose some more TextIndicator options to Internals so that we can
     44    turn on the legibility mechanism.
     45   
     46    LayoutTests:
     47   
     48    * fast/text-indicator/text-indicator-with-low-contrast-text-expected.txt: Added.
     49    * fast/text-indicator/text-indicator-with-low-contrast-text.html: Added.
     50   
     51   
     52    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247792 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     53
     54    2019-07-24  Tim Horton  <timothy_horton@apple.com>
     55
     56            Daring Fireball long press highlights are unnecessarily inflated due to false illegibility
     57            https://bugs.webkit.org/show_bug.cgi?id=200064
     58
     59            Reviewed by Geoff Garen.
     60
     61            If we consider text illegible on the given estimated background color,
     62            we bail from doing a tightly fitted selection-only TextIndicator and
     63            instead just paint the page without modification into the indicator,
     64            causing ugly overlap and an excessively inflated indicator.
     65
     66            Change the mechanism we use to determine illegibility to be based on
     67            a standard, instead of a constant chosen by hand 13 years ago.
     68
     69            Test: fast/text-indicator/text-indicator-with-low-contrast-text.html
     70
     71            * platform/graphics/ColorUtilities.cpp:
     72            (WebCore::luminance):
     73            Fix a typo.
     74
     75            (WebCore::contrastRatio):
     76            Add a function that computes the contrast ratio given two colors using
     77            the formula from WCAG.
     78
     79            * platform/graphics/ColorUtilities.h:
     80            * rendering/TextPaintStyle.cpp:
     81            (WebCore::textColorIsLegibleAgainstBackgroundColor):
     82            Make use of WCAG's minimum legible contrast ratio instead of an
     83            arbitrary color difference cutoff for determining whether we consider
     84            text legible. It seems sensible and also considers the text on DF readable
     85            (which it seems to be to me!).
     86
     87            * testing/Internals.cpp:
     88            (WebCore::Internals::TextIndicatorInfo::TextIndicatorInfo):
     89            * testing/Internals.h:
     90            * testing/Internals.idl:
     91            Expose all of the text rects to Internals, not just the bounding rect.
     92            Expose some more TextIndicator options to Internals so that we can
     93            turn on the legibility mechanism.
     94
    1952019-08-06  Kocsen Chung  <kocsen_chung@apple.com>
    296

  • branches/safari-608.1-branch/Source/WebCore/platform/graphics/ColorUtilities.cpp

    r233877 r248347  
    105105}
    106106
    107 float luminance(const FloatComponents& sRGBCompontents)
     107float luminance(const FloatComponents& sRGBComponents)
    108108{
    109109    // Values from https://www.w3.org/TR/2008/REC-WCAG20-20081211/#relativeluminancedef
    110     return 0.2126f * sRGBToLinearColorComponentForLuminance(sRGBCompontents.components[0])
    111         + 0.7152f * sRGBToLinearColorComponentForLuminance(sRGBCompontents.components[1])
    112         + 0.0722f * sRGBToLinearColorComponentForLuminance(sRGBCompontents.components[2]);
     110    return 0.2126f * sRGBToLinearColorComponentForLuminance(sRGBComponents.components[0])
     111        + 0.7152f * sRGBToLinearColorComponentForLuminance(sRGBComponents.components[1])
     112        + 0.0722f * sRGBToLinearColorComponentForLuminance(sRGBComponents.components[2]);
     113}
     114
     115float contrastRatio(const FloatComponents& componentsA, const FloatComponents& componentsB)
     116{
     117    // Uses the WCAG 2.0 definition of contrast ratio.
     118    // https://www.w3.org/TR/WCAG20/#contrast-ratiodef
     119    float lighterLuminance = luminance(componentsA);
     120    float darkerLuminance = luminance(componentsB);
     121
     122    if (lighterLuminance < darkerLuminance)
     123        std::swap(lighterLuminance, darkerLuminance);
     124
     125    return (lighterLuminance + 0.05) / (darkerLuminance + 0.05);
    113126}
    114127

  • branches/safari-608.1-branch/Source/WebCore/platform/graphics/ColorUtilities.h

    r233877 r248347  
    163163
    164164float luminance(const FloatComponents& sRGBCompontents);
     165float contrastRatio(const FloatComponents&, const FloatComponents&);
    165166
    166167class ColorMatrix {

  • branches/safari-608.1-branch/Source/WebCore/rendering/TextPaintStyle.cpp

    r245752 r248347  
    2727#include "TextPaintStyle.h"
    2828
     29#include "ColorUtilities.h"
    2930#include "FocusController.h"
    3031#include "Frame.h"
    … …  
    6162bool textColorIsLegibleAgainstBackgroundColor(const Color& textColor, const Color& backgroundColor)
    6263{
    63     // Semi-arbitrarily chose 65025 (255^2) value here after a few tests.
    64     return differenceSquared(textColor, backgroundColor) > 65025;
     64    // Uses the WCAG 2.0 definition of legibility: a contrast ratio of 4.5:1 or greater.
     65    // https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast
     66    return contrastRatio(textColor, backgroundColor) > 4.5;
    6567}
    6668

  • branches/safari-608.1-branch/Source/WebCore/testing/Internals.cpp

    r247367 r248347  
    51105110Internals::TextIndicatorInfo::TextIndicatorInfo(const WebCore::TextIndicatorData& data)
    51115111    : textBoundingRectInRootViewCoordinates(DOMRect::create(data.textBoundingRectInRootViewCoordinates))
     5112    , textRectsInBoundingRectCoordinates(DOMRectList::create(data.textRectsInBoundingRectCoordinates))
    51125113{
    51135114}

  • branches/safari-608.1-branch/Source/WebCore/testing/Internals.h

    r246917 r248347  
    836836    struct TextIndicatorInfo {
    837837        RefPtr<DOMRectReadOnly> textBoundingRectInRootViewCoordinates;
     838        RefPtr<DOMRectList> textRectsInBoundingRectCoordinates;
    838839       
    839840        TextIndicatorInfo();
    … …  
    844845    struct TextIndicatorOptions {
    845846        bool useBoundingRectAndPaintAllContentForComplexRanges { false };
     847        bool computeEstimatedBackgroundColor { false };
     848        bool respectTextColor { false };
    846849       
    847850        WebCore::TextIndicatorOptions core()
    … …  
    850853            if (useBoundingRectAndPaintAllContentForComplexRanges)
    851854                options = options | TextIndicatorOptionUseBoundingRectAndPaintAllContentForComplexRanges;
     855            if (computeEstimatedBackgroundColor)
     856                options = options | TextIndicatorOptionComputeEstimatedBackgroundColor;
     857            if (respectTextColor)
     858                options = options | TextIndicatorOptionRespectTextColor;
    852859            return options;
    853860        }

  • branches/safari-608.1-branch/Source/WebCore/testing/Internals.idl

    r246917 r248347  
    165165] dictionary TextIndicatorInfo {
    166166    DOMRectReadOnly textBoundingRectInRootViewCoordinates;
     167    DOMRectList textRectsInBoundingRectCoordinates;
    167168};
    168169
    … …  
    172173] dictionary TextIndicatorOptions {
    173174    boolean useBoundingRectAndPaintAllContentForComplexRanges = false;
     175    boolean computeEstimatedBackgroundColor = false;
     176    boolean respectTextColor = false;
    174177};
    175178
Note: See TracChangeset for help on using the changeset viewer.