Title: [242477] releases/WebKitGTK/webkit-2.24
Revision
242477
Author
carlo...@webkit.org
Date
2019-03-05 09:21:28 -0800 (Tue, 05 Mar 2019)

Log Message

Merge r242117 - Remove conditional compile guard for InsertIntoTextNodeCommand::doReapply
https://bugs.webkit.org/show_bug.cgi?id=195067
<rdar://problem/44812080>

Reviewed by Tim Horton.

Source/WebCore:

This iOS-specific override was introduced to fix <rdar://problem/7114425>, in which the last typed character
would be revealed when redoing text input on iOS inside a password field. The associated change fixed this bug
by overriding doReapply on iOS to only insert text (instead of additionally handling password echo); however, it
really makes sense to skip password echo when redoing on all platforms, so we can just remove the platform-
specific guards around this logic.

Doing this allows us to add the `hasEditableStyle()` check on iOS when redoing text insertion, which results in
a very subtle behavior change covered by the new layout test below.

Test: editing/undo/redo-text-insertion-in-non-editable-node.html

* editing/InsertIntoTextNodeCommand.cpp:
(WebCore::InsertIntoTextNodeCommand::doReapply):
* editing/InsertIntoTextNodeCommand.h:

LayoutTests:

Add a new layout test to verify that redoing text insertion in a non-editable element (which was previously
editable) does not mutate the text nodes affected by editing. This test case currently fails on iOS, since we
take a separate codepath when redoing that does not contain this additional check.

* editing/undo/redo-text-insertion-in-non-editable-node-expected.txt: Added.
* editing/undo/redo-text-insertion-in-non-editable-node.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.24/LayoutTests/ChangeLog (242476 => 242477)


--- releases/WebKitGTK/webkit-2.24/LayoutTests/ChangeLog	2019-03-05 17:21:23 UTC (rev 242476)
+++ releases/WebKitGTK/webkit-2.24/LayoutTests/ChangeLog	2019-03-05 17:21:28 UTC (rev 242477)
@@ -1,3 +1,18 @@
+2019-02-26  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Remove conditional compile guard for InsertIntoTextNodeCommand::doReapply
+        https://bugs.webkit.org/show_bug.cgi?id=195067
+        <rdar://problem/44812080>
+
+        Reviewed by Tim Horton.
+
+        Add a new layout test to verify that redoing text insertion in a non-editable element (which was previously
+        editable) does not mutate the text nodes affected by editing. This test case currently fails on iOS, since we
+        take a separate codepath when redoing that does not contain this additional check.
+
+        * editing/undo/redo-text-insertion-in-non-editable-node-expected.txt: Added.
+        * editing/undo/redo-text-insertion-in-non-editable-node.html: Added.
+
 2019-02-25  Sihui Liu  <sihui_...@apple.com>
 
         IndexedDB: IDBDatabase and IDBTransaction are leaked in layout tests

Added: releases/WebKitGTK/webkit-2.24/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node-expected.txt (0 => 242477)


--- releases/WebKitGTK/webkit-2.24/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.24/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node-expected.txt	2019-03-05 17:21:28 UTC (rev 242477)
@@ -0,0 +1,11 @@
+This test verifies that redoing text insertion in a non-editable element is a no-op.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS textNode.data is "Hello"
+PASS textNode.data is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: releases/WebKitGTK/webkit-2.24/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node.html (0 => 242477)


--- releases/WebKitGTK/webkit-2.24/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.24/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node.html	2019-03-05 17:21:28 UTC (rev 242477)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<div id="editor" contenteditable></div>
+</body>
+<script>
+description("This test verifies that redoing text insertion in a non-editable element is a no-op.");
+editor.focus();
+
+document.execCommand("InsertText", true, "Hello");
+const textNode = editor.firstChild;
+shouldBeEqualToString("textNode.data", "Hello");
+
+document.execCommand("Undo");
+editor.setAttribute("contenteditable", "false");
+document.execCommand("Redo");
+
+shouldBeEqualToString("textNode.data", "");
+</script>
+</html>

Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/ChangeLog (242476 => 242477)


--- releases/WebKitGTK/webkit-2.24/Source/WebCore/ChangeLog	2019-03-05 17:21:23 UTC (rev 242476)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/ChangeLog	2019-03-05 17:21:28 UTC (rev 242477)
@@ -1,3 +1,26 @@
+2019-02-26  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Remove conditional compile guard for InsertIntoTextNodeCommand::doReapply
+        https://bugs.webkit.org/show_bug.cgi?id=195067
+        <rdar://problem/44812080>
+
+        Reviewed by Tim Horton.
+
+        This iOS-specific override was introduced to fix <rdar://problem/7114425>, in which the last typed character
+        would be revealed when redoing text input on iOS inside a password field. The associated change fixed this bug
+        by overriding doReapply on iOS to only insert text (instead of additionally handling password echo); however, it
+        really makes sense to skip password echo when redoing on all platforms, so we can just remove the platform-
+        specific guards around this logic.
+
+        Doing this allows us to add the `hasEditableStyle()` check on iOS when redoing text insertion, which results in
+        a very subtle behavior change covered by the new layout test below.
+
+        Test: editing/undo/redo-text-insertion-in-non-editable-node.html
+
+        * editing/InsertIntoTextNodeCommand.cpp:
+        (WebCore::InsertIntoTextNodeCommand::doReapply):
+        * editing/InsertIntoTextNodeCommand.h:
+
 2019-02-26  Keith Miller  <keith_mil...@apple.com>
 
         Code quality cleanup in NeverDestroyed

Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/editing/InsertIntoTextNodeCommand.cpp (242476 => 242477)


--- releases/WebKitGTK/webkit-2.24/Source/WebCore/editing/InsertIntoTextNodeCommand.cpp	2019-03-05 17:21:23 UTC (rev 242476)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/editing/InsertIntoTextNodeCommand.cpp	2019-03-05 17:21:28 UTC (rev 242477)
@@ -65,18 +65,14 @@
     m_node->insertData(m_offset, m_text);
 }
 
-#if PLATFORM(IOS_FAMILY)
-
-// FIXME: Why would reapply be iOS-specific?
 void InsertIntoTextNodeCommand::doReapply()
 {
-    // FIXME: Shouldn't this have a hasEditableStyle check?
+    if (!m_node->hasEditableStyle())
+        return;
 
     m_node->insertData(m_offset, m_text);
 }
 
-#endif
-    
 void InsertIntoTextNodeCommand::doUnapply()
 {
     if (!m_node->hasEditableStyle())

Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/editing/InsertIntoTextNodeCommand.h (242476 => 242477)


--- releases/WebKitGTK/webkit-2.24/Source/WebCore/editing/InsertIntoTextNodeCommand.h	2019-03-05 17:21:23 UTC (rev 242476)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/editing/InsertIntoTextNodeCommand.h	2019-03-05 17:21:28 UTC (rev 242477)
@@ -46,9 +46,7 @@
 private:
     void doApply() override;
     void doUnapply() override;
-#if PLATFORM(IOS_FAMILY)
     void doReapply() override;
-#endif
     
 #ifndef NDEBUG
     void getNodesInCommand(HashSet<Node*>&) override;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to