Title: [242117] trunk
- Revision
- 242117
- Author
- [email protected]
- Date
- 2019-02-26 17:13:50 -0800 (Tue, 26 Feb 2019)
Log Message
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: trunk/LayoutTests/ChangeLog (242116 => 242117)
--- trunk/LayoutTests/ChangeLog 2019-02-27 00:59:49 UTC (rev 242116)
+++ trunk/LayoutTests/ChangeLog 2019-02-27 01:13:50 UTC (rev 242117)
@@ -1,3 +1,18 @@
+2019-02-26 Wenson Hsieh <[email protected]>
+
+ 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-26 Youenn Fablet <[email protected]>
Move service worker response validation from the service worker client to the service worker itself
Added: trunk/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node-expected.txt (0 => 242117)
--- trunk/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node-expected.txt 2019-02-27 01:13:50 UTC (rev 242117)
@@ -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: trunk/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node.html (0 => 242117)
--- trunk/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node.html (rev 0)
+++ trunk/LayoutTests/editing/undo/redo-text-insertion-in-non-editable-node.html 2019-02-27 01:13:50 UTC (rev 242117)
@@ -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: trunk/Source/WebCore/ChangeLog (242116 => 242117)
--- trunk/Source/WebCore/ChangeLog 2019-02-27 00:59:49 UTC (rev 242116)
+++ trunk/Source/WebCore/ChangeLog 2019-02-27 01:13:50 UTC (rev 242117)
@@ -1,3 +1,26 @@
+2019-02-26 Wenson Hsieh <[email protected]>
+
+ 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 <[email protected]>
Code quality cleanup in NeverDestroyed
Modified: trunk/Source/WebCore/editing/InsertIntoTextNodeCommand.cpp (242116 => 242117)
--- trunk/Source/WebCore/editing/InsertIntoTextNodeCommand.cpp 2019-02-27 00:59:49 UTC (rev 242116)
+++ trunk/Source/WebCore/editing/InsertIntoTextNodeCommand.cpp 2019-02-27 01:13:50 UTC (rev 242117)
@@ -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: trunk/Source/WebCore/editing/InsertIntoTextNodeCommand.h (242116 => 242117)
--- trunk/Source/WebCore/editing/InsertIntoTextNodeCommand.h 2019-02-27 00:59:49 UTC (rev 242116)
+++ trunk/Source/WebCore/editing/InsertIntoTextNodeCommand.h 2019-02-27 01:13:50 UTC (rev 242117)
@@ -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
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
