- Revision
- 272249
- Author
- [email protected]
- Date
- 2021-02-02 17:39:22 -0800 (Tue, 02 Feb 2021)
Log Message
Cherry-pick r271635. rdar://problem/73890109
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.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271635 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-611-branch/LayoutTests/ChangeLog (272248 => 272249)
--- branches/safari-611-branch/LayoutTests/ChangeLog 2021-02-03 01:39:17 UTC (rev 272248)
+++ branches/safari-611-branch/LayoutTests/ChangeLog 2021-02-03 01:39:22 UTC (rev 272249)
@@ -1,3 +1,54 @@
+2021-02-02 Alan Coon <[email protected]>
+
+ Cherry-pick r271635. rdar://problem/73890109
+
+ 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.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271635 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-01-19 Megan Gardner <[email protected]>
+
+ Elements in a table are incorrectly selected in _javascript_.
+ https://bugs.webkit.org/show_bug.cgi?id=220607
+
+ Reviewed by Wenson Hsieh.
+
+ * 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.
+
2021-02-01 Alan Coon <[email protected]>
Cherry-pick r271933. rdar://problem/73838863
Modified: branches/safari-611-branch/LayoutTests/editing/execCommand/null_calc_primitive_value_for_css_property.html (272248 => 272249)
--- branches/safari-611-branch/LayoutTests/editing/execCommand/null_calc_primitive_value_for_css_property.html 2021-02-03 01:39:17 UTC (rev 272248)
+++ branches/safari-611-branch/LayoutTests/editing/execCommand/null_calc_primitive_value_for_css_property.html 2021-02-03 01:39:22 UTC (rev 272249)
@@ -16,7 +16,7 @@
</script>
<body _onload_=cssPrimitiveValTest()>
-<ins id="x">
+<ins id="x"></ins>
</body>
</html>
Modified: branches/safari-611-branch/LayoutTests/editing/inserting/insert-list-in-table-cell-07-expected.txt (272248 => 272249)
--- branches/safari-611-branch/LayoutTests/editing/inserting/insert-list-in-table-cell-07-expected.txt 2021-02-03 01:39:17 UTC (rev 272248)
+++ branches/safari-611-branch/LayoutTests/editing/inserting/insert-list-in-table-cell-07-expected.txt 2021-02-03 01:39:22 UTC (rev 272249)
@@ -1,7 +1,6 @@
Exec insertOrderedList twice in all the cells of a table removes the previously inserted list items:
Before:
-| <#selection-focus>
| <table>
| border="1"
| <tbody>
@@ -17,6 +16,7 @@
| <td>
| "fsfg"
| <tbody>
+| <#selection-focus>
After:
| <table>
@@ -25,13 +25,16 @@
| id="element"
| <tr>
| <td>
-| "<#selection-caret>fsdf"
+| "<#selection-anchor>fsdf"
| <br>
| <td>
| "fsdf"
+| <br>
| <tr>
| <td>
| "gghfg"
+| <br>
| <td>
-| "fsfg"
+| "fsfg<#selection-focus>"
+| <br>
| <tbody>
Added: branches/safari-611-branch/LayoutTests/editing/selection/editable-table-cell-selection-expected.txt (0 => 272249)
--- branches/safari-611-branch/LayoutTests/editing/selection/editable-table-cell-selection-expected.txt (rev 0)
+++ branches/safari-611-branch/LayoutTests/editing/selection/editable-table-cell-selection-expected.txt 2021-02-03 01:39:22 UTC (rev 272249)
@@ -0,0 +1,11 @@
+A B C D
+1A 1B 1C 1D
+2A 2B 2C 2D
+3A 3B 3C 3D
+4A 4B 4C 4D
+5A 5B 5C 5D
+PASS: Correctly Selects row in thead
+PASS: Correctly Selects last cell in thead
+PASS: Correctly Selects row in tbody
+PASS: Correctly Selects last cell in tbody
+
Added: branches/safari-611-branch/LayoutTests/editing/selection/editable-table-cell-selection.html (0 => 272249)
--- branches/safari-611-branch/LayoutTests/editing/selection/editable-table-cell-selection.html (rev 0)
+++ branches/safari-611-branch/LayoutTests/editing/selection/editable-table-cell-selection.html 2021-02-03 01:39:22 UTC (rev 272249)
@@ -0,0 +1,122 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ async function runTest()
+ {
+ var output = '';
+
+ function select(element) {
+ const range = document.createRange();
+
+ range.setStartBefore(element);
+ range.setEndAfter(element);
+
+ const selection = window.getSelection();
+
+ selection.removeAllRanges();
+ selection.addRange(range);
+ }
+
+ select(document.querySelector('thead tr'));
+ if (document.getSelection().toString() == "A\tB\tC\tD\n")
+ output += 'PASS: Correctly Selects row in thead';
+ else
+ output += 'FAIL: Incorrect Selection: ' + document.getSelection().toString();
+ output += '<br>';
+
+ const lastHeadCell = [ ...document.querySelectorAll('thead th') ].pop();
+ select(lastHeadCell);
+ if (document.getSelection().toString() == "D\n")
+ output += 'PASS: Correctly Selects last cell in thead';
+ else
+ output += 'FAIL: Incorrect Selection: ' + document.getSelection().toString();
+ output += '<br>';
+
+ select(document.querySelector('tbody tr'));
+ if (document.getSelection().toString() == "1A\t1B\t1C\t1D\n")
+ output += 'PASS: Correctly Selects row in tbody';
+ else
+ output += 'FAIL: Incorrect Selection: ' + document.getSelection().toString();
+ output += '<br>';
+
+ const lastBodyCell = [ ...document.querySelectorAll('tbody td') ].pop();
+ select(lastBodyCell);
+ if (document.getSelection().toString() == "5D\n")
+ output += 'PASS: Correctly Selects last cell in tbody';
+ else
+ output += 'FAIL: Incorrect Selection: ' + document.getSelection().toString();
+ output += '<br>';
+
+ document.getElementById('target').innerHTML = output;
+ testRunner.notifyDone();
+ }
+
+ window.addEventListener('load', runTest, false);
+ </script>
+ <style>
+ #target {
+ height: 50px;
+ width: 300px;
+ background-color: silver;
+ font-family: monospace;
+ font-size: 18px;
+ }
+ </style>
+ <meta name="viewport" content="width=device-width, initial-scale=1">
+</head>
+<body>
+ <div contenteditable>
+ <table border="1">
+ <thead>
+ <tr>
+ <th scope="col">A</th>
+ <th scope="col">B</th>
+ <th scope="col">C</th>
+ <th scope="col">D</th>
+ </tr>
+ </thead>
+ <tbody>
+ <tr>
+ <th scope="row">1A</th>
+ <td>1B</td>
+ <td>1C</td>
+ <td>1D</td>
+ </tr>
+ <tr>
+ <th scope="row">2A</th>
+ <td>2B</td>
+ <td>2C</td>
+ <td>2D</td>
+ </tr>
+ <tr>
+ <th scope="row">3A</th>
+ <td>3B</td>
+ <td>3C</td>
+ <td>3D</td>
+ </tr>
+ <tr>
+ <th scope="row">4A</th>
+ <td>4B</td>
+ <td>4C</td>
+ <td>4D</td>
+ </tr>
+ <tr>
+ <th scope="row">5A</th>
+ <td>5B</td>
+ <td>5C</td>
+ <td>5D</td>
+ </tr>
+ </tbody>
+ </table>
+</div>
+ <div id="target">
+ </div>
+</body>
+</html>
Modified: branches/safari-611-branch/Source/WebCore/ChangeLog (272248 => 272249)
--- branches/safari-611-branch/Source/WebCore/ChangeLog 2021-02-03 01:39:17 UTC (rev 272248)
+++ branches/safari-611-branch/Source/WebCore/ChangeLog 2021-02-03 01:39:22 UTC (rev 272249)
@@ -1,5 +1,64 @@
2021-02-02 Alan Coon <[email protected]>
+ Cherry-pick r271635. rdar://problem/73890109
+
+ 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.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271635 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-01-19 Megan Gardner <[email protected]>
+
+ Elements in a table are incorrectly selected in _javascript_.
+ https://bugs.webkit.org/show_bug.cgi?id=220607
+
+ Reviewed by Wenson Hsieh.
+
+ 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):
+
+
+2021-02-02 Alan Coon <[email protected]>
+
Cherry-pick r271521. rdar://problem/73890570
Mark only child for layout
Modified: branches/safari-611-branch/Source/WebCore/dom/PositionIterator.cpp (272248 => 272249)
--- branches/safari-611-branch/Source/WebCore/dom/PositionIterator.cpp 2021-02-03 01:39:17 UTC (rev 272248)
+++ branches/safari-611-branch/Source/WebCore/dom/PositionIterator.cpp 2021-02-03 01:39:22 UTC (rev 272249)
@@ -145,7 +145,35 @@
bool PositionIterator::isCandidate() const
{
- return m_anchorNode ? Position(*this).isCandidate() : false;
+ if (!m_anchorNode)
+ return false;
+
+ RenderObject* renderer = m_anchorNode->renderer();
+ if (!renderer)
+ return false;
+
+ if (renderer->style().visibility() != Visibility::Visible)
+ return false;
+
+ if (renderer->isBR())
+ return Position(*this).isCandidate();
+
+ if (is<RenderText>(*renderer))
+ return !Position::nodeIsUserSelectNone(m_anchorNode) && downcast<RenderText>(*renderer).containsCaretOffset(m_offsetInAnchor);
+
+ if (isRenderedTable(m_anchorNode) || editingIgnoresContent(*m_anchorNode))
+ return (atStartOfNode() || atEndOfNode()) && !Position::nodeIsUserSelectNone(m_anchorNode->parentNode());
+
+ if (!is<HTMLHtmlElement>(*m_anchorNode) && is<RenderBlockFlow>(*renderer)) {
+ RenderBlockFlow& block = downcast<RenderBlockFlow>(*renderer);
+ if (block.logicalHeight() || is<HTMLBodyElement>(*m_anchorNode)) {
+ if (!Position::hasRenderedNonAnonymousDescendantsWithHeight(block))
+ return atStartOfNode() && !Position::nodeIsUserSelectNone(m_anchorNode);
+ return m_anchorNode->hasEditableStyle() && !Position::nodeIsUserSelectNone(m_anchorNode) && Position(*this).atEditingBoundary();
+ }
+ }
+
+ return false;
}
} // namespace WebCore
Modified: branches/safari-611-branch/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp (272248 => 272249)
--- branches/safari-611-branch/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp 2021-02-03 01:39:17 UTC (rev 272248)
+++ branches/safari-611-branch/Source/WebCore/editing/InsertParagraphSeparatorCommand.cpp 2021-02-03 01:39:22 UTC (rev 272249)
@@ -200,6 +200,10 @@
bool isLastInBlock = isEndOfBlock(visiblePos);
bool nestNewBlock = false;
+ // FIXME: <http://webkit.org/b/211864> If insertionPosition is not editable, we should compute a position that is.
+ if (!isEditablePosition(insertionPosition))
+ return;
+
// Create block to be inserted.
RefPtr<Element> blockToInsert;
if (startBlock->isRootEditableElement()) {
@@ -298,10 +302,6 @@
// it if visiblePos is at the start of a paragraph so that the
// content will move down a line.
if (isStartOfParagraph(visiblePos)) {
- // FIXME: <http://webkit.org/b/211864> If insertionPosition is not editable, we should compute a position that is.
- if (!isEditablePosition(insertionPosition))
- return;
-
auto br = HTMLBRElement::create(document());
auto* brPtr = br.ptr();
insertNodeAt(WTFMove(br), insertionPosition);
