Changeset 191262 in webkit


Ignore:
Timestamp:
Oct 18, 2015, 3:15:18 PM (10 years ago)
Author:
Antti Koivisto
Message:

Computed style should work correctly with slotted elements that have display:none
​https://bugs.webkit.org/show_bug.cgi?id=150237

Source/WebCore:

Reviewed by Andreas Kling..

If an element has display:none we don't normally retain or even compute its style (as it is not rendered).
If getComputedStyle is invoked for such element we resolve the style (along with any ancestors) and cache
it separately to rare data. This path needs to work with slotted elements in shadow trees.

This patch also make computedStyle() iterative rather than recursive.

Test: fast/shadow-dom/computed-style-display-none.html

  • dom/Document.cpp:

(WebCore::Document::styleForElementIgnoringPendingStylesheets):

Pass in the parent style instead of invoking computedStyle() recursively.

  • dom/Document.h:
  • dom/Element.cpp:

(WebCore::beforeOrAfterPseudoElement):
(WebCore::Element::existingComputedStyle):
(WebCore::Element::resolveComputedStyle):

Iterative resolve function that uses composed tree iterator.

(WebCore::Element::computedStyle):

Factor into helpers.

  • dom/Element.h:
  • dom/Node.cpp:

(WebCore::Node::computedStyle):

Use the composed tree iterator.

  • html/HTMLSelectElement.cpp:

(WebCore::HTMLSelectElement::selectOption):

Call updateValidity() before calling renderer->updateFromElement(). Calling updateFromElement()
may end up in Element::computedStyle() which can asserts if validity is not up to date.

LayoutTests:

Reviewed by Andreas Kling.

  • editing/style/apply-style-atomic-expected.txt:

Rebase.

  • fast/css/getComputedStyle/getComputedStyle-with-pseudo-element-expected.txt:
  • fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html:

We now also compute style of display:none pseudo elements correctly.
This is a progression and matches other browsers.

  • fast/shadow-dom/computed-style-display-none-expected.txt: Added.
  • fast/shadow-dom/computed-style-display-none.html: Added.
Location:
trunk
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r191259 r191262  
     12015-10-18  Antti Koivisto  <antti@apple.com>
     2
     3        Computed style should work correctly with slotted elements that have display:none
     4        https://bugs.webkit.org/show_bug.cgi?id=150237
     5
     6        Reviewed by Andreas Kling.
     7
     8        * editing/style/apply-style-atomic-expected.txt:
     9
     10            Rebase.
     11
     12        * fast/css/getComputedStyle/getComputedStyle-with-pseudo-element-expected.txt:
     13        * fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html:
     14
     15            We now also compute style of display:none pseudo elements correctly.
     16            This is a progression and matches other browsers.
     17
     18        * fast/shadow-dom/computed-style-display-none-expected.txt: Added.
     19        * fast/shadow-dom/computed-style-display-none.html: Added.
     20
    1212015-10-18  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
    222

  • trunk/LayoutTests/editing/style/apply-style-atomic-expected.txt

    r191229 r191262  
    33|   href="a"
    44|   "<#selection-anchor>1"
    5 |   <progress>
    6 |     <a>
    7 |       style=""
    8 |       "2"
    9 |     <shadow:root>
     5| <progress>
     6|   <a>
     7|     style=""
     8|     "2"
     9|   <shadow:root>
     10|     <div>
     11|       pseudo="-webkit-progress-inner-element"
     12|       shadow:pseudoId="-webkit-progress-inner-element"
    1013|       <div>
    11 |         pseudo="-webkit-progress-inner-element"
    12 |         shadow:pseudoId="-webkit-progress-inner-element"
     14|         pseudo="-webkit-progress-bar"
     15|         shadow:pseudoId="-webkit-progress-bar"
    1316|         <div>
    14 |           pseudo="-webkit-progress-bar"
    15 |           shadow:pseudoId="-webkit-progress-bar"
    16 |           <div>
    17 |             pseudo="-webkit-progress-value"
    18 |             style="width: -100%;"
    19 |             shadow:pseudoId="-webkit-progress-value"
    20 |   <#selection-focus>
     17|           pseudo="-webkit-progress-value"
     18|           style="width: -100%;"
     19|           shadow:pseudoId="-webkit-progress-value"

  • trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element-expected.txt

    r191229 r191262  
    5252PASS Expected '0px' for margin in the computed style for element with id testBeforeAfterInline and pseudo-element :after and got '0px'
    5353PASS Expected '10px 20px 30px 40px' for padding in the computed style for element with id testBeforeAfterInline and pseudo-element :after and got '10px 20px 30px 40px'
    54 PASS Expected '' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got ''
    55 PASS Expected '' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got ''
    56 PASS Expected '' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got ''
    57 PASS Expected '' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got ''
     54PASS Expected '100px' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got '100px'
     55PASS Expected '100px' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :after and got '100px'
     56PASS Expected '100px' for width in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got '100px'
     57PASS Expected '100px' for height in the computed style for element with id testBeforeAfterDisplayNone and pseudo-element :before and got '100px'
    5858PASS Expected 'rgb(165, 42, 42)' for color in the computed style for element with id testNoPseudoElement and pseudo-element null and got 'rgb(165, 42, 42)'
    5959PASS Expected 'rgb(165, 42, 42)' for color in the computed style for element with id testNoPseudoElement and pseudo-element :first-line and got 'rgb(165, 42, 42)'

  • trunk/LayoutTests/fast/css/getComputedStyle/getComputedStyle-with-pseudo-element.html

    r191229 r191262  
    169169        { 'elementId' : 'testBeforeAfterInline', 'pseudoElement' : ':after', 'property' : 'margin', 'expectedValue' : '0px' },
    170170        { 'elementId' : 'testBeforeAfterInline', 'pseudoElement' : ':after', 'property' : 'padding', 'expectedValue' : '10px 20px 30px 40px' },
    171         { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'width', 'expectedValue' : '' },
    172         { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'height', 'expectedValue' : '' },
    173         { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'width', 'expectedValue' : '' },
    174         { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'height', 'expectedValue' : '' },
     171        { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'width', 'expectedValue' : '100px' },
     172        { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':after', 'property' : 'height', 'expectedValue' : '100px' },
     173        { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'width', 'expectedValue' : '100px' },
     174        { 'elementId' : 'testBeforeAfterDisplayNone', 'pseudoElement' : ':before', 'property' : 'height', 'expectedValue' : '100px' },
    175175        { 'elementId' : 'testNoPseudoElement', 'pseudoElement' : null, 'property' : 'color', 'expectedValue' : 'rgb(165, 42, 42)' },
    176176        { 'elementId' : 'testNoPseudoElement', 'pseudoElement' : ':first-line', 'property' : 'color', 'expectedValue' : 'rgb(165, 42, 42)' },

  • trunk/Source/WebCore/ChangeLog

    r191260 r191262  
     12015-10-18  Antti Koivisto  <antti@apple.com>
     2
     3        Computed style should work correctly with slotted elements that have display:none
     4        https://bugs.webkit.org/show_bug.cgi?id=150237
     5
     6        Reviewed by Andreas Kling..
     7
     8        If an element has display:none we don't normally retain or even compute its style (as it is not rendered).
     9        If getComputedStyle is invoked for such element we resolve the style (along with any ancestors) and cache
     10        it separately to rare data. This path needs to work with slotted elements in shadow trees.
     11
     12        This patch also make computedStyle() iterative rather than recursive.
     13
     14        Test: fast/shadow-dom/computed-style-display-none.html
     15
     16        * dom/Document.cpp:
     17        (WebCore::Document::styleForElementIgnoringPendingStylesheets):
     18
     19            Pass in the parent style instead of invoking computedStyle() recursively.
     20
     21        * dom/Document.h:
     22        * dom/Element.cpp:
     23        (WebCore::beforeOrAfterPseudoElement):
     24        (WebCore::Element::existingComputedStyle):
     25        (WebCore::Element::resolveComputedStyle):
     26
     27            Iterative resolve function that uses composed tree iterator.
     28
     29        (WebCore::Element::computedStyle):
     30
     31            Factor into helpers.
     32
     33        * dom/Element.h:
     34        * dom/Node.cpp:
     35        (WebCore::Node::computedStyle):
     36
     37            Use the composed tree iterator.
     38
     39        * html/HTMLSelectElement.cpp:
     40        (WebCore::HTMLSelectElement::selectOption):
     41
     42            Call updateValidity() before calling renderer->updateFromElement(). Calling updateFromElement()
     43            may end up in Element::computedStyle() which can asserts if validity is not up to date.
     44
    1452015-10-18  Myles C. Maxfield  <mmaxfield@apple.com>
    246

  • trunk/Source/WebCore/dom/Document.cpp

    r191229 r191262  
    19551955}
    19561956
    1957 Ref<RenderStyle> Document::styleForElementIgnoringPendingStylesheets(Element* element)
    1958 {
    1959     ASSERT_ARG(element, &element->document() == this);
     1957Ref<RenderStyle> Document::styleForElementIgnoringPendingStylesheets(Element& element, RenderStyle* parentStyle)
     1958{
     1959    ASSERT(&element.document() == this);
    19601960
    19611961    // On iOS request delegates called during styleForElement may result in re-entering WebKit and killing the style resolver.
    … …  
    19631963
    19641964    TemporaryChange<bool> change(m_ignorePendingStylesheets, true);
    1965     return ensureStyleResolver().styleForElement(element, element->parentNode() ? element->parentNode()->computedStyle() : nullptr);
     1965    return element.resolveStyle(parentStyle);
    19661966}
    19671967

  • trunk/Source/WebCore/dom/Document.h

    r191229 r191262  
    571571    WEBCORE_EXPORT void updateLayoutIgnorePendingStylesheets(RunPostLayoutTasks = RunPostLayoutTasks::Asynchronously);
    572572
    573     Ref<RenderStyle> styleForElementIgnoringPendingStylesheets(Element*);
     573    Ref<RenderStyle> styleForElementIgnoringPendingStylesheets(Element&, RenderStyle* parentStyle);
    574574
    575575    // Returns true if page box (margin boxes and page borders) is visible.

  • trunk/Source/WebCore/dom/Element.cpp

    r191229 r191262  
    3434#include "ClientRect.h"
    3535#include "ClientRectList.h"
     36#include "ComposedTreeAncestorIterator.h"
    3637#include "ContainerNodeAlgorithms.h"
    3738#include "DOMTokenList.h"
    … …  
    24352436}
    24362437
    2437 static PseudoElement* beforeOrAfterPseudoElement(Element* host, PseudoId pseudoElementSpecifier)
     2438static PseudoElement* beforeOrAfterPseudoElement(Element& host, PseudoId pseudoElementSpecifier)
    24382439{
    24392440    switch (pseudoElementSpecifier) {
    24402441    case BEFORE:
    2441         return host->beforePseudoElement();
     2442        return host.beforePseudoElement();
    24422443    case AFTER:
    2443         return host->afterPseudoElement();
     2444        return host.afterPseudoElement();
    24442445    default:
    2445         return 0;
    2446     }
     2446        return nullptr;
     2447    }
     2448}
     2449
     2450RenderStyle* Element::existingComputedStyle()
     2451{
     2452    if (auto* renderTreeStyle = renderStyle())
     2453        return renderTreeStyle;
     2454
     2455    if (hasRareData())
     2456        return elementRareData()->computedStyle();
     2457
     2458    return nullptr;
     2459}
     2460
     2461RenderStyle& Element::resolveComputedStyle()
     2462{
     2463    ASSERT(inDocument());
     2464    ASSERT(!existingComputedStyle());
     2465
     2466    Deque<Element*, 32> elementsRequiringComputedStyle({ this });
     2467    RenderStyle* computedStyle = nullptr;
     2468
     2469    // Collect ancestors until we find one that has style.
     2470    auto composedAncestors = composedTreeAncestors(*this);
     2471    for (auto& ancestor : composedAncestors) {
     2472        if (!is<Element>(ancestor))
     2473            break;
     2474        auto& ancestorElement = downcast<Element>(ancestor);
     2475        elementsRequiringComputedStyle.prepend(&ancestorElement);
     2476        if (auto* existingStyle = ancestorElement.existingComputedStyle()) {
     2477            computedStyle = existingStyle;
     2478            break;
     2479        }
     2480    }
     2481
     2482    // Resolve and cache styles starting from the most distant ancestor.
     2483    for (auto* element : elementsRequiringComputedStyle) {
     2484        auto style = document().styleForElementIgnoringPendingStylesheets(*element, computedStyle);
     2485        computedStyle = style.ptr();
     2486        ElementRareData& rareData = element->ensureElementRareData();
     2487        rareData.setComputedStyle(WTF::move(style));
     2488    }
     2489
     2490    return *computedStyle;
    24472491}
    24482492
    24492493RenderStyle* Element::computedStyle(PseudoId pseudoElementSpecifier)
    24502494{
    2451     if (PseudoElement* pseudoElement = beforeOrAfterPseudoElement(this, pseudoElementSpecifier))
     2495    if (PseudoElement* pseudoElement = beforeOrAfterPseudoElement(*this, pseudoElementSpecifier))
    24522496        return pseudoElement->computedStyle();
    24532497
    2454     // FIXME: Find and use the renderer from the pseudo element instead of the actual element so that the 'length'
    2455     // properties, which are only known by the renderer because it did the layout, will be correct and so that the
    2456     // values returned for the ":selection" pseudo-element will be correct.
    2457     if (RenderStyle* usedStyle = renderStyle()) {
    2458         if (pseudoElementSpecifier) {
    2459             RenderStyle* cachedPseudoStyle = usedStyle->getCachedPseudoStyle(pseudoElementSpecifier);
    2460             return cachedPseudoStyle ? cachedPseudoStyle : usedStyle;
    2461         }
    2462         return usedStyle;
    2463     }
    2464 
    2465     if (!inDocument()) {
    2466         // FIXME: Try to do better than this. Ensure that styleForElement() works for elements that are not in the
    2467         // document tree and figure out when to destroy the computed style for such elements.
    2468         return nullptr;
    2469     }
    2470 
    2471     ElementRareData& data = ensureElementRareData();
    2472     if (!data.computedStyle())
    2473         data.setComputedStyle(document().styleForElementIgnoringPendingStylesheets(this));
    2474     return pseudoElementSpecifier ? data.computedStyle()->getCachedPseudoStyle(pseudoElementSpecifier) : data.computedStyle();
     2498    auto* style = existingComputedStyle();
     2499    if (!style) {
     2500        if (!inDocument())
     2501            return nullptr;
     2502        style = &resolveComputedStyle();
     2503    }
     2504
     2505    if (pseudoElementSpecifier) {
     2506        if (auto* cachedPseudoStyle = style->getCachedPseudoStyle(pseudoElementSpecifier))
     2507            return cachedPseudoStyle;
     2508    }
     2509
     2510    return style;
    24752511}
    24762512

  • trunk/Source/WebCore/dom/Element.h

    r191229 r191262  
    574574    void removeShadowRoot();
    575575
     576    RenderStyle* existingComputedStyle();
     577    RenderStyle& resolveComputedStyle();
     578
    576579    bool rareDataStyleAffectedByEmpty() const;
    577580    bool rareDataChildrenAffectedByHover() const;

  • trunk/Source/WebCore/dom/Node.cpp

    r191229 r191262  
    10071007RenderStyle* Node::computedStyle(PseudoId pseudoElementSpecifier)
    10081008{
    1009     for (Node* node = this; node; node = node->parentOrShadowHostNode()) {
    1010         if (is<Element>(*node))
    1011             return downcast<Element>(*node).computedStyle(pseudoElementSpecifier);
    1012     }
    1013     return nullptr;
     1009    auto* composedParent = composedTreeAncestors(*this).first();
     1010    if (!composedParent)
     1011        return nullptr;
     1012    return composedParent->computedStyle(pseudoElementSpecifier);
    10141013}
    10151014

  • trunk/Source/WebCore/html/HTMLSelectElement.cpp

    r190613 r191262  
    888888    }
    889889
     890    updateValidity();
     891
    890892    // For the menu list case, this is what makes the selected element appear.
    891893    if (auto* renderer = this->renderer())
    … …  
    894896    scrollToSelection();
    895897
    896     updateValidity();
    897898    if (usesMenuList()) {
    898899        m_isProcessingUserDrivenChange = flags & UserDriven;
Note: See TracChangeset for help on using the changeset viewer.