Changeset 198990 in webkit


Ignore:
Timestamp:
Apr 3, 2016 1:19:47 PM (8 years ago)
Author:
Antti Koivisto
Message:

Shadow DOM: Slot style is not computed
​https://bugs.webkit.org/show_bug.cgi?id=156144

Reviewed by Darin Adler.

Source/WebCore:

We don’t currently compute style for active slots. While slots have have implicit display:contents and don’t create
boxes themselves the style should still inherit to slotted children.

Basically

<slot style=“color:red”></slot>

should work as expected.

The implementation falls out from the new style resolve architecture and this patch mostly just removes the special
case code that prevented this from working.

Test: fast/shadow-dom/css-scoping-shadow-slot-style.html

  • html/HTMLSlotElement.h:

(WebCore::hasImplicitDisplayContents):

Move to a shared location.

  • style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::RenderTreeUpdater):
(WebCore::findRenderingRoot):
(WebCore::RenderTreeUpdater::updateRenderTree):

Remove the special case code. What remains is a display:contents test for rendererless elements.

(WebCore::RenderTreeUpdater::updateElementRenderer):

Don't create renderers for (implicit) display:contents.

(WebCore::hasDisplayContents): Deleted.

  • style/StyleTreeResolver.cpp:

(WebCore::Style::detachRenderTree):
(WebCore::Style::affectsRenderedSubtree):

Factor into a function.

(WebCore::Style::TreeResolver::resolveElement):

Remove the special case code.

(WebCore::Style::TreeResolver::resolveComposedTree):

Always resolve slots as we don't currently save their style.

LayoutTests:

  • fast/shadow-dom/css-scoping-shadow-slot-style-expected.html: Added.
  • fast/shadow-dom/css-scoping-shadow-slot-style.html: Added.
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r198989 r198990  
     12016-04-03  Antti Koivisto  <antti@apple.com>
     2
     3        Shadow DOM: Slot style is not computed
     4        https://bugs.webkit.org/show_bug.cgi?id=156144
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/shadow-dom/css-scoping-shadow-slot-style-expected.html: Added.
     9        * fast/shadow-dom/css-scoping-shadow-slot-style.html: Added.
     10
    1112016-04-03  Saam barati  <sbarati@apple.com>
    212

  • trunk/LayoutTests/editing/execCommand/justify-right-then-indent-with-problematic-body-expected.txt

    r192477 r198990  
    11Pass.
     2
    23WebKit didn't Crash.
    34

  • trunk/Source/WebCore/ChangeLog

    r198986 r198990  
     12016-04-03  Antti Koivisto  <antti@apple.com>
     2
     3        Shadow DOM: Slot style is not computed
     4        https://bugs.webkit.org/show_bug.cgi?id=156144
     5
     6        Reviewed by Darin Adler.
     7
     8        We don’t currently compute style for active slots. While slots have have implicit display:contents and don’t create
     9        boxes themselves the style should still inherit to slotted children.
     10
     11        Basically
     12
     13        <slot style=“color:red”></slot>
     14
     15        should work as expected.
     16
     17        The implementation falls out from the new style resolve architecture and this patch mostly just removes the special
     18        case code that prevented this from working.
     19
     20        Test: fast/shadow-dom/css-scoping-shadow-slot-style.html
     21
     22        * html/HTMLSlotElement.h:
     23        (WebCore::hasImplicitDisplayContents):
     24
     25            Move to a shared location.
     26
     27        * style/RenderTreeUpdater.cpp:
     28        (WebCore::RenderTreeUpdater::RenderTreeUpdater):
     29        (WebCore::findRenderingRoot):
     30        (WebCore::RenderTreeUpdater::updateRenderTree):
     31
     32            Remove the special case code. What remains is a display:contents test for rendererless elements.
     33
     34        (WebCore::RenderTreeUpdater::updateElementRenderer):
     35
     36            Don't create renderers for (implicit) display:contents.
     37
     38        (WebCore::hasDisplayContents): Deleted.
     39        * style/StyleTreeResolver.cpp:
     40        (WebCore::Style::detachRenderTree):
     41        (WebCore::Style::affectsRenderedSubtree):
     42
     43            Factor into a function.
     44
     45        (WebCore::Style::TreeResolver::resolveElement):
     46
     47            Remove the special case code.
     48
     49        (WebCore::Style::TreeResolver::resolveComposedTree):
     50
     51            Always resolve slots as we don't currently save their style.
     52
    1532016-04-03  David Kilzer  <ddkilzer@apple.com>
    254

  • trunk/Source/WebCore/html/HTMLSlotElement.h

    r198115 r198990  
    5454};
    5555
     56// Slots have implicit display:contents until it is supported for reals.
     57inline bool hasImplicitDisplayContents(const Element& element) { return is<HTMLSlotElement>(element); }
     58
     59}
     60
     61#else
     62
     63namespace WebCore {
     64
     65inline bool hasImplicitDisplayContents(const Element&) { return false; }
     66
    5667}
    5768

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

    r198943 r198990  
    9090    while (it != end) {
    9191        auto& node = *it;
    92         bool hasDisplayContents = is<HTMLSlotElement>(node);
     92        bool hasDisplayContents = is<Element>(node) && hasImplicitDisplayContents(downcast<Element>(node));
    9393        if (hasDisplayContents) {
    9494            it.traverseNext();

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

    r198943 r198990  
    6262}
    6363
    64 // Slots have implicit display:contents until it is supported for reals.
    65 static bool hasDisplayContents(const Node& node)
    66 {
    67     return is<HTMLSlotElement>(node);
    68 }
    69 
    7064static ContainerNode& findRenderingRoot(ContainerNode& node)
    7165{
    … …  
    7468        if (it->renderer())
    7569            return *it;
    76         ASSERT(hasDisplayContents(*it));
     70        ASSERT(hasImplicitDisplayContents(downcast<Element>(*it)));
    7771    }
    7872    ASSERT_NOT_REACHED();
    … …  
    138132
    139133        auto* elementUpdate = m_styleUpdate->elementUpdate(element);
    140 
    141         auto changeType = Style::NoChange;
    142         if (elementUpdate) {
    143             if (hasDisplayContents(element)) {
    144                 if (!shouldCreateRenderer(element, renderTreePosition().parent())) {
    145                     it.traverseNextSkippingChildren();
    146                     continue;
    147                 }
    148                 pushParent(element, parent().styleChange);
    149                 it.traverseNext();
    150                 continue;
    151             }
    152 
    153             updateElementRenderer(element, *elementUpdate);
    154             changeType = elementUpdate->change;
    155         }
    156 
    157         if (!element.renderer() || !elementUpdate) {
     134        if (!elementUpdate) {
    158135            it.traverseNextSkippingChildren();
    159136            continue;
    160137        }
    161138
    162         pushParent(element, changeType);
     139        updateElementRenderer(element, *elementUpdate);
     140
     141        bool mayHaveRenderedDescendants = element.renderer() || (hasImplicitDisplayContents(element) && shouldCreateRenderer(element, renderTreePosition().parent()));
     142        if (!mayHaveRenderedDescendants) {
     143            it.traverseNextSkippingChildren();
     144            continue;
     145        }
     146
     147        pushParent(element, elementUpdate ? elementUpdate->change : Style::NoChange);
    163148
    164149        it.traverseNext();
    … …  
    247232        detachRenderTree(element, Style::ReattachDetach);
    248233
    249     bool shouldCreateNewRenderer = !element.renderer() && update.style;
     234    bool shouldCreateNewRenderer = !element.renderer() && update.style && !hasImplicitDisplayContents(element);
    250235    if (shouldCreateNewRenderer) {
    251236        if (element.hasCustomStyleResolveCallbacks())

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

    r198943 r198990  
    386386}
    387387
     388static bool affectsRenderedSubtree(Element& element, const RenderStyle& newStyle)
     389{
     390    if (element.renderer())
     391        return true;
     392    if (newStyle.display() != NONE)
     393        return true;
     394    // FIXME: Make 'contents' an actual display property value.
     395    if (hasImplicitDisplayContents(element))
     396        return true;
     397    if (element.rendererIsNeeded(newStyle))
     398        return true;
     399    if (element.shouldMoveToFlowThread(newStyle))
     400        return true;
     401    return false;
     402}
     403
    388404ElementUpdate TreeResolver::resolveElement(Element& element)
    389405{
    … …  
    392408    auto* renderer = element.renderer();
    393409
    394     bool affectsRenderedSubtree = renderer || newStyle->display() != NONE || element.rendererIsNeeded(newStyle) || element.shouldMoveToFlowThread(newStyle);
    395     if (!affectsRenderedSubtree)
     410    if (!affectsRenderedSubtree(element, newStyle.get()))
    396411        return { };
    397412
    … …  
    600615        update.style = element.renderStyle();
    601616
    602         bool shouldResolve = parent.change >= Inherit || element.needsStyleRecalc() || shouldResolveForPseudoElement || affectedByPreviousSibling;
     617        bool shouldResolve = parent.change >= Inherit || element.needsStyleRecalc() || shouldResolveForPseudoElement || affectedByPreviousSibling || hasImplicitDisplayContents(element);
    603618        if (shouldResolve) {
    604619#if PLATFORM(IOS)
    … …  
    628643        }
    629644
    630 
    631 #if ENABLE(SHADOW_DOM) || ENABLE(DETAILS_ELEMENT)
    632         if (is<HTMLSlotElement>(element)) {
    633             // FIXME: We should compute style for the slot and use it as parent style.
    634             // Duplicate the style from the parent context.
    635             ElementUpdate slotUpdate;
    636             slotUpdate.style = parent.style.ptr();
    637             slotUpdate.change = update.change;
    638             if (!shouldResolve)
    639                 m_update->addElement(element, parent.element, update);
    640             pushParent(element, slotUpdate);
    641             it.traverseNext();
    642             continue;
    643         }
    644 #endif
    645645        if (!update.style) {
    646646            resetStyleForNonRenderedDescendants(element);
Note: See TracChangeset for help on using the changeset viewer.