- Revision
- 274170
- Author
- [email protected]
- Date
- 2021-03-09 13:17:43 -0800 (Tue, 09 Mar 2021)
Log Message
REGRESSION (r273003): Animated style may lose original display property value
https://bugs.webkit.org/show_bug.cgi?id=222979
rdar://75056684
Reviewed by Zalan Bujtas.
Source/WebCore:
Test: fast/animation/animation-display-style-adjustment.html
The original (non-blockified) display property value is saved in the beginning of Style::Adjuster::adjust.
It is needed to implement absolute positioning correctly in some situations. However with animations
the style adjustment code may run twice on the same style and the second run will clobber the saved original value.
* rendering/RenderTheme.cpp:
(WebCore::RenderTheme::adjustStyle):
* rendering/style/RenderStyle.h:
(WebCore::RenderStyle::setDisplay):
Always save the original value when setting the property normally.
(WebCore::RenderStyle::setEffectiveDisplay):
(WebCore::RenderStyle::setOriginalDisplay): Deleted.
Add setEffectiveDisplay that doesn't affect the original value for adjuster use.
* style/StyleAdjuster.cpp:
(WebCore::Style::Adjuster::adjust const):
Remove the saving of the original value.
Use setEffectiveDisplay in all adjuster code, preserving the original value.
(WebCore::Style::Adjuster::adjustDisplayContentsStyle const):
(WebCore::Style::Adjuster::adjustSVGElementStyle):
(WebCore::Style::Adjuster::adjustForSiteSpecificQuirks const):
LayoutTests:
* fast/animation/animation-display-style-adjustment-expected.html: Added.
* fast/animation/animation-display-style-adjustment.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (274169 => 274170)
--- trunk/LayoutTests/ChangeLog 2021-03-09 20:52:33 UTC (rev 274169)
+++ trunk/LayoutTests/ChangeLog 2021-03-09 21:17:43 UTC (rev 274170)
@@ -1,3 +1,14 @@
+2021-03-09 Antti Koivisto <[email protected]>
+
+ REGRESSION (r273003): Animated style may lose original display property value
+ https://bugs.webkit.org/show_bug.cgi?id=222979
+ rdar://75056684
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/animation/animation-display-style-adjustment-expected.html: Added.
+ * fast/animation/animation-display-style-adjustment.html: Added.
+
2021-03-09 Antoine Quint <[email protected]>
[Web Animations] setKeyframes does not preserve animation's current offset
Added: trunk/LayoutTests/fast/animation/animation-display-style-adjustment-expected.html (0 => 274170)
--- trunk/LayoutTests/fast/animation/animation-display-style-adjustment-expected.html (rev 0)
+++ trunk/LayoutTests/fast/animation/animation-display-style-adjustment-expected.html 2021-03-09 21:17:43 UTC (rev 274170)
@@ -0,0 +1,18 @@
+<style>
+.test {
+ text-align: center;
+ border: 2px solid green;
+ height:100px;
+}
+.test > span {
+ top: 40px;
+ position: absolute;
+ width: 64px;
+ height: 64px;
+ background-color: #ff4500;
+ border-radius: 100%;
+}
+</style>
+<body>
+<div class=test><span></span>centered</div>
+</body>
Added: trunk/LayoutTests/fast/animation/animation-display-style-adjustment.html (0 => 274170)
--- trunk/LayoutTests/fast/animation/animation-display-style-adjustment.html (rev 0)
+++ trunk/LayoutTests/fast/animation/animation-display-style-adjustment.html 2021-03-09 21:17:43 UTC (rev 274170)
@@ -0,0 +1,25 @@
+<style>
+@keyframes scaleout {
+ 0% {
+ }
+ to {
+ }
+}
+.test {
+ text-align: center;
+ border: 2px solid green;
+ height:100px;
+}
+.test > span {
+ top: 40px;
+ position: absolute;
+ width: 64px;
+ height: 64px;
+ background-color: #ff4500;
+ border-radius: 100%;
+ animation:scaleout 1.5s infinite ease-in-out
+}
+</style>
+<body>
+<div class=test><span></span>centered</div>
+</body>
Modified: trunk/Source/WebCore/ChangeLog (274169 => 274170)
--- trunk/Source/WebCore/ChangeLog 2021-03-09 20:52:33 UTC (rev 274169)
+++ trunk/Source/WebCore/ChangeLog 2021-03-09 21:17:43 UTC (rev 274170)
@@ -1,3 +1,39 @@
+2021-03-09 Antti Koivisto <[email protected]>
+
+ REGRESSION (r273003): Animated style may lose original display property value
+ https://bugs.webkit.org/show_bug.cgi?id=222979
+ rdar://75056684
+
+ Reviewed by Zalan Bujtas.
+
+ Test: fast/animation/animation-display-style-adjustment.html
+
+ The original (non-blockified) display property value is saved in the beginning of Style::Adjuster::adjust.
+ It is needed to implement absolute positioning correctly in some situations. However with animations
+ the style adjustment code may run twice on the same style and the second run will clobber the saved original value.
+
+ * rendering/RenderTheme.cpp:
+ (WebCore::RenderTheme::adjustStyle):
+ * rendering/style/RenderStyle.h:
+ (WebCore::RenderStyle::setDisplay):
+
+ Always save the original value when setting the property normally.
+
+ (WebCore::RenderStyle::setEffectiveDisplay):
+ (WebCore::RenderStyle::setOriginalDisplay): Deleted.
+
+ Add setEffectiveDisplay that doesn't affect the original value for adjuster use.
+
+ * style/StyleAdjuster.cpp:
+ (WebCore::Style::Adjuster::adjust const):
+
+ Remove the saving of the original value.
+ Use setEffectiveDisplay in all adjuster code, preserving the original value.
+
+ (WebCore::Style::Adjuster::adjustDisplayContentsStyle const):
+ (WebCore::Style::Adjuster::adjustSVGElementStyle):
+ (WebCore::Style::Adjuster::adjustForSiteSpecificQuirks const):
+
2021-03-09 Sam Weinig <[email protected]>
Remove CSSParserContext::enforcesCSSMIMETypeInNoQuirksMode as it is unused
Modified: trunk/Source/WebCore/rendering/RenderTheme.cpp (274169 => 274170)
--- trunk/Source/WebCore/rendering/RenderTheme.cpp 2021-03-09 20:52:33 UTC (rev 274169)
+++ trunk/Source/WebCore/rendering/RenderTheme.cpp 2021-03-09 21:17:43 UTC (rev 274170)
@@ -82,9 +82,9 @@
|| style.display() == DisplayType::TableHeaderGroup || style.display() == DisplayType::TableFooterGroup
|| style.display() == DisplayType::TableRow || style.display() == DisplayType::TableColumnGroup || style.display() == DisplayType::TableColumn
|| style.display() == DisplayType::TableCell || style.display() == DisplayType::TableCaption)
- style.setDisplay(DisplayType::InlineBlock);
+ style.setEffectiveDisplay(DisplayType::InlineBlock);
else if (style.display() == DisplayType::ListItem || style.display() == DisplayType::Table)
- style.setDisplay(DisplayType::Block);
+ style.setEffectiveDisplay(DisplayType::Block);
if (userAgentAppearanceStyle && isControlStyled(style, *userAgentAppearanceStyle)) {
switch (part) {
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (274169 => 274170)
--- trunk/Source/WebCore/rendering/style/RenderStyle.h 2021-03-09 20:52:33 UTC (rev 274169)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h 2021-03-09 21:17:43 UTC (rev 274170)
@@ -842,8 +842,12 @@
// attribute setter methods
- void setDisplay(DisplayType v) { m_nonInheritedFlags.effectiveDisplay = static_cast<unsigned>(v); }
- void setOriginalDisplay(DisplayType v) { m_nonInheritedFlags.originalDisplay = static_cast<unsigned>(v); }
+ void setDisplay(DisplayType value)
+ {
+ m_nonInheritedFlags.originalDisplay = static_cast<unsigned>(value);
+ m_nonInheritedFlags.effectiveDisplay = m_nonInheritedFlags.originalDisplay;
+ }
+ void setEffectiveDisplay(DisplayType v) { m_nonInheritedFlags.effectiveDisplay = static_cast<unsigned>(v); }
void setPosition(PositionType v) { m_nonInheritedFlags.position = static_cast<unsigned>(v); }
void setFloating(Float v) { m_nonInheritedFlags.floating = static_cast<unsigned>(v); }
Modified: trunk/Source/WebCore/style/StyleAdjuster.cpp (274169 => 274170)
--- trunk/Source/WebCore/style/StyleAdjuster.cpp 2021-03-09 20:52:33 UTC (rev 274169)
+++ trunk/Source/WebCore/style/StyleAdjuster.cpp 2021-03-09 21:17:43 UTC (rev 274170)
@@ -243,9 +243,6 @@
void Adjuster::adjust(RenderStyle& style, const RenderStyle* userAgentAppearanceStyle) const
{
- // Cache our original display.
- style.setOriginalDisplay(style.display());
-
if (style.display() == DisplayType::Contents)
adjustDisplayContentsStyle(style);
@@ -257,10 +254,10 @@
// these tags to retain their display types.
if (m_document.inQuirksMode()) {
if (m_element->hasTagName(tdTag)) {
- style.setDisplay(DisplayType::TableCell);
+ style.setEffectiveDisplay(DisplayType::TableCell);
style.setFloating(Float::No);
} else if (is<HTMLTableElement>(*m_element))
- style.setDisplay(style.isDisplayInlineType() ? DisplayType::InlineTable : DisplayType::Table);
+ style.setEffectiveDisplay(style.isDisplayInlineType() ? DisplayType::InlineTable : DisplayType::Table);
}
if (m_element->hasTagName(tdTag) || m_element->hasTagName(thTag)) {
@@ -283,7 +280,7 @@
// fix a crash where a site tries to position these objects. They also never honor display.
if (m_element->hasTagName(frameTag) || m_element->hasTagName(framesetTag)) {
style.setPosition(PositionType::Static);
- style.setDisplay(DisplayType::Block);
+ style.setEffectiveDisplay(DisplayType::Block);
}
// Ruby text does not support float or position. This might change with evolution of the specification.
@@ -300,17 +297,17 @@
style.setTextAlign(TextAlignMode::Center);
if (m_element->hasTagName(legendTag))
- style.setDisplay(DisplayType::Block);
+ style.setEffectiveDisplay(DisplayType::Block);
}
// Absolute/fixed positioned elements, floating elements and the document element need block-like outside display.
if (style.hasOutOfFlowPosition() || style.isFloating() || (m_element && m_document.documentElement() == m_element))
- style.setDisplay(equivalentBlockDisplay(style, m_document));
+ style.setEffectiveDisplay(equivalentBlockDisplay(style, m_document));
// FIXME: Don't support this mutation for pseudo styles like first-letter or first-line, since it's not completely
// clear how that should work.
if (style.display() == DisplayType::Inline && style.styleType() == PseudoId::None && style.writingMode() != m_parentStyle.writingMode())
- style.setDisplay(DisplayType::InlineBlock);
+ style.setEffectiveDisplay(DisplayType::InlineBlock);
// After performing the display mutation, check table rows. We do not honor position:relative or position:sticky on
// table rows or cells. This has been established for position:relative in CSS2.1 (and caused a crash in containingBlock()
@@ -337,7 +334,7 @@
// "A parent with a grid or flex display value blockifies the box’s display type."
if (m_parentBoxStyle.isDisplayFlexibleOrGridBox()) {
style.setFloating(Float::No);
- style.setDisplay(equivalentBlockDisplay(style, m_document));
+ style.setEffectiveDisplay(equivalentBlockDisplay(style, m_document));
}
}
@@ -544,17 +541,17 @@
{
if (!m_element) {
if (style.styleType() != PseudoId::Before && style.styleType() != PseudoId::After)
- style.setDisplay(DisplayType::None);
+ style.setEffectiveDisplay(DisplayType::None);
return;
}
if (m_document.documentElement() == m_element) {
- style.setDisplay(DisplayType::Block);
+ style.setEffectiveDisplay(DisplayType::Block);
return;
}
if (hasEffectiveDisplayNoneForDisplayContents(*m_element))
- style.setDisplay(DisplayType::None);
+ style.setEffectiveDisplay(DisplayType::None);
}
void Adjuster::adjustSVGElementStyle(RenderStyle& style, const SVGElement& svgElement)
@@ -571,7 +568,7 @@
// SVG text layout code expects us to be a block-level style element.
if ((svgElement.hasTagName(SVGNames::foreignObjectTag) || svgElement.hasTagName(SVGNames::textTag)) && style.isDisplayInlineType())
- style.setDisplay(DisplayType::Block);
+ style.setEffectiveDisplay(DisplayType::Block);
}
void Adjuster::adjustAnimatedStyle(RenderStyle& style, OptionSet<AnimationImpact> impact) const
@@ -630,7 +627,7 @@
if (div.hasClass() && div.classNames().contains(instreamNativeVideoDivClass)) {
auto* video = div.treeScope().getElementById(videoElementID);
if (is<HTMLVideoElement>(video) && downcast<HTMLVideoElement>(*video).isFullscreen())
- style.setDisplay(DisplayType::Block);
+ style.setEffectiveDisplay(DisplayType::Block);
}
}
}
