- Revision
- 276187
- Author
- [email protected]
- Date
- 2021-04-16 18:08:45 -0700 (Fri, 16 Apr 2021)
Log 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.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (276186 => 276187)
--- trunk/LayoutTests/ChangeLog 2021-04-17 00:57:21 UTC (rev 276186)
+++ trunk/LayoutTests/ChangeLog 2021-04-17 01:08:45 UTC (rev 276187)
@@ -1,3 +1,16 @@
+2021-04-16 Darin Adler <[email protected]>
+
+ 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.
+
+ * 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.
+
2021-04-16 Ian Gilbert <[email protected]>
Nullptr deref in CompositeEditCommand::isRemovableBlock in DeleteSelectionCommand::removeRedundantBlocks
Modified: trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize-expected.txt (276186 => 276187)
--- trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize-expected.txt 2021-04-17 00:57:21 UTC (rev 276186)
+++ trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize-expected.txt 2021-04-17 01:08:45 UTC (rev 276187)
@@ -8,32 +8,52 @@
TEST COMPLETE
PASS innerWidth is 800
PASS innerHeight is 600
-PASS getComputedStyle(test).fontSize is "30px"
PASS getComputedStyle(test).width is "400px"
+PASS getComputedStyle(testfontsize).fontSize is "30px"
+PASS getComputedStyle(testcalc).width is "800px"
+PASS getComputedStyle(testfontsizecalc).fontSize is "60px"
PASS getComputedStyle(testpseudo, ':after').marginLeft is "120px"
PASS getComputedStyle(testpseudo, ':after').paddingRight is "200px"
+PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "240px"
+PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "400px"
PASS innerWidth is 900
PASS innerHeight is 600
-PASS getComputedStyle(test).fontSize is "30px"
PASS getComputedStyle(test).width is "450px"
+PASS getComputedStyle(testfontsize).fontSize is "30px"
+PASS getComputedStyle(testcalc).width is "900px"
+PASS getComputedStyle(testfontsizecalc).fontSize is "60px"
PASS getComputedStyle(testpseudo, ':after').marginLeft is "120px"
PASS getComputedStyle(testpseudo, ':after').paddingRight is "225px"
+PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "240px"
+PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "450px"
PASS innerWidth is 900
PASS innerHeight is 640
-PASS getComputedStyle(test).fontSize is "32px"
PASS getComputedStyle(test).width is "450px"
+PASS getComputedStyle(testfontsize).fontSize is "32px"
+PASS getComputedStyle(testcalc).width is "900px"
+PASS getComputedStyle(testfontsizecalc).fontSize is "64px"
PASS getComputedStyle(testpseudo, ':after').marginLeft is "128px"
PASS getComputedStyle(testpseudo, ':after').paddingRight is "225px"
+PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "256px"
+PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "450px"
PASS innerWidth is 500
PASS innerHeight is 640
-PASS getComputedStyle(test).fontSize is "32px"
PASS getComputedStyle(test).width is "250px"
+PASS getComputedStyle(testfontsize).fontSize is "32px"
+PASS getComputedStyle(testcalc).width is "500px"
+PASS getComputedStyle(testfontsizecalc).fontSize is "64px"
PASS getComputedStyle(testpseudo, ':after').marginLeft is "100px"
PASS getComputedStyle(testpseudo, ':after').paddingRight is "160px"
+PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "200px"
+PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "320px"
PASS innerWidth is 800
PASS innerHeight is 600
-PASS getComputedStyle(test).fontSize is "30px"
PASS getComputedStyle(test).width is "400px"
+PASS getComputedStyle(testfontsize).fontSize is "30px"
+PASS getComputedStyle(testcalc).width is "800px"
+PASS getComputedStyle(testfontsizecalc).fontSize is "60px"
PASS getComputedStyle(testpseudo, ':after').marginLeft is "120px"
PASS getComputedStyle(testpseudo, ':after').paddingRight is "200px"
+PASS getComputedStyle(testpseudocalc, ':after').marginLeft is "240px"
+PASS getComputedStyle(testpseudocalc, ':after').paddingRight is "400px"
Modified: trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html (276186 => 276187)
--- trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html 2021-04-17 00:57:21 UTC (rev 276186)
+++ trunk/LayoutTests/css3/viewport-percentage-lengths/viewport-percentage-lengths-resize.html 2021-04-17 01:08:45 UTC (rev 276187)
@@ -1,18 +1,35 @@
<!DOCTYPE html>
<style>
#test {
- font-size: 5vh;
width: 50vw;
}
+#testfontsize {
+ font-size: 5vh;
+}
+#testcalc {
+ width: calc(50vw * 2);
+}
+#testfontsizecalc {
+ font-size: calc(5vh * 2);
+}
#testpseudo:after {
margin-left: 20vmin;
padding-right: 25vmax;
content: '';
}
+#testpseudocalc:after {
+ margin-left: calc(20vmin * 2);
+ padding-right: calc(25vmax * 2);
+ content: '';
+}
</style>
<body>
<div id="test"></div>
+ <div id="testfontsize"></div>
+ <div id="testcalc"></div>
+ <div id="testfontsizecalc"></div>
<div id="testpseudo"></div>
+ <div id="testpseudocalc"></div>
</body>
<script src=""
<script src=""
@@ -21,10 +38,14 @@
standardResizeTest(function () {
var min = Math.min(innerWidth, innerHeight);
var max = Math.max(innerWidth, innerHeight);
- shouldBeEqualToString("getComputedStyle(test).fontSize", innerHeight / 20 + "px");
shouldBeEqualToString("getComputedStyle(test).width", innerWidth / 2 + "px");
+ shouldBeEqualToString("getComputedStyle(testfontsize).fontSize", innerHeight / 20 + "px");
+ shouldBeEqualToString("getComputedStyle(testcalc).width", innerWidth + "px");
+ shouldBeEqualToString("getComputedStyle(testfontsizecalc).fontSize", innerHeight / 10 + "px");
shouldBeEqualToString("getComputedStyle(testpseudo, ':after').marginLeft", min / 5 + "px");
shouldBeEqualToString("getComputedStyle(testpseudo, ':after').paddingRight", max / 4 + "px");
+ shouldBeEqualToString("getComputedStyle(testpseudocalc, ':after').marginLeft", (min * 2) / 5 + "px");
+ shouldBeEqualToString("getComputedStyle(testpseudocalc, ':after').paddingRight", max / 2 + "px");
});
</script>
<script src=""
Modified: trunk/Source/WebCore/ChangeLog (276186 => 276187)
--- trunk/Source/WebCore/ChangeLog 2021-04-17 00:57:21 UTC (rev 276186)
+++ trunk/Source/WebCore/ChangeLog 2021-04-17 01:08:45 UTC (rev 276187)
@@ -1,3 +1,35 @@
+2021-04-16 Darin Adler <[email protected]>
+
+ 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.
+
+ * 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.
+
2021-04-16 Ian Gilbert <[email protected]>
Nullptr deref in CompositeEditCommand::isRemovableBlock in DeleteSelectionCommand::removeRedundantBlocks
Modified: trunk/Source/WebCore/css/CSSToLengthConversionData.cpp (276186 => 276187)
--- trunk/Source/WebCore/css/CSSToLengthConversionData.cpp 2021-04-17 00:57:21 UTC (rev 276186)
+++ trunk/Source/WebCore/css/CSSToLengthConversionData.cpp 2021-04-17 01:08:45 UTC (rev 276187)
@@ -38,15 +38,15 @@
float CSSToLengthConversionData::zoom() const
{
- if (m_useEffectiveZoom)
+ if (!m_zoom)
return m_style ? m_style->effectiveZoom() : 1;
- return m_zoom;
+ return *m_zoom;
}
double CSSToLengthConversionData::viewportWidthFactor() const
{
- if (m_style && !computingFontSize())
- const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
+ if (m_viewportDependencyDetectionStyle)
+ m_viewportDependencyDetectionStyle->setHasViewportUnits();
if (!m_renderView)
return 0;
@@ -56,8 +56,8 @@
double CSSToLengthConversionData::viewportHeightFactor() const
{
- if (m_style && !computingFontSize())
- const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
+ if (m_viewportDependencyDetectionStyle)
+ m_viewportDependencyDetectionStyle->setHasViewportUnits();
if (!m_renderView)
return 0;
@@ -67,8 +67,8 @@
double CSSToLengthConversionData::viewportMinFactor() const
{
- if (m_style && !computingFontSize())
- const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
+ if (m_viewportDependencyDetectionStyle)
+ m_viewportDependencyDetectionStyle->setHasViewportUnits();
if (!m_renderView)
return 0;
@@ -79,8 +79,8 @@
double CSSToLengthConversionData::viewportMaxFactor() const
{
- if (m_style && !computingFontSize())
- const_cast<RenderStyle*>(m_style)->setHasViewportUnits();
+ if (m_viewportDependencyDetectionStyle)
+ m_viewportDependencyDetectionStyle->setHasViewportUnits();
if (!m_renderView)
return 0;
Modified: trunk/Source/WebCore/css/CSSToLengthConversionData.h (276186 => 276187)
--- trunk/Source/WebCore/css/CSSToLengthConversionData.h 2021-04-17 00:57:21 UTC (rev 276186)
+++ trunk/Source/WebCore/css/CSSToLengthConversionData.h 2021-04-17 01:08:45 UTC (rev 276187)
@@ -41,13 +41,13 @@
class CSSToLengthConversionData {
public:
- CSSToLengthConversionData(const RenderStyle* style, const RenderStyle* rootStyle, const RenderStyle* parentStyle, const RenderView* renderView, float zoom, Optional<CSSPropertyID> propertyToCompute = WTF::nullopt)
+ CSSToLengthConversionData(const RenderStyle* style, const RenderStyle* rootStyle, const RenderStyle* parentStyle, const RenderView* renderView, float zoom, Optional<CSSPropertyID> propertyToCompute = WTF::nullopt, RenderStyle* viewportDependencyDetectionStyle = nullptr)
: m_style(style)
, m_rootStyle(rootStyle)
, m_parentStyle(parentStyle)
+ , m_viewportDependencyDetectionStyle(viewportDependencyDetectionStyle ? viewportDependencyDetectionStyle : const_cast<RenderStyle*>(style))
, m_renderView(renderView)
, m_zoom(zoom)
- , m_useEffectiveZoom(false)
, m_propertyToCompute(propertyToCompute)
{
ASSERT(zoom > 0);
@@ -57,17 +57,13 @@
: m_style(style)
, m_rootStyle(rootStyle)
, m_parentStyle(parentStyle)
+ , m_viewportDependencyDetectionStyle(const_cast<RenderStyle*>(style))
, m_renderView(renderView)
- , m_zoom(1)
- , m_useEffectiveZoom(true)
, m_propertyToCompute(propertyToCompute)
{
}
- CSSToLengthConversionData()
- : CSSToLengthConversionData(nullptr, nullptr, nullptr, nullptr)
- {
- }
+ CSSToLengthConversionData() = default;
const RenderStyle* style() const { return m_style; }
const RenderStyle* rootStyle() const { return m_rootStyle; }
@@ -94,12 +90,12 @@
}
private:
- const RenderStyle* m_style;
- const RenderStyle* m_rootStyle;
- const RenderStyle* m_parentStyle;
- const RenderView* m_renderView;
- float m_zoom;
- bool m_useEffectiveZoom;
+ const RenderStyle* m_style { nullptr };
+ const RenderStyle* m_rootStyle { nullptr };
+ const RenderStyle* m_parentStyle { nullptr };
+ RenderStyle* m_viewportDependencyDetectionStyle { nullptr };
+ const RenderView* m_renderView { nullptr };
+ Optional<float> m_zoom;
Optional<CSSPropertyID> m_propertyToCompute;
};
Modified: trunk/Source/WebCore/style/StyleBuilderCustom.h (276186 => 276187)
--- trunk/Source/WebCore/style/StyleBuilderCustom.h 2021-04-17 00:57:21 UTC (rev 276186)
+++ trunk/Source/WebCore/style/StyleBuilderCustom.h 2021-04-17 01:08:45 UTC (rev 276187)
@@ -1831,15 +1831,14 @@
} else {
fontDescription.setIsAbsoluteSize(parentIsAbsoluteSize || !(primitiveValue.isPercentage() || primitiveValue.isFontRelativeLength()));
if (primitiveValue.isLength()) {
- size = primitiveValue.computeLength<float>(CSSToLengthConversionData(&builderState.parentStyle(), builderState.rootElementStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize));
- if (primitiveValue.isViewportPercentageLength())
- builderState.style().setHasViewportUnits();
+ CSSToLengthConversionData conversionData { &builderState.parentStyle(), builderState.rootElementStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize, &builderState.style() };
+ size = primitiveValue.computeLength<float>(conversionData);
} else if (primitiveValue.isPercentage())
size = (primitiveValue.floatValue() * parentSize) / 100.0f;
else if (primitiveValue.isCalculatedPercentageWithLength()) {
- const auto& conversionData = builderState.cssToLengthConversionData();
- CSSToLengthConversionData parentConversionData { &builderState.parentStyle(), conversionData.rootStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize };
- size = primitiveValue.cssCalcValue()->createCalculationValue(parentConversionData)->evaluate(parentSize);
+ // FIXME: Why does this need a different root style than the isLength case above?
+ CSSToLengthConversionData conversionData { &builderState.parentStyle(), builderState.cssToLengthConversionData().rootStyle(), &builderState.parentStyle(), builderState.document().renderView(), 1.0f, CSSPropertyFontSize, &builderState.style() };
+ size = primitiveValue.cssCalcValue()->createCalculationValue(conversionData)->evaluate(parentSize);
} else
return;
}
