Changeset 248112 in webkit


Ignore:
Timestamp:
Aug 1, 2019 11:32:32 AM (5 years ago)
Author:
Wenson Hsieh
Message:

[Text autosizing] [iPadOS] Add targeted hacks to address some remaining text autosizing issues
​https://bugs.webkit.org/show_bug.cgi?id=200271
<rdar://problem/51734741>

Reviewed by Zalan Bujtas.

Source/WebCore:

Makes some targeted adjustments to the text autosizing heuristic, to ensure compatibility with several high-
profile websites. See changes below for more detail.

Tests: fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidate-special-cases.html

fast/text-autosizing/ios/idempotentmode/line-height-boosting.html

  • css/StyleResolver.cpp:

(WebCore::StyleResolver::adjustRenderStyleForTextAutosizing):

Avoid clipped sidebar links on sohu.com by not performing line-height boosting in the case where the element
probably has a small, fixed number of lines. See below for more detail. Additionally, don't attempt to adjust
the line height using the boosted font size, in the case where the element is not a candidate for idempotent
text autosizing.

  • rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):

Make various targeted hacks to fix a few websites:

  • Add a special case for top navigation bar links on yandex.ru, where line height greatly exceeds the

specified font size.

  • Avoid boosting some related video links on v.youku.com by considering the line-clamp CSS property when

determining the maximum number of lines of text an element is expected to contain.

  • Avoid boosting some front page links on asahi.com, which have non-repeating background images.
  • Add several other adjustments to more aggressively boost pieces of text on Google search results, such as

taking the word-break CSS property into account.

The bottom few pixels of sidebar links on naver.com are also no longer clipped after these changes.

  • rendering/style/TextSizeAdjustment.cpp:

(WebCore::AutosizeStatus::probablyContainsASmallFixedNumberOfLines):

Pulls out a piece of the heuristic added to fix sephora.com in r247467 out into a separate helper method. To
recap, this heuristic identifies elements with both a fixed height and fixed line height, for which the fixed
height is close to an integer multiple of the line height.

Also makes several small tweaks in the process: (1) change the max difference between fixed line height and
font size from 6 to 5 to ensure that some multiline caption text on Google search results is boosted, and (2)
replace usages of lineHeight() with specifiedLineHeight(), which current prevents this function from being
truly idempotent.

(WebCore::AutosizeStatus::updateStatus):

  • rendering/style/TextSizeAdjustment.h:

LayoutTests:

Add tests to cover some changes to line height boosting and the idempotent text autosizing candidate heuristic.

  • fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidate-special-cases-expected.txt: Added.
  • fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidate-special-cases.html: Added.
  • fast/text-autosizing/ios/idempotentmode/line-height-boosting-expected.txt:
  • fast/text-autosizing/ios/idempotentmode/line-height-boosting.html:
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r248111 r248112  
     12019-08-01  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Text autosizing] [iPadOS] Add targeted hacks to address some remaining text autosizing issues
     4        https://bugs.webkit.org/show_bug.cgi?id=200271
     5        <rdar://problem/51734741>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Add tests to cover some changes to line height boosting and the idempotent text autosizing candidate heuristic.
     10
     11        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidate-special-cases-expected.txt: Added.
     12        * fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidate-special-cases.html: Added.
     13        * fast/text-autosizing/ios/idempotentmode/line-height-boosting-expected.txt:
     14        * fast/text-autosizing/ios/idempotentmode/line-height-boosting.html:
     15
    1162019-08-01  Truitt Savell  <tsavell@apple.com>
    217

  • trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/line-height-boosting-expected.txt

    r246943 r248112  
    22PASS result is >= 13
    33PASS result is >= 11
    4 PASS result is >= 12
    5 PASS result is >= 18
     4PASS result is 12
     5PASS result is 22
     6PASS result is 18
     7PASS result is 18
     8PASS result is 18
    69PASS successfullyParsed is true
    710
    … …  
    1215no line-height boost
    1316no line-height boost
     17no line-height boost
     18no line-height boost
     19no line-height boost

  • trunk/LayoutTests/fast/text-autosizing/ios/idempotentmode/line-height-boosting.html

    r246943 r248112  
    1717<div><span id=target4 style="font-size: 6px; line-height: 12px">no line-height boost</span></div>
    1818<div><span id=target5 style="font-size: 18px;">no line-height boost</span></div>
     19<div style="height: 54px; overflow: hidden;">
     20    <div id=target6 style="font-size: 12px; line-height: 18px;">no line-height boost</div>
     21    <div id=target7 style="font-size: 12px; line-height: 18px;">no line-height boost</div>
     22    <div id=target8 style="font-size: 12px; line-height: 18px;">no line-height boost</div>
     23</div>
    1924<script>
    2025document.body.offsetHeight;
    … …  
    2934
    3035result = Number.parseInt(window.getComputedStyle(target4).getPropertyValue("line-height"));
    31 shouldBeGreaterThanOrEqual("result", "12");
     36shouldBe("result", "12");
    3237
    3338result = Number.parseInt(window.getComputedStyle(target5).getPropertyValue("line-height"));
    34 shouldBeGreaterThanOrEqual("result", "18");
     39shouldBe("result", "22");
     40
     41result = Number.parseInt(window.getComputedStyle(target6).getPropertyValue("line-height"));
     42shouldBe("result", "18");
     43
     44result = Number.parseInt(window.getComputedStyle(target7).getPropertyValue("line-height"));
     45shouldBe("result", "18");
     46
     47result = Number.parseInt(window.getComputedStyle(target8).getPropertyValue("line-height"));
     48shouldBe("result", "18");
    3549</script>
    3650<script src="../../../../resources/js-test-post.js"></script>

  • trunk/Source/WebCore/ChangeLog

    r248105 r248112  
     12019-08-01  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        [Text autosizing] [iPadOS] Add targeted hacks to address some remaining text autosizing issues
     4        https://bugs.webkit.org/show_bug.cgi?id=200271
     5        <rdar://problem/51734741>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Makes some targeted adjustments to the text autosizing heuristic, to ensure compatibility with several high-
     10        profile websites. See changes below for more detail.
     11
     12        Tests:  fast/text-autosizing/ios/idempotentmode/idempotent-autosizing-candidate-special-cases.html
     13                fast/text-autosizing/ios/idempotentmode/line-height-boosting.html
     14
     15        * css/StyleResolver.cpp:
     16        (WebCore::StyleResolver::adjustRenderStyleForTextAutosizing):
     17
     18        Avoid clipped sidebar links on sohu.com by not performing line-height boosting in the case where the element
     19        probably has a small, fixed number of lines. See below for more detail. Additionally, don't attempt to adjust
     20        the line height using the boosted font size, in the case where the element is not a candidate for idempotent
     21        text autosizing.
     22
     23        * rendering/style/RenderStyle.cpp:
     24        (WebCore::RenderStyle::isIdempotentTextAutosizingCandidate const):
     25
     26        Make various targeted hacks to fix a few websites:
     27
     28        -   Add a special case for top navigation bar links on yandex.ru, where line height greatly exceeds the
     29            specified font size.
     30
     31        -   Avoid boosting some related video links on v.youku.com by considering the line-clamp CSS property when
     32            determining the maximum number of lines of text an element is expected to contain.
     33
     34        -   Avoid boosting some front page links on asahi.com, which have non-repeating background images.
     35
     36        -   Add several other adjustments to more aggressively boost pieces of text on Google search results, such as
     37            taking the `word-break` CSS property into account.
     38
     39        The bottom few pixels of sidebar links on naver.com are also no longer clipped after these changes.
     40
     41        * rendering/style/TextSizeAdjustment.cpp:
     42        (WebCore::AutosizeStatus::probablyContainsASmallFixedNumberOfLines):
     43
     44        Pulls out a piece of the heuristic added to fix sephora.com in r247467 out into a separate helper method. To
     45        recap, this heuristic identifies elements with both a fixed height and fixed line height, for which the fixed
     46        height is close to an integer multiple of the line height.
     47
     48        Also makes several small tweaks in the process: (1) change the max difference between fixed line height and
     49        font size from 6 to 5 to ensure that some multiline caption text on Google search results is boosted, and (2)
     50        replace usages of `lineHeight()` with `specifiedLineHeight()`, which current prevents this function from being
     51        truly idempotent.
     52
     53        (WebCore::AutosizeStatus::updateStatus):
     54        * rendering/style/TextSizeAdjustment.h:
     55
    1562019-07-31  Mark Lam  <mark.lam@apple.com>
    257

  • trunk/Source/WebCore/css/StyleResolver.cpp

    r247667 r248112  
    902902            return;
    903903
     904        if (AutosizeStatus::probablyContainsASmallFixedNumberOfLines(style))
     905            return;
     906
    904907        style.setLineHeight({ minimumLineHeight, Fixed });
    905908    };
    … …  
    922925    style.setFontDescription(WTFMove(fontDescription));
    923926    style.fontCascade().update(&document().fontSelector());
    924     adjustLineHeightIfNeeded(adjustedFontSize);
     927
     928    // FIXME: We should restore computed line height to its original value in the case where the element is not
     929    // an idempotent text autosizing candidate; otherwise, if an element that is a text autosizing candidate contains
     930    // children which are not autosized, the non-autosized content will end up with a boosted line height.
     931    if (isCandidate)
     932        adjustLineHeightIfNeeded(adjustedFontSize);
     933
    925934    return true;
    926935}

  • trunk/Source/WebCore/rendering/style/RenderStyle.cpp

    r247467 r248112  
    500500        return false;
    501501
     502    const float smallMinimumDifferenceThresholdBetweenLineHeightAndSpecifiedFontSizeForBoostingText = 5;
     503    const float largeMinimumDifferenceThresholdBetweenLineHeightAndSpecifiedFontSizeForBoostingText = 25;
     504
    502505    if (fields.contains(AutosizeStatus::Fields::FixedHeight)) {
    503506        if (fields.contains(AutosizeStatus::Fields::FixedWidth)) {
    … …  
    506509                    return false;
    507510
     511                if (height().isFixed() && specifiedLineHeight().isFixed()) {
     512                    float specifiedSize = specifiedFontSize();
     513                    if (height().value() == specifiedSize && specifiedLineHeight().value() == specifiedSize)
     514                        return false;
     515                }
     516
    508517                return true;
    509518            }
    510519
    511             if (fields.contains(AutosizeStatus::Fields::Floating))
     520            if (fields.contains(AutosizeStatus::Fields::Floating)) {
     521                if (specifiedLineHeight().isFixed() && height().isFixed()) {
     522                    float specifiedSize = specifiedFontSize();
     523                    if (specifiedLineHeight().value() - specifiedSize > smallMinimumDifferenceThresholdBetweenLineHeightAndSpecifiedFontSizeForBoostingText
     524                        && height().value() - specifiedSize > smallMinimumDifferenceThresholdBetweenLineHeightAndSpecifiedFontSizeForBoostingText) {
     525                        return true;
     526                    }
     527                }
    512528                return false;
     529            }
    513530
    514531            if (fields.contains(AutosizeStatus::Fields::OverflowXHidden))
    … …  
    528545    }
    529546
    530     if (width().isFixed())
     547    if (width().isFixed()) {
     548        if (breakWords())
     549            return true;
     550
    531551        return false;
     552    }
    532553
    533554    if (textSizeAdjust().isPercentage() && textSizeAdjust().percentage() == 100) {
    … …  
    538559            return true;
    539560
     561        if (specifiedLineHeight().isFixed() && specifiedLineHeight().value() - specifiedFontSize() > largeMinimumDifferenceThresholdBetweenLineHeightAndSpecifiedFontSizeForBoostingText)
     562            return true;
     563
    540564        return false;
    541565    }
     566
     567    if (hasBackgroundImage() && backgroundRepeatX() == FillRepeat::NoRepeat && backgroundRepeatY() == FillRepeat::NoRepeat)
     568        return false;
    542569
    543570    return true;

  • trunk/Source/WebCore/rendering/style/TextSizeAdjustment.cpp

    r247484 r248112  
    4343}
    4444
     45bool AutosizeStatus::probablyContainsASmallFixedNumberOfLines(const RenderStyle& style)
     46{
     47    auto& lineHeightAsLength = style.specifiedLineHeight();
     48    if (!lineHeightAsLength.isFixed() && !lineHeightAsLength.isPercent())
     49        return false;
     50
     51    auto& maxHeight = style.maxHeight();
     52    Optional<Length> heightOrMaxHeightAsLength;
     53    if (maxHeight.isFixed())
     54        heightOrMaxHeightAsLength = style.maxHeight();
     55    else if (style.height().isFixed() && (!maxHeight.isSpecified() || maxHeight.isUndefined()))
     56        heightOrMaxHeightAsLength = style.height();
     57
     58    if (!heightOrMaxHeightAsLength)
     59        return false;
     60
     61    float heightOrMaxHeight = heightOrMaxHeightAsLength->value();
     62    if (heightOrMaxHeight <= 0)
     63        return false;
     64
     65    float approximateLineHeight = lineHeightAsLength.isPercent() ? lineHeightAsLength.percent() * style.specifiedFontSize() / 100 : lineHeightAsLength.value();
     66    if (approximateLineHeight <= 0)
     67        return false;
     68
     69    float approximateNumberOfLines = heightOrMaxHeight / approximateLineHeight;
     70    auto& lineClamp = style.lineClamp();
     71    if (!lineClamp.isNone() && !lineClamp.isPercentage()) {
     72        int lineClampValue = lineClamp.value();
     73        return lineClampValue && std::floor(approximateNumberOfLines) == lineClampValue;
     74    }
     75
     76    const int maximumNumberOfLines = 5;
     77    const float thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger = 0.01;
     78    return approximateNumberOfLines <= maximumNumberOfLines + thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger
     79        && approximateNumberOfLines - std::floor(approximateNumberOfLines) <= thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger;
     80}
     81
    4582void AutosizeStatus::updateStatus(RenderStyle& style)
    4683{
    … …  
    5188            return true;
    5289
    53         if (!style.lineHeight().isSpecified() || style.whiteSpace() == WhiteSpace::NoWrap)
     90        const float maximumDifferenceBetweenFixedLineHeightAndFontSize = 5;
     91        auto& lineHeight = style.specifiedLineHeight();
     92        if (lineHeight.isFixed() && lineHeight.value() - style.specifiedFontSize() > maximumDifferenceBetweenFixedLineHeightAndFontSize)
    5493            return false;
    5594
    56         const float maximumDifferenceBetweenFixedLineHeightAndFontSize = 6;
    57         if (style.lineHeight().isFixed() && style.lineHeight().value() - style.fontDescription().specifiedSize() > maximumDifferenceBetweenFixedLineHeightAndFontSize)
     95        if (style.whiteSpace() == WhiteSpace::NoWrap)
    5896            return false;
    5997
    60         Optional<Length> heightOrMaxHeightAsLength;
    61         if (style.height().isFixed() && style.maxHeight().isAuto())
    62             heightOrMaxHeightAsLength = style.height();
    63         else if (style.maxHeight().isFixed())
    64             heightOrMaxHeightAsLength = style.maxHeight();
    65 
    66         if (!heightOrMaxHeightAsLength)
    67             return false;
    68 
    69         float heightOrMaxHeight = heightOrMaxHeightAsLength->value();
    70         float computedLineHeight = style.computedLineHeight();
    71         if (computedLineHeight <= 0)
    72             return false;
    73 
    74         float approximateNumberOfLines = heightOrMaxHeight / computedLineHeight;
    75         const int maximumNumberOfLines = 5;
    76         const float thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger = 0.01;
    77         return approximateNumberOfLines <= maximumNumberOfLines + thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger
    78             && approximateNumberOfLines - std::floor(approximateNumberOfLines) <= thresholdForConsideringAnApproximateNumberOfLinesToBeCloseToAnInteger;
     98        return probablyContainsASmallFixedNumberOfLines(style);
    7999    };
    80100

  • trunk/Source/WebCore/rendering/style/TextSizeAdjustment.h

    r247467 r248112  
    6969    static void updateStatus(RenderStyle&);
    7070
     71    static bool probablyContainsASmallFixedNumberOfLines(const RenderStyle&);
     72
    7173private:
    7274    OptionSet<Fields> m_fields;
Note: See TracChangeset for help on using the changeset viewer.