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

Changeset 276187 in webkit


Ignore:
Timestamp:
Apr 16, 2021, 6:08:45 PM (5 years ago)
Author:
Darin Adler
Message:

font-size with viewport units in calc() doesn't change when viewport resizes
​https://bugs.webkit.org/show_bug.cgi?id=224614

Reviewed by Zalan Bujtas.

Source/WebCore:

  • css/CSSToLengthConversionData.cpp:

(WebCore::CSSToLengthConversionData::zoom const): Updated since m_zoom is now optional.
We use effectiveZoom when m_zoom is not specified, which is the same semantic that was
implemented before with a separate boolean.
(WebCore::CSSToLengthConversionData::viewportWidthFactor const): When calling the
setHasViewportUnits function as a side effect, use m_viewportDependencyDetectionStyle,
rather than always using m_style. This lets us handle the font-size case correctly.
Also removed the explicit computingFontSize check for the same reason.
(WebCore::CSSToLengthConversionData::viewportHeightFactor const): Ditto.
(WebCore::CSSToLengthConversionData::viewportMinFactor const): Ditto.
(WebCore::CSSToLengthConversionData::viewportMaxFactor const): Ditto.

  • css/CSSToLengthConversionData.h: Added a new member, m_viewportDependencyDetectionStyle,

which defaults to the same value as m_style. Also changed m_zoom to use Optional instead
of a separate boolean and an ignored "must be 1.0" value. Initialized data members in
the modern way, allowing us to use the default constructor.

  • style/StyleBuilderCustom.h:

(WebCore::Style::BuilderCustom::applyValueFontSize): Pass in the builder's style as the
viewportDependencyDetectionStyle. This does the same thing that the existing code to
call setHasViewportUnits did directly, but does it even for more complex cases involving
calc(). Also made the isLength and isCalculatedPercentageWithLength cases more similar
to each other and left a FIXME behind about taking that a bit further, but doing that
probably requires creating some more test cases.

LayoutTests:

  • css3/viewport-percentage-lengths/viewport-percentage-lengths-resize-expected.txt:
  • css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html:

Added tests that involve calc, and broke rules up into multiple elements so that side
effects from one style won't give us false negatives. This now has a subtest that was
failing without the fix in this patch.

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r276186 r276187  
     12021-04-16  Darin Adler  <darin@apple.com>
     2
     3        font-size with viewport units in calc() doesn't change when viewport resizes
     4        https://bugs.webkit.org/show_bug.cgi?id=224614
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        * css3/viewport-percentage-lengths/viewport-percentage-lengths-resize-expected.txt:
     9        * css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html:
     10        Added tests that involve calc, and broke rules up into multiple elements so that side
     11        effects from one style won't give us false negatives. This now has a subtest that was
     12        failing without the fix in this patch.
     13
    1142021-04-16  Ian Gilbert  <iang@apple.com>
    215

  • trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize-expected.txt

    r198943 r276187  
    99PASS innerWidth is 800
    1010PASS innerHeight is 600
    11 PASS getComputedStyle(test).fontSize is "30px"
    1211PASS getComputedStyle(test).width is "400px"
     12PASS getComputedStyle(testfontsize).fontSize is "30px"
     13PASS getComputedStyle(testcalc).width is "800px"
     14PASS getComputedStyle(testfontsizecalc).fontSize is "60px"
    1315PASS getComputedStyle(testpseudo, ':after').marginLeft is "120px"
    1416PASS getComputedStyle(testpseudo, ':after').paddingRight is "200px"
     17PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "240px"
     18PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "400px"
    1519PASS innerWidth is 900
    1620PASS innerHeight is 600
    17 PASS getComputedStyle(test).fontSize is "30px"
    1821PASS getComputedStyle(test).width is "450px"
     22PASS getComputedStyle(testfontsize).fontSize is "30px"
     23PASS getComputedStyle(testcalc).width is "900px"
     24PASS getComputedStyle(testfontsizecalc).fontSize is "60px"
    1925PASS getComputedStyle(testpseudo, ':after').marginLeft is "120px"
    2026PASS getComputedStyle(testpseudo, ':after').paddingRight is "225px"
     27PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "240px"
     28PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "450px"
    2129PASS innerWidth is 900
    2230PASS innerHeight is 640
    23 PASS getComputedStyle(test).fontSize is "32px"
    2431PASS getComputedStyle(test).width is "450px"
     32PASS getComputedStyle(testfontsize).fontSize is "32px"
     33PASS getComputedStyle(testcalc).width is "900px"
     34PASS getComputedStyle(testfontsizecalc).fontSize is "64px"
    2535PASS getComputedStyle(testpseudo, ':after').marginLeft is "128px"
    2636PASS getComputedStyle(testpseudo, ':after').paddingRight is "225px"
     37PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "256px"
     38PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "450px"
    2739PASS innerWidth is 500
    2840PASS innerHeight is 640
    29 PASS getComputedStyle(test).fontSize is "32px"
    3041PASS getComputedStyle(test).width is "250px"
     42PASS getComputedStyle(testfontsize).fontSize is "32px"
     43PASS getComputedStyle(testcalc).width is "500px"
     44PASS getComputedStyle(testfontsizecalc).fontSize is "64px"
    3145PASS getComputedStyle(testpseudo, ':after').marginLeft is "100px"
    3246PASS getComputedStyle(testpseudo, ':after').paddingRight is "160px"
     47PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "200px"
     48PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "320px"
    3349PASS innerWidth is 800
    3450PASS innerHeight is 600
    35 PASS getComputedStyle(test).fontSize is "30px"
    3651PASS getComputedStyle(test).width is "400px"
     52PASS getComputedStyle(testfontsize).fontSize is "30px"
     53PASS getComputedStyle(testcalc).width is "800px"
     54PASS getComputedStyle(testfontsizecalc).fontSize is "60px"
    3755PASS getComputedStyle(testpseudo, ':after').marginLeft is "120px"
    3856PASS getComputedStyle(testpseudo, ':after').paddingRight is "200px"
     57PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "240px"
     58PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "400px"
    3959

  • trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html

    r169505 r276187  
    22<style>
    33#test {
     4    width: 50vw;
     5}
     6#testfontsize {
    47    font-size: 5vh;
    5     width: 50vw;
     8}
     9#testcalc {
     10    width: calc(50vw * 2);
     11}
     12#testfontsizecalc {
     13    font-size: calc(5vh * 2);
    614}
    715#testpseudo:after {
    … …  
    1018    content: '';
    1119}
     20#testpseudocalc:after {
     21    margin-left: calc(20vmin * 2);
     22    padding-right: calc(25vmax * 2);
     23    content: '';
     24}
    1225</style>
    1326<body>
    1427    <div id="test"></div>
     28    <div id="testfontsize"></div>
     29    <div id="testcalc"></div>
     30    <div id="testfontsizecalc"></div>
    1531    <div id="testpseudo"></div>
     32    <div id="testpseudocalc"></div>
    1633</body>
    1734<script src="../../resources/js-test-pre.js"></script>
    … …  
    2239    var min = Math.min(innerWidth, innerHeight);
    2340    var max = Math.max(innerWidth, innerHeight);
    24     shouldBeEqualToString("getComputedStyle(test).fontSize", innerHeight / 20 + "px");
    2541    shouldBeEqualToString("getComputedStyle(test).width", innerWidth / 2 + "px");
     42    shouldBeEqualToString("getComputedStyle(testfontsize).fontSize", innerHeight / 20 + "px");
     43    shouldBeEqualToString("getComputedStyle(testcalc).width", innerWidth + "px");
     44    shouldBeEqualToString("getComputedStyle(testfontsizecalc).fontSize", innerHeight / 10 + "px");
    2645    shouldBeEqualToString("getComputedStyle(testpseudo, ':after').marginLeft", min / 5 + "px");
    2746    shouldBeEqualToString("getComputedStyle(testpseudo, ':after').paddingRight", max / 4 + "px");
     47    shouldBeEqualToString("getComputedStyle(testpseudocalc, ':after').marginLeft", (min * 2) / 5 + "px");
     48    shouldBeEqualToString("getComputedStyle(testpseudocalc, ':after').paddingRight", max / 2 + "px");
    2849});
    2950</script>

  • trunk/Source/WebCore/ChangeLog

    r276186 r276187  
     12021-04-16  Darin Adler  <darin@apple.com>
     2
     3        font-size with viewport units in calc() doesn't change when viewport resizes
     4        https://bugs.webkit.org/show_bug.cgi?id=224614
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        * css/CSSToLengthConversionData.cpp:
     9        (WebCore::CSSToLengthConversionData::zoom const): Updated since m_zoom is now optional.
     10        We use effectiveZoom when m_zoom is not specified, which is the same semantic that was
     11        implemented before with a separate boolean.
     12        (WebCore::CSSToLengthConversionData::viewportWidthFactor const): When calling the
     13        setHasViewportUnits function as a side effect, use m_viewportDependencyDetectionStyle,
     14        rather than always using m_style. This lets us handle the font-size case correctly.
     15        Also removed the explicit computingFontSize check for the same reason.
     16        (WebCore::CSSToLengthConversionData::viewportHeightFactor const): Ditto.
     17        (WebCore::CSSToLengthConversionData::viewportMinFactor const): Ditto.
     18        (WebCore::CSSToLengthConversionData::viewportMaxFactor const): Ditto.
     19
     20        * css/CSSToLengthConversionData.h: Added a new member, m_viewportDependencyDetectionStyle,
     21        which defaults to the same value as m_style. Also changed m_zoom to use Optional instead
     22        of a separate boolean and an ignored "must be 1.0" value. Initialized data members in
     23        the modern way, allowing us to use the default constructor.
     24
     25        * style/StyleBuilderCustom.h:
     26        (WebCore::Style::BuilderCustom::applyValueFontSize): Pass in the builder's style as the
     27        viewportDependencyDetectionStyle. This does the same thing that the existing code to
     28        call setHasViewportUnits did directly, but does it even for more complex cases involving
     29        calc(). Also made the isLength and isCalculatedPercentageWithLength cases more similar
     30        to each other and left a FIXME behind about taking that a bit further, but doing that
     31        probably requires creating some more test cases.
     32
    1332021-04-16  Ian Gilbert  <iang@apple.com>
    234

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

    r259703 r276187  
    3939float CSSToLengthConversionData::zoom() const
    4040{
    41     if (m_useEffectiveZoom)
     41    if (!m_zoom)
    4242        return m_style ? m_style->effectiveZoom() : 1;
    43     return m_zoom;
     43    return *m_zoom;
    4444}
    4545
    4646double CSSToLengthConversionData::viewportWidthFactor() const
    4747{
    48     if (m_style && !computingFontSize())
    49         const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
     48    if (m_viewportDependencyDetectionStyle)
     49        m_viewportDependencyDetectionStyle->setHasViewportUnits();
    5050
    5151    if (!m_renderView)
    … …  
    5757double CSSToLengthConversionData::viewportHeightFactor() const
    5858{
    59     if (m_style && !computingFontSize())
    60         const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
     59    if (m_viewportDependencyDetectionStyle)
     60        m_viewportDependencyDetectionStyle->setHasViewportUnits();
    6161
    6262    if (!m_renderView)
    … …  
    6868double CSSToLengthConversionData::viewportMinFactor() const
    6969{
    70     if (m_style && !computingFontSize())
    71         const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
     70    if (m_viewportDependencyDetectionStyle)
     71        m_viewportDependencyDetectionStyle->setHasViewportUnits();
    7272
    7373    if (!m_renderView)
    … …  
    8080double CSSToLengthConversionData::viewportMaxFactor() const
    8181{
    82     if (m_style && !computingFontSize())
    83         const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
     82    if (m_viewportDependencyDetectionStyle)
     83        m_viewportDependencyDetectionStyle->setHasViewportUnits();
    8484
    8585    if (!m_renderView)

  • trunk/Source/WebCore/css/CSSToLengthConversionData.h

    r273964 r276187  
    4242class CSSToLengthConversionData {
    4343public:
    44     CSSToLengthConversionData(const RenderStyle* style, const RenderStyle* rootStyle, const RenderStyle* parentStyle, const RenderView* renderView, float zoom, Optional<CSSPropertyID> propertyToCompute = WTF::nullopt)
     44    CSSToLengthConversionData(const RenderStyle* style, const RenderStyle* rootStyle, const RenderStyle* parentStyle, const RenderView* renderView, float zoom, Optional<CSSPropertyID> propertyToCompute = WTF::nullopt, RenderStyle* viewportDependencyDetectionStyle = nullptr)
    4545        : m_style(style)
    4646        , m_rootStyle(rootStyle)
    4747        , m_parentStyle(parentStyle)
     48        , m_viewportDependencyDetectionStyle(viewportDependencyDetectionStyle ? viewportDependencyDetectionStyle : const_cast<RenderStyle*>(style))
    4849        , m_renderView(renderView)
    4950        , m_zoom(zoom)
    50         , m_useEffectiveZoom(false)
    5151        , m_propertyToCompute(propertyToCompute)
    5252    {
    … …  
    5858        , m_rootStyle(rootStyle)
    5959        , m_parentStyle(parentStyle)
     60        , m_viewportDependencyDetectionStyle(const_cast<RenderStyle*>(style))
    6061        , m_renderView(renderView)
    61         , m_zoom(1)
    62         , m_useEffectiveZoom(true)
    6362        , m_propertyToCompute(propertyToCompute)
    6463    {
    6564    }
    6665
    67     CSSToLengthConversionData()
    68         : CSSToLengthConversionData(nullptr, nullptr, nullptr, nullptr)
    69     {
    70     }
     66    CSSToLengthConversionData() = default;
    7167
    7268    const RenderStyle* style() const { return m_style; }
    … …  
    9591
    9692private:
    97     const RenderStyle* m_style;
    98     const RenderStyle* m_rootStyle;
    99     const RenderStyle* m_parentStyle;
    100     const RenderView* m_renderView;
    101     float m_zoom;
    102     bool m_useEffectiveZoom;
     93    const RenderStyle* m_style { nullptr };
     94    const RenderStyle* m_rootStyle { nullptr };
     95    const RenderStyle* m_parentStyle { nullptr };
     96    RenderStyle* m_viewportDependencyDetectionStyle { nullptr };
     97    const RenderView* m_renderView { nullptr };
     98    Optional<float> m_zoom;
    10399    Optional<CSSPropertyID> m_propertyToCompute;
    104100};

  • trunk/Source/WebCore/style/StyleBuilderCustom.h

    r275495 r276187  
    18321832        fontDescription.setIsAbsoluteSize(parentIsAbsoluteSize || !(primitiveValue.isPercentage() || primitiveValue.isFontRelativeLength()));
    18331833        if (primitiveValue.isLength()) {
    1834             size = primitiveValue.computeLength<float>(CSSToLengthConversionData(&builderState.parentStyle(), builderState.rootElementStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize));
    1835             if (primitiveValue.isViewportPercentageLength())
    1836                 builderState.style().setHasViewportUnits();
     1834            CSSToLengthConversionData conversionData { &builderState.parentStyle(), builderState.rootElementStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize, &builderState.style() };
     1835            size = primitiveValue.computeLength<float>(conversionData);
    18371836        } else if (primitiveValue.isPercentage())
    18381837            size = (primitiveValue.floatValue() * parentSize) / 100.0f;
    18391838        else if (primitiveValue.isCalculatedPercentageWithLength()) {
    1840             const auto& conversionData = builderState.cssToLengthConversionData();
    1841             CSSToLengthConversionData parentConversionData { &builderState.parentStyle(), conversionData.rootStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize };
    1842             size = primitiveValue.cssCalcValue()->createCalculationValue(parentConversionData)->evaluate(parentSize);
     1839            // FIXME: Why does this need a different root style than the isLength case above?
     1840            CSSToLengthConversionData conversionData { &builderState.parentStyle(), builderState.cssToLengthConversionData().rootStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize, &builderState.style() };
     1841            size = primitiveValue.cssCalcValue()->createCalculationValue(conversionData)->evaluate(parentSize);
    18431842        } else
    18441843            return;
Note: See TracChangeset for help on using the changeset viewer.