Title: [292381] trunk
Revision
292381
Author
megan_gard...@apple.com
Date
2022-04-05 01:01:50 -0700 (Tue, 05 Apr 2022)

Log Message

Moving forwards and backwards by sentences across a table results in different breaks in each direction.
https://bugs.webkit.org/show_bug.cgi?id=238621

Reviewed by Darin Adler.

Tests: editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity.html
       editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity.html

We need to emit tabs even in a simplified backwards text iterator because otherwise traversing a table by
sentence granularity results in not having the same behavior backwards and forwards.

* editing/TextIterator.cpp:
(WebCore::SimplifiedBackwardsTextIterator::handleNonTextNode):
(WebCore::SimplifiedBackwardsTextIterator::exitNode):

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity-expected.txt (0 => 292381)


--- trunk/LayoutTests/editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity-expected.txt	2022-04-05 08:01:50 UTC (rev 292381)
@@ -0,0 +1,19 @@
+This tests moving the selection backwards at the end of a table by sentence granularity. The caret should move to the beginning of the table.
+| "\n"
+| <table>
+|   "\n  "
+|   <tbody>
+|     <tr>
+|       "\n    "
+|       <th>
+|         "<#selection-caret>First"
+|       "\n    "
+|       <th>
+|         "Second"
+|       " \n    "
+|       <th>
+|         id="target"
+|         "Third"
+|       "\n  "
+|     "\n"
+| "\n"

Added: trunk/LayoutTests/editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity.html (0 => 292381)


--- trunk/LayoutTests/editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity.html	2022-04-05 08:01:50 UTC (rev 292381)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<style>
+table, th, td {
+  border:1px solid black;
+}
+</style>
+<body>
+<div id="editor" contenteditable>
+<table>
+  <tr>
+    <th>First</th>
+    <th>Second</th> 
+    <th id="target">Third</th>
+  </tr>
+</table>
+</div>
+<script src=""
+<script>
+
+Markup.description('This tests moving the selection backwards at the end of a table by sentence granularity. The caret should move to the beginning of the table.');
+
+editor.focus();
+getSelection().setPosition(target, 5);
+getSelection().modify('move', 'left', 'sentence');
+
+Markup.dump(editor);
+
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity-expected.txt (0 => 292381)


--- trunk/LayoutTests/editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity-expected.txt	2022-04-05 08:01:50 UTC (rev 292381)
@@ -0,0 +1,20 @@
+This tests moving the selection forwards to the end of a table by sentence granularity. The caret should move to the end of the table.
+| "\n"
+| <table>
+|   "\n  "
+|   <tbody>
+|     <tr>
+|       "\n    "
+|       <th>
+|         id="target"
+|         "First"
+|       "\n    "
+|       <th>
+|         "Second"
+|       " \n    "
+|       <th>
+|         "Third"
+|       "\n  "
+|     "\n"
+| <#selection-caret>
+| "\n"

Added: trunk/LayoutTests/editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity.html (0 => 292381)


--- trunk/LayoutTests/editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity.html	2022-04-05 08:01:50 UTC (rev 292381)
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<style>
+table, th, td {
+  border:1px solid black;
+}
+</style>
+<body>
+<div id="editor" contenteditable>
+<table>
+  <tr>
+    <th  id="target">First</th>
+    <th>Second</th> 
+    <th>Third</th>
+  </tr>
+</table>
+</div>
+<script src=""
+<script>
+
+Markup.description('This tests moving the selection forwards to the end of a table by sentence granularity. The caret should move to the end of the table.');
+
+editor.focus();
+getSelection().setPosition(target, 0);
+getSelection().modify('move', 'right', 'sentence');
+
+Markup.dump(editor);
+
+</script>
+</body>
+</html>

Modified: trunk/LayoutTests/editing/text-iterator/backwards-text-iterator-basic-expected.txt (292380 => 292381)


--- trunk/LayoutTests/editing/text-iterator/backwards-text-iterator-basic-expected.txt	2022-04-05 07:32:25 UTC (rev 292380)
+++ trunk/LayoutTests/editing/text-iterator/backwards-text-iterator-basic-expected.txt	2022-04-05 08:01:50 UTC (rev 292381)
@@ -19,7 +19,7 @@
 PASS plainTextWithBackwardsTextIterator("<ul><li>hello<li>world</ul>WebKit") is "\n\nhello\nworld\nWebKit"
 
 PASS plainTextWithTextIterator("<table><tr><td>hello</td><td>world</td></table>") is "hello	world"
-PASS plainTextWithBackwardsTextIterator("<table><tr><td>hello</td><td>world</td></table>") is "\n\nhello\nworld"
+PASS plainTextWithBackwardsTextIterator("<table><tr><td>hello</td><td>world</td></table>") is "\n\nhello\tworld\t"
 
 PASS plainTextWithBackwardsTextIterator("img<img>iframe<iframe></iframe>meter<meter></meter>progress<progress></progress><legend>legend</legend>imgrole<span role='img'></span><button>button</button>") is "img,iframe,meter,progress,\nlegend,imgrole,button,"
 PASS plainTextWithBackwardsTextIterator("audio<audio controls><source src='' type='audio/mpeg'></audio>file") is "audio,file"

Modified: trunk/LayoutTests/editing/text-iterator/backwards-text-iterator-basic.html (292380 => 292381)


--- trunk/LayoutTests/editing/text-iterator/backwards-text-iterator-basic.html	2022-04-05 07:32:25 UTC (rev 292380)
+++ trunk/LayoutTests/editing/text-iterator/backwards-text-iterator-basic.html	2022-04-05 08:01:50 UTC (rev 292381)
@@ -48,7 +48,7 @@
 debug('');
 
 shouldBe('plainTextWithTextIterator("<table><tr><td>hello</td><td>world</td></table>")', '"hello\tworld"');
-shouldBe('plainTextWithBackwardsTextIterator("<table><tr><td>hello</td><td>world</td></table>")', '"\\n\\nhello\\nworld"');
+shouldBe('plainTextWithBackwardsTextIterator("<table><tr><td>hello</td><td>world</td></table>")', '"\\n\\nhello\\tworld\\t"');
 
 debug('');
 

Modified: trunk/Source/WebCore/ChangeLog (292380 => 292381)


--- trunk/Source/WebCore/ChangeLog	2022-04-05 07:32:25 UTC (rev 292380)
+++ trunk/Source/WebCore/ChangeLog	2022-04-05 08:01:50 UTC (rev 292381)
@@ -1,3 +1,20 @@
+2022-04-05  Megan Gardner  <megan_gard...@apple.com>
+
+        Moving forwards and backwards by sentences across a table results in different breaks in each direction.
+        https://bugs.webkit.org/show_bug.cgi?id=238621
+
+        Reviewed by Darin Adler.
+
+        Tests: editing/selection/move-selection-backwards-at-end-of-table-by-sentence-granularity.html
+               editing/selection/move-selection-forwards-at-beginning-of-table-by-sentence-granularity.html
+
+        We need to emit tabs even in a simplified backwards text iterator because otherwise traversing a table by
+        sentence granularity results in not having the same behavior backwards and forwards.
+
+        * editing/TextIterator.cpp:
+        (WebCore::SimplifiedBackwardsTextIterator::handleNonTextNode):
+        (WebCore::SimplifiedBackwardsTextIterator::exitNode):
+
 2022-04-05  Zan Dobersek  <zdober...@igalia.com>
 
         Unreviewed, reverting r292377.

Modified: trunk/Source/WebCore/editing/TextIterator.cpp (292380 => 292381)


--- trunk/Source/WebCore/editing/TextIterator.cpp	2022-04-05 07:32:25 UTC (rev 292380)
+++ trunk/Source/WebCore/editing/TextIterator.cpp	2022-04-05 08:01:50 UTC (rev 292381)
@@ -1352,10 +1352,11 @@
 }
 
 bool SimplifiedBackwardsTextIterator::handleNonTextNode()
-{    
-    // We can use a linefeed in place of a tab because this simple iterator is only used to
-    // find boundaries, not actual content. A linefeed breaks words, sentences, and paragraphs.
-    if (shouldEmitNewlineForNode(m_node, m_behaviors.contains(TextIteratorBehavior::EmitsOriginalText)) || shouldEmitNewlineAfterNode(*m_node) || shouldEmitTabBeforeNode(*m_node)) {
+{
+    if (shouldEmitTabBeforeNode(*m_node)) {
+        unsigned index = m_node->computeNodeIndex();
+        emitCharacter('\t', *m_node->parentNode(), index + 1, index + 1);
+    } else if (shouldEmitNewlineForNode(m_node, m_behaviors.contains(TextIteratorBehavior::EmitsOriginalText)) || shouldEmitNewlineAfterNode(*m_node)) {
         if (m_lastCharacter != '\n') {
             // Corresponds to the same check in TextIterator::exitNode.
             unsigned index = m_node->computeNodeIndex();
@@ -1369,7 +1370,9 @@
 
 void SimplifiedBackwardsTextIterator::exitNode()
 {
-    if (shouldEmitNewlineForNode(m_node, m_behaviors.contains(TextIteratorBehavior::EmitsOriginalText)) || shouldEmitNewlineBeforeNode(*m_node) || shouldEmitTabBeforeNode(*m_node)) {
+    if (shouldEmitTabBeforeNode(*m_node))
+        emitCharacter('\t', *m_node, 0, 0);
+    else if (shouldEmitNewlineForNode(m_node, m_behaviors.contains(TextIteratorBehavior::EmitsOriginalText)) || shouldEmitNewlineBeforeNode(*m_node)) {
         // The start of this emitted range is wrong. Ensuring correctness would require
         // VisiblePositions and so would be slow. previousBoundary expects this.
         emitCharacter('\n', *m_node, 0, 0);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to