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

Changeset 274170 in webkit


Ignore:
Timestamp:
Mar 9, 2021, 1:17:43 PM (6 years ago)
Author:
Antti Koivisto
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.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r274165 r274170  
     12021-03-09  Antti Koivisto  <antti@apple.com>
     2
     3        REGRESSION (r273003): Animated style may lose original display property value
     4        https://bugs.webkit.org/show_bug.cgi?id=222979
     5        rdar://75056684
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        * fast/animation/animation-display-style-adjustment-expected.html: Added.
     10        * fast/animation/animation-display-style-adjustment.html: Added.
     11
    1122021-03-09  Antoine Quint  <graouts@webkit.org>
    213

  • trunk/Source/WebCore/ChangeLog

    r274168 r274170  
     12021-03-09  Antti Koivisto  <antti@apple.com>
     2
     3        REGRESSION (r273003): Animated style may lose original display property value
     4        https://bugs.webkit.org/show_bug.cgi?id=222979
     5        rdar://75056684
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Test: fast/animation/animation-display-style-adjustment.html
     10
     11        The original (non-blockified) display property value is saved in the beginning of Style::Adjuster::adjust.
     12        It is needed to implement absolute positioning correctly in some situations. However with animations
     13        the style adjustment code may run twice on the same style and the second run will clobber the saved original value.
     14
     15        * rendering/RenderTheme.cpp:
     16        (WebCore::RenderTheme::adjustStyle):
     17        * rendering/style/RenderStyle.h:
     18        (WebCore::RenderStyle::setDisplay):
     19
     20        Always save the original value when setting the property normally.
     21
     22        (WebCore::RenderStyle::setEffectiveDisplay):
     23        (WebCore::RenderStyle::setOriginalDisplay): Deleted.
     24
     25        Add setEffectiveDisplay that doesn't affect the original value for adjuster use.
     26
     27        * style/StyleAdjuster.cpp:
     28        (WebCore::Style::Adjuster::adjust const):
     29
     30        Remove the saving of the original value.
     31        Use setEffectiveDisplay in all adjuster code, preserving the original value.
     32
     33        (WebCore::Style::Adjuster::adjustDisplayContentsStyle const):
     34        (WebCore::Style::Adjuster::adjustSVGElementStyle):
     35        (WebCore::Style::Adjuster::adjustForSiteSpecificQuirks const):
     36
    1372021-03-09  Sam Weinig  <weinig@apple.com>
    238

  • trunk/Source/WebCore/rendering/RenderTheme.cpp

    r273683 r274170  
    8383        || style.display() == DisplayType::TableRow || style.display() == DisplayType::TableColumnGroup || style.display() == DisplayType::TableColumn
    8484        || style.display() == DisplayType::TableCell || style.display() == DisplayType::TableCaption)
    85         style.setDisplay(DisplayType::InlineBlock);
     85        style.setEffectiveDisplay(DisplayType::InlineBlock);
    8686    else if (style.display() == DisplayType::ListItem || style.display() == DisplayType::Table)
    87         style.setDisplay(DisplayType::Block);
     87        style.setEffectiveDisplay(DisplayType::Block);
    8888
    8989    if (userAgentAppearanceStyle && isControlStyled(style, *userAgentAppearanceStyle)) {

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

    r274050 r274170  
    843843// attribute setter methods
    844844
    845     void setDisplay(DisplayType v) { m_nonInheritedFlags.effectiveDisplay = static_cast<unsigned>(v); }
    846     void setOriginalDisplay(DisplayType v) { m_nonInheritedFlags.originalDisplay = static_cast<unsigned>(v); }
     845    void setDisplay(DisplayType value)
     846    {
     847        m_nonInheritedFlags.originalDisplay = static_cast<unsigned>(value);
     848        m_nonInheritedFlags.effectiveDisplay = m_nonInheritedFlags.originalDisplay;
     849    }
     850    void setEffectiveDisplay(DisplayType v) { m_nonInheritedFlags.effectiveDisplay = static_cast<unsigned>(v); }
    847851    void setPosition(PositionType v) { m_nonInheritedFlags.position = static_cast<unsigned>(v); }
    848852    void setFloating(Float v) { m_nonInheritedFlags.floating = static_cast<unsigned>(v); }

  • trunk/Source/WebCore/style/StyleAdjuster.cpp

    r273003 r274170  
    244244void Adjuster::adjust(RenderStyle& style, const RenderStyle* userAgentAppearanceStyle) const
    245245{
    246     // Cache our original display.
    247     style.setOriginalDisplay(style.display());
    248 
    249246    if (style.display() == DisplayType::Contents)
    250247        adjustDisplayContentsStyle(style);
    … …  
    258255            if (m_document.inQuirksMode()) {
    259256                if (m_element->hasTagName(tdTag)) {
    260                     style.setDisplay(DisplayType::TableCell);
     257                    style.setEffectiveDisplay(DisplayType::TableCell);
    261258                    style.setFloating(Float::No);
    262259                } else if (is<HTMLTableElement>(*m_element))
    263                     style.setDisplay(style.isDisplayInlineType() ? DisplayType::InlineTable : DisplayType::Table);
     260                    style.setEffectiveDisplay(style.isDisplayInlineType() ? DisplayType::InlineTable : DisplayType::Table);
    264261            }
    265262
    … …  
    284281            if (m_element->hasTagName(frameTag) || m_element->hasTagName(framesetTag)) {
    285282                style.setPosition(PositionType::Static);
    286                 style.setDisplay(DisplayType::Block);
     283                style.setEffectiveDisplay(DisplayType::Block);
    287284            }
    288285
    … …  
    301298
    302299            if (m_element->hasTagName(legendTag))
    303                 style.setDisplay(DisplayType::Block);
     300                style.setEffectiveDisplay(DisplayType::Block);
    304301        }
    305302
    306303        // Absolute/fixed positioned elements, floating elements and the document element need block-like outside display.
    307304        if (style.hasOutOfFlowPosition() || style.isFloating() || (m_element && m_document.documentElement() == m_element))
    308             style.setDisplay(equivalentBlockDisplay(style, m_document));
     305            style.setEffectiveDisplay(equivalentBlockDisplay(style, m_document));
    309306
    310307        // FIXME: Don't support this mutation for pseudo styles like first-letter or first-line, since it's not completely
    311308        // clear how that should work.
    312309        if (style.display() == DisplayType::Inline && style.styleType() == PseudoId::None && style.writingMode() != m_parentStyle.writingMode())
    313             style.setDisplay(DisplayType::InlineBlock);
     310            style.setEffectiveDisplay(DisplayType::InlineBlock);
    314311
    315312        // After performing the display mutation, check table rows. We do not honor position:relative or position:sticky on
    … …  
    338335        if (m_parentBoxStyle.isDisplayFlexibleOrGridBox()) {
    339336            style.setFloating(Float::No);
    340             style.setDisplay(equivalentBlockDisplay(style, m_document));
     337            style.setEffectiveDisplay(equivalentBlockDisplay(style, m_document));
    341338        }
    342339    }
    … …  
    545542    if (!m_element) {
    546543        if (style.styleType() != PseudoId::Before && style.styleType() != PseudoId::After)
    547             style.setDisplay(DisplayType::None);
     544            style.setEffectiveDisplay(DisplayType::None);
    548545        return;
    549546    }
    550547
    551548    if (m_document.documentElement() == m_element) {
    552         style.setDisplay(DisplayType::Block);
     549        style.setEffectiveDisplay(DisplayType::Block);
    553550        return;
    554551    }
    555552
    556553    if (hasEffectiveDisplayNoneForDisplayContents(*m_element))
    557         style.setDisplay(DisplayType::None);
     554        style.setEffectiveDisplay(DisplayType::None);
    558555}
    559556
    … …  
    572569    // SVG text layout code expects us to be a block-level style element.
    573570    if ((svgElement.hasTagName(SVGNames::foreignObjectTag) || svgElement.hasTagName(SVGNames::textTag)) && style.isDisplayInlineType())
    574         style.setDisplay(DisplayType::Block);
     571        style.setEffectiveDisplay(DisplayType::Block);
    575572}
    576573
    … …  
    631628                auto* video = div.treeScope().getElementById(videoElementID);
    632629                if (is<HTMLVideoElement>(video) && downcast<HTMLVideoElement>(*video).isFullscreen())
    633                     style.setDisplay(DisplayType::Block);
     630                    style.setEffectiveDisplay(DisplayType::Block);
    634631            }
    635632        }
Note: See TracChangeset for help on using the changeset viewer.