Changeset 271635 in webkit


Ignore:
Timestamp:
Jan 19, 2021 6:00:03 PM (18 months ago)
Author:
Megan Gardner
Message:

Elements in a table are incorrectly selected in JavaScript.
​https://bugs.webkit.org/show_bug.cgi?id=220607

Reviewed by Wenson Hsieh.

Source/WebCore:

After the fix for ​https://bugs.webkit.org/show_bug.cgi?id=130844 in ​https://trac.webkit.org/changeset/260207/webkit a regression was found for
table cell selections through JavaScript. The previous fix was for an issue in the difference between how PositionIterator and Position calculated
if it was canonical (::isCandidate) in the cases where the renderer was a BR. Instead of only converging isCandidate for BR, this function in PositionIterator
was completely subsumed by just calling the implementation for Position. While there are many similarities, there are important differences, especially in the
table code, therefore PositionIterator::isCandidate is being reinstated, but with the convergence in the BR case only.
Also, moved the check added in ​http://webkit.org/b/211864 so it will cover cases that were missed due to this code being wrong while that fix was added.

Test: editing/selection/editable-table-cell-selection.html

  • dom/PositionIterator.cpp:

(WebCore::PositionIterator::isCandidate const):

  • editing/InsertParagraphSeparatorCommand.cpp:

(WebCore::InsertParagraphSeparatorCommand::doApply):

LayoutTests:

  • editing/execCommand/null_calc_primitive_value_for_css_property.html:

Added ending tag that was missing and does not affect the issue the test was verifying.

  • editing/inserting/insert-list-in-table-cell-07-expected.txt:

Restored expected text file to original output.

  • editing/selection/editable-table-cell-selection-expected.txt: Added.
  • editing/selection/editable-table-cell-selection.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r271618 r271635  
     12021-01-19  Megan Gardner  <megan_gardner@apple.com>
     2
     3        Elements in a table are incorrectly selected in JavaScript.
     4        https://bugs.webkit.org/show_bug.cgi?id=220607
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        * editing/execCommand/null_calc_primitive_value_for_css_property.html:
     9        Added ending tag that was missing and does not affect the issue the test was verifying.
     10        * editing/inserting/insert-list-in-table-cell-07-expected.txt:
     11        Restored expected text file to original output.
     12        * editing/selection/editable-table-cell-selection-expected.txt: Added.
     13        * editing/selection/editable-table-cell-selection.html: Added.
     14
    1152021-01-19  Sergio Villar Senin  <svillar@igalia.com>
    216

  • trunk/LayoutTests/editing/execCommand/null_calc_primitive_value_for_css_property.html

    r261830 r271635  
    1717
    1818<body onload=cssPrimitiveValTest()>
    19 <ins id="x">
     19<ins id="x"></ins>
    2020
    2121</body>

  • trunk/LayoutTests/editing/inserting/insert-list-in-table-cell-07-expected.txt

    r267362 r271635  
    22
    33Before:
    4 | <#selection-focus>
    54| <table>
    65|   border="1"
    … …  
    1817|         "fsfg"
    1918|   <tbody>
     19| <#selection-focus>
    2020
    2121After:
    … …  
    2626|     <tr>
    2727|       <td>
    28 |         "<#selection-caret>fsdf"
     28|         "<#selection-anchor>fsdf"
    2929|         <br>
    3030|       <td>
    3131|         "fsdf"
     32|         <br>
    3233|     <tr>
    3334|       <td>
    3435|         "gghfg"
     36|         <br>
    3537|       <td>
    36 |         "fsfg"
     38|         "fsfg<#selection-focus>"
     39|         <br>
    3740|   <tbody>

  • trunk/Source/WebCore/ChangeLog

    r271620 r271635  
     12021-01-19  Megan Gardner  <megan_gardner@apple.com>
     2
     3        Elements in a table are incorrectly selected in JavaScript.
     4        https://bugs.webkit.org/show_bug.cgi?id=220607
     5
     6        Reviewed by Wenson Hsieh.
     7
     8        After the fix for https://bugs.webkit.org/show_bug.cgi?id=130844 in https://trac.webkit.org/changeset/260207/webkit a regression was found for
     9        table cell selections through JavaScript. The previous fix was for an issue in the difference between how PositionIterator and Position calculated
     10        if it was canonical (::isCandidate) in the cases where the renderer was a BR. Instead of only converging isCandidate for BR, this function in PositionIterator
     11        was completely subsumed by just calling the implementation for Position. While there are many similarities, there are important differences, especially in the
     12        table code, therefore PositionIterator::isCandidate is being reinstated, but with the convergence in the BR case only.
     13        Also, moved the check added in http://webkit.org/b/211864 so it will cover cases that were missed due to this code being wrong while that fix was added.
     14
     15        Test: editing/selection/editable-table-cell-selection.html
     16
     17        * dom/PositionIterator.cpp:
     18        (WebCore::PositionIterator::isCandidate const):
     19        * editing/InsertParagraphSeparatorCommand.cpp:
     20        (WebCore::InsertParagraphSeparatorCommand::doApply):
     21       
     22
    1232021-01-19  Patrick Angle  <pangle@apple.com>
    224

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

    r267363 r271635  
    146146bool PositionIterator::isCandidate() const
    147147{
    148     return m_anchorNode ? Position(*this).isCandidate() : false;
     148    if (!m_anchorNode)
     149        return false;
     150
     151    RenderObject* renderer = m_anchorNode->renderer();
     152    if (!renderer)
     153        return false;
     154
     155    if (renderer->style().visibility() != Visibility::Visible)
     156        return false;
     157
     158    if (renderer->isBR())
     159        return Position(*this).isCandidate();
     160
     161    if (is<RenderText>(*renderer))
     162        return !Position::nodeIsUserSelectNone(m_anchorNode) && downcast<RenderText>(*renderer).containsCaretOffset(m_offsetInAnchor);
     163
     164    if (isRenderedTable(m_anchorNode) || editingIgnoresContent(*m_anchorNode))
     165        return (atStartOfNode() || atEndOfNode()) && !Position::nodeIsUserSelectNone(m_anchorNode->parentNode());
     166
     167    if (!is<HTMLHtmlElement>(*m_anchorNode) && is<RenderBlockFlow>(*renderer)) {
     168        RenderBlockFlow& block = downcast<RenderBlockFlow>(*renderer);
     169        if (block.logicalHeight() || is<HTMLBodyElement>(*m_anchorNode)) {
     170            if (!Position::hasRenderedNonAnonymousDescendantsWithHeight(block))
     171                return atStartOfNode() && !Position::nodeIsUserSelectNone(m_anchorNode);
     172            return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
     173        }
     174    }
     175
     176    return false;
    149177}
    150178

  • trunk/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp

    r266557 r271635  
    201201    bool nestNewBlock = false;
    202202
     203    // FIXME: <http://webkit.org/b/211864> If insertionPosition is not editable, we should compute a position that is.
     204    if (!isEditablePosition(insertionPosition))
     205        return;
     206
    203207    // Create block to be inserted.
    204208    RefPtr<Element> blockToInsert;
    … …  
    299303    // content will move down a line.
    300304    if (isStartOfParagraph(visiblePos)) {
    301         // FIXME: <http://webkit.org/b/211864> If insertionPosition is not editable, we should compute a position that is.
    302         if (!isEditablePosition(insertionPosition))
    303             return;
    304 
    305305        auto br = HTMLBRElement::create(document());
    306306        auto* brPtr = br.ptr();
Note: See TracChangeset for help on using the changeset viewer.