Changeset 262049 in webkit


Ignore:
Timestamp:
May 22, 2020 1:34:29 AM (4 years ago)
Author:
commit-queue@webkit.org
Message:

Cannot style ::selection for a flex container
https://bugs.webkit.org/show_bug.cgi?id=209822

Patch by Tyler Wilcock <Tyler Wilcock> on 2020-05-22
Reviewed by Antti Koivisto.

Source/WebCore:

When needing to query for pseudostyles, RenderText used to unconditionally check the parent's pseudostyles. The parent of
RenderText objects is often an anonymous box, depending on the presence of siblings, display type, etc. This is problematic
as pseudostyles are associated with an element of the DOM, meaning RenderText elements would often fail to find any pseudostyle
thanks to their anonymous parent.

This patch changes RenderText to traverse its tree of ancestry upwards until it finds a non-anonymous ancestor and gets those pseudostyles,
rather than unconditionally trying to get pseudostyles from its direct parent.

Blink does something similar when retrieving pseudostyles:

https://github.com/chromium/chromium/blob/793cb59c18334f8b506863192bf630776da0f4d2/third_party/blink/renderer/core/paint/selection_painting_utils.cc#L54

Tests: editing/selection/selection-display-block-sibling.html

editing/selection/selection-display-flex.html

  • rendering/RenderObject.cpp:

(WebCore::RenderObject::firstNonAnonymousAncestor const):

  • rendering/RenderObject.h:
  • rendering/RenderText.h:

(WebCore::RenderText::getCachedPseudoStyle const): getCachedPseudoStyle from first non-anonymous ancestor, rather than only checking the direct parent.
(WebCore::RenderText::selectionBackgroundColor const): Retrieve selectionBackgroundColor from first non-anonymous ancestor rather than only checking the direct parent.
(WebCore::RenderText::selectionForegroundColor const): Retrieve selectionForegroundColor from first non-anonymous ancestor rather than only checking the direct parent.
(WebCore::RenderText::selectionEmphasisMarkColor const): Retrieve selectionEmphasisMarkColor from first non-anonymous ancestor rather than only checking the direct parent.
(WebCore::RenderText::selectionPseudoStyle const): Retrieve selectionPseudoStyle from first non-anonymous ancestor rather than only checking the direct parent.

LayoutTests:

Add tests verifying ::selection pseudoelement styling is properly applied on direct text-children of a display: flex; div and on
direct text-children of a display: block div with siblings.

  • editing/selection/selection-display-block-sibling.html: Added.
  • editing/selection/selection-display-flex.html: Added.
  • platform/gtk/editing/selection/selection-display-block-sibling-expected.png: Added.
  • platform/gtk/editing/selection/selection-display-block-sibling-expected.txt: Added.
  • platform/gtk/editing/selection/selection-display-flex-expected.png: Added.
  • platform/gtk/editing/selection/selection-display-flex-expected.txt: Added.
Location:
trunk
Files:
6 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r262048 r262049  
     12020-05-22  Tyler Wilcock  <twilco.o@protonmail.com>
     2
     3        Cannot style ::selection for a flex container
     4        https://bugs.webkit.org/show_bug.cgi?id=209822
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Add tests verifying ::selection pseudoelement styling is properly applied on direct text-children of a `display: flex;` div and on
     9        direct text-children of a `display: block` div with siblings.
     10
     11        * editing/selection/selection-display-block-sibling.html: Added.
     12        * editing/selection/selection-display-flex.html: Added.
     13        * platform/gtk/editing/selection/selection-display-block-sibling-expected.png: Added.
     14        * platform/gtk/editing/selection/selection-display-block-sibling-expected.txt: Added.
     15        * platform/gtk/editing/selection/selection-display-flex-expected.png: Added.
     16        * platform/gtk/editing/selection/selection-display-flex-expected.txt: Added.
     17
    1182020-05-21  Diego Pino Garcia  <dpino@igalia.com>
    219
  • trunk/Source/WebCore/ChangeLog

    r262047 r262049  
     12020-05-22  Tyler Wilcock  <twilco.o@protonmail.com>
     2
     3        Cannot style ::selection for a flex container
     4        https://bugs.webkit.org/show_bug.cgi?id=209822
     5
     6        Reviewed by Antti Koivisto.
     7
     8        When needing to query for pseudostyles, RenderText used to unconditionally check the parent's pseudostyles.  The parent of
     9        RenderText objects is often an anonymous box, depending on the presence of siblings, `display` type, etc.  This is problematic
     10        as pseudostyles are associated with an element of the DOM, meaning RenderText elements would often fail to find any pseudostyle
     11        thanks to their anonymous parent.
     12
     13        This patch changes RenderText to traverse its tree of ancestry upwards until it finds a non-anonymous ancestor and gets those pseudostyles,
     14        rather than unconditionally trying to get pseudostyles from its direct parent.
     15
     16        Blink does something similar when retrieving pseudostyles:
     17
     18        https://github.com/chromium/chromium/blob/793cb59c18334f8b506863192bf630776da0f4d2/third_party/blink/renderer/core/paint/selection_painting_utils.cc#L54
     19
     20        Tests: editing/selection/selection-display-block-sibling.html
     21               editing/selection/selection-display-flex.html
     22
     23        * rendering/RenderObject.cpp:
     24        (WebCore::RenderObject::firstNonAnonymousAncestor const):
     25        * rendering/RenderObject.h:
     26        * rendering/RenderText.h:
     27        (WebCore::RenderText::getCachedPseudoStyle const): getCachedPseudoStyle from first non-anonymous ancestor, rather than only checking the direct parent.
     28        (WebCore::RenderText::selectionBackgroundColor const): Retrieve selectionBackgroundColor from first non-anonymous ancestor rather than only checking the direct parent.
     29        (WebCore::RenderText::selectionForegroundColor const): Retrieve selectionForegroundColor from first non-anonymous ancestor rather than only checking the direct parent.
     30        (WebCore::RenderText::selectionEmphasisMarkColor const): Retrieve selectionEmphasisMarkColor from first non-anonymous ancestor rather than only checking the direct parent.
     31        (WebCore::RenderText::selectionPseudoStyle const): Retrieve selectionPseudoStyle from first non-anonymous ancestor rather than only checking the direct parent.
     32
    1332020-05-21  Wenson Hsieh  <wenson_hsieh@apple.com>
    234
  • trunk/Source/WebCore/rendering/RenderObject.cpp

    r261941 r262049  
    166166}
    167167
     168RenderElement* RenderObject::firstNonAnonymousAncestor() const
     169{
     170    auto* ancestor = parent();
     171    while (ancestor && ancestor->isAnonymous())
     172        ancestor = ancestor->parent();
     173    return ancestor;
     174}
     175
    168176bool RenderObject::isLegend() const
    169177{
  • trunk/Source/WebCore/rendering/RenderObject.h

    r261979 r262049  
    730730    void addPDFURLRect(PaintInfo&, const LayoutPoint&);
    731731    Node& nodeForNonAnonymous() const { ASSERT(!isAnonymous()); return m_node; }
     732
     733    RenderElement* firstNonAnonymousAncestor() const;
    732734
    733735    void adjustRectForOutlineAndShadow(LayoutRect&) const;
  • trunk/Source/WebCore/rendering/RenderText.h

    r259611 r262049  
    284284inline const RenderStyle* RenderText::getCachedPseudoStyle(PseudoId pseudoId, const RenderStyle* parentStyle) const
    285285{
    286     return parent()->getCachedPseudoStyle(pseudoId, parentStyle);
     286    // Pseudostyle is associated with an element, so ascend the tree until we find a non-anonymous ancestor.
     287    if (auto* ancestor = firstNonAnonymousAncestor())
     288        return ancestor->getCachedPseudoStyle(pseudoId, parentStyle);
     289    return nullptr;
    287290}
    288291
    289292inline Color RenderText::selectionBackgroundColor() const
    290293{
    291     return parent()->selectionBackgroundColor();
     294    if (auto* ancestor = firstNonAnonymousAncestor())
     295        return ancestor->selectionBackgroundColor();
     296    return Color();
    292297}
    293298
    294299inline Color RenderText::selectionForegroundColor() const
    295300{
    296     return parent()->selectionForegroundColor();
     301    if (auto* ancestor = firstNonAnonymousAncestor())
     302        return ancestor->selectionForegroundColor();
     303    return Color();
    297304}
    298305
    299306inline Color RenderText::selectionEmphasisMarkColor() const
    300307{
    301     return parent()->selectionEmphasisMarkColor();
     308    if (auto* ancestor = firstNonAnonymousAncestor())
     309        return ancestor->selectionEmphasisMarkColor();
     310    return Color();
    302311}
    303312
    304313inline std::unique_ptr<RenderStyle> RenderText::selectionPseudoStyle() const
    305314{
    306     return parent()->selectionPseudoStyle();
     315    if (auto* ancestor = firstNonAnonymousAncestor())
     316        return ancestor->selectionPseudoStyle();
     317    return nullptr;
    307318}
    308319
Note: See TracChangeset for help on using the changeset viewer.