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

Changeset 280585 in webkit


Ignore:
Timestamp:
Aug 2, 2021, 11:54:03 PM (5 years ago)
Author:
commit-queue@webkit.org
Message:

Align implementation of PositionIterator::isCandidate() on Position::isCandidate()
​https://bugs.webkit.org/show_bug.cgi?id=228635

Patch by Frédéric Wang <​fwang@igalia.com> on 2021-08-02
Reviewed by Darin Adler.

The bug fixed in r280381 was due to the fact that PositionIterator::isCandidate() and
Position::isCandidate() had gone out of sync. To prevent future bugs of this kind, this patch
modifies PositionIterator::isCandidate() so that it is aligned with
PositionIterator::isCandidate() (except when an m_anchorType check is needed) and add code
comments in both functions to ensure the same changes are always applied to them.

  • dom/Position.cpp:

(WebCore::Position::isCandidate const): Add a comment to make sure we update PositionIterator
when changing that function. Also use auto for a local variable like in PositionIterator.

  • dom/PositionIterator.cpp:

(WebCore::PositionIterator::isCandidate const): Add a comment to make sure we update Position
when changing that function. Rearrange the code to use positionBeforeOrAfterNodeIsCandidate
and early return when the node is a <html> element (these are not behavior changes). For
block flow / grid / flexbox renderers, add a special handling when the anchor node is a root
editable element ; also change the fallback value returned at the end of the function (these
are two behavior changes).

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r280584 r280585  
     12021-08-02  Frédéric Wang  <fwang@igalia.com>
     2
     3        Align implementation of PositionIterator::isCandidate() on Position::isCandidate()
     4        https://bugs.webkit.org/show_bug.cgi?id=228635
     5
     6        Reviewed by Darin Adler.
     7
     8        The bug fixed in r280381 was due to the fact that PositionIterator::isCandidate() and
     9        Position::isCandidate() had gone out of sync. To prevent future bugs of this kind, this patch
     10        modifies PositionIterator::isCandidate() so that it is aligned with
     11        PositionIterator::isCandidate() (except when an m_anchorType check is needed) and add code
     12        comments in both functions to ensure the same changes are always applied to them.
     13
     14        * dom/Position.cpp:
     15        (WebCore::Position::isCandidate const): Add a comment to make sure we update PositionIterator
     16        when changing that function. Also use auto for a local variable like in PositionIterator.
     17        * dom/PositionIterator.cpp:
     18        (WebCore::PositionIterator::isCandidate const): Add a comment to make sure we update Position
     19        when changing that function. Rearrange the code to use positionBeforeOrAfterNodeIsCandidate
     20        and early return when the node is a <html> element (these are not behavior changes). For
     21        block flow / grid / flexbox renderers, add a special handling when the anchor node is a root
     22        editable element ; also change the fallback value returned at the end of the function (these
     23        are two behavior changes).
     24
    1252021-08-02  Jean-Yves Avenard  <jya@apple.com>
    226

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

    r280174 r280585  
    968968}
    969969
     970// This function should be kept in sync with PositionIterator::isCandidate().
    970971bool Position::isCandidate() const
    971972{
    … …  
    998999
    9991000    if (is<RenderBlockFlow>(*renderer) || is<RenderGrid>(*renderer) || is<RenderFlexibleBox>(*renderer)) {
    1000         RenderBlock& block = downcast<RenderBlock>(*renderer);
     1001        auto& block = downcast<RenderBlock>(*renderer);
    10011002        if (block.logicalHeight() || is<HTMLBodyElement>(*m_anchorNode) || m_anchorNode->isRootEditableElement()) {
    10021003            if (!Position::hasRenderedNonAnonymousDescendantsWithHeight(block))

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

    r280381 r280585  
    146146}
    147147
     148// This function should be kept in sync with Position::isCandidate().
    148149bool PositionIterator::isCandidate() const
    149150{
    … …  
    164165        return !Position::nodeIsUserSelectNone(m_anchorNode) && downcast<RenderText>(*renderer).containsCaretOffset(m_offsetInAnchor);
    165166
    166     if (isRenderedTable(m_anchorNode) || editingIgnoresContent(*m_anchorNode))
     167    if (positionBeforeOrAfterNodeIsCandidate(*m_anchorNode))
    167168        return (atStartOfNode() || atEndOfNode()) && !Position::nodeIsUserSelectNone(m_anchorNode->parentNode());
    168169
    169     if (!is<HTMLHtmlElement>(*m_anchorNode) && (is<RenderBlockFlow>(*renderer) || is<RenderGrid>(*renderer) || is<RenderFlexibleBox>(*renderer))) {
     170    if (is<HTMLHtmlElement>(*m_anchorNode))
     171        return false;
     172
     173    if (is<RenderBlockFlow>(*renderer) || is<RenderGrid>(*renderer) || is<RenderFlexibleBox>(*renderer)) {
    170174        auto& block = downcast<RenderBlock>(*renderer);
    171         if (block.logicalHeight() || is<HTMLBodyElement>(*m_anchorNode)) {
     175        if (block.logicalHeight() || is<HTMLBodyElement>(*m_anchorNode) || m_anchorNode->isRootEditableElement()) {
    172176            if (!Position::hasRenderedNonAnonymousDescendantsWithHeight(block))
    173177                return atStartOfNode() && !Position::nodeIsUserSelectNone(m_anchorNode);
    174178            return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
    175179        }
     180        return false;
    176181    }
    177182
    178     return false;
     183    return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
    179184}
    180185
Note: See TracChangeset for help on using the changeset viewer.