[Mac] REGRESSION: Auto substitution strips new lines
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Apr 2013 22:46:01 +0000 (22:46 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Apr 2013 22:46:01 +0000 (22:46 +0000)
https://bugs.webkit.org/show_bug.cgi?id=114537

Reviewed by Enrica Casucci.

Source/WebCore:

The bug was caused by SpellingCorrectionCommand's use of InsertTextCommand. This command can't insert
new lines and there's even an assertion for it. Use TypingCommand::insertText instead.

Since TypingCommand::insertText calls appliedEditing on its own, we need to avoid calling that again in
CompositeEditCommand::apply after SpellingCorrectionCommand::doApply. Replaced the check in apply to use
callsAppliedEditingInDoApply instead of isTypingCommand, and added callsAppliedEditingInDoApply to both
TypingCommand and SpellingCorrectionCommand to return true (it returns false by default).

Test: platform/mac/editing/spelling/autocorrection-with-multi-line-text.html

* editing/CompositeEditCommand.cpp:
(WebCore::CompositeEditCommand::apply): Use TypingCommand::insertText instead of InsertTextCommand
(WebCore::CompositeEditCommand::callsAppliedEditingInDoApply): Added. Returns false.
* editing/CompositeEditCommand.h:
(CompositeEditCommand):
* editing/SpellingCorrectionCommand.cpp:
(WebCore::SpellingCorrectionCommand::doApply):
(WebCore::SpellingCorrectionCommand::callsAppliedEditingInDoApply): Added. Returns true.
* editing/SpellingCorrectionCommand.h:
(SpellingCorrectionCommand):
* editing/TypingCommand.cpp:
(WebCore::TypingCommand::callsAppliedEditingInDoApply): Added. Returns true.
* editing/TypingCommand.h:
(TypingCommand):

Tools:

Add a rule to replace "helloleworld" by "hello\nworld" for testing purpose.

* DumpRenderTree/mac/DumpRenderTree.mm:
(resetDefaultsToConsistentValues):

LayoutTests:

Added a regression test. Unfortunately, we can't setup text substituion in DumpRenderTree
so use NSSpellChecker to exercise the relevant code path.

* platform/mac/editing/spelling/autocorrection-with-multi-line-text-expected.txt: Added.
* platform/mac/editing/spelling/autocorrection-with-multi-line-text.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@148306 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac/editing/spelling/autocorrection-with-multi-line-text-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/editing/spelling/autocorrection-with-multi-line-text.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/CompositeEditCommand.cpp
Source/WebCore/editing/CompositeEditCommand.h
Source/WebCore/editing/SpellingCorrectionCommand.cpp
Source/WebCore/editing/SpellingCorrectionCommand.h
Source/WebCore/editing/TypingCommand.cpp
Source/WebCore/editing/TypingCommand.h
Tools/ChangeLog
Tools/DumpRenderTree/mac/DumpRenderTree.mm

index 3c43475..aa10601 100644 (file)
@@ -1,3 +1,16 @@
+2013-04-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [Mac] REGRESSION: Auto substitution strips new lines
+        https://bugs.webkit.org/show_bug.cgi?id=114537
+
+        Reviewed by Enrica Casucci.
+
+        Added a regression test. Unfortunately, we can't setup text substituion in DumpRenderTree
+        so use NSSpellChecker to exercise the relevant code path.
+
+        * platform/mac/editing/spelling/autocorrection-with-multi-line-text-expected.txt: Added.
+        * platform/mac/editing/spelling/autocorrection-with-multi-line-text.html: Added.
+
 2013-04-12  Commit Queue  <rniwa@webkit.org>
 
         Unreviewed, rolling out r147942, r148026, and r148092.
diff --git a/LayoutTests/platform/mac/editing/spelling/autocorrection-with-multi-line-text-expected.txt b/LayoutTests/platform/mac/editing/spelling/autocorrection-with-multi-line-text-expected.txt
new file mode 100644 (file)
index 0000000..8fee57b
--- /dev/null
@@ -0,0 +1,10 @@
+This tests auto-substitution with a multi-line text works.
+To manually test, open Language & Text system preference and setup "helloleworld" to be replaced by
+"helloworld" (copy paste it) and reload this page. You should see hello and world in two distinct lines.
+| "hello"
+| <div>
+|   "world"
+|   <br>
+|   <div>
+|     <#selection-caret>
+|     <br>
diff --git a/LayoutTests/platform/mac/editing/spelling/autocorrection-with-multi-line-text.html b/LayoutTests/platform/mac/editing/spelling/autocorrection-with-multi-line-text.html
new file mode 100644 (file)
index 0000000..067fce2
--- /dev/null
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p id="description">This tests auto-substitution with a multi-line text works.
+To manually test, open Language &amp; Text system preference and setup "helloleworld" to be replaced by
+"hello<br>world" (copy paste it) and reload this page. You should see hello and world in two distinct lines.</p>
+<div id="editor" contenteditable></div>
+<script src="../../../../resources/dump-as-markup.js"></script>
+<script>
+
+var editor = document.getElementById('editor');
+document.getElementById('description');
+editor.focus();
+document.execCommand('InsertText', false, 'h');
+document.execCommand('InsertText', false, 'e');
+document.execCommand('InsertText', false, 'l');
+document.execCommand('InsertText', false, 'l');
+document.execCommand('InsertText', false, 'o');
+document.execCommand('InsertText', false, 'l');
+document.execCommand('InsertText', false, 'f');
+document.execCommand('InsertText', false, 'w');
+document.execCommand('InsertText', false, 'o');
+document.execCommand('InsertText', false, 'r');
+document.execCommand('InsertText', false, 'l');
+document.execCommand('InsertText', false, 'd');
+document.execCommand('InsertParagraph');
+
+Markup.description(document.getElementById('description').textContent);
+Markup.dump('editor');
+
+</script>
+</body>
+</html>
index dde88eb..fe184cc 100644 (file)
@@ -1,3 +1,35 @@
+2013-04-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [Mac] REGRESSION: Auto substitution strips new lines
+        https://bugs.webkit.org/show_bug.cgi?id=114537
+
+        Reviewed by Enrica Casucci.
+
+        The bug was caused by SpellingCorrectionCommand's use of InsertTextCommand. This command can't insert
+        new lines and there's even an assertion for it. Use TypingCommand::insertText instead.
+
+        Since TypingCommand::insertText calls appliedEditing on its own, we need to avoid calling that again in
+        CompositeEditCommand::apply after SpellingCorrectionCommand::doApply. Replaced the check in apply to use
+        callsAppliedEditingInDoApply instead of isTypingCommand, and added callsAppliedEditingInDoApply to both
+        TypingCommand and SpellingCorrectionCommand to return true (it returns false by default).
+
+        Test: platform/mac/editing/spelling/autocorrection-with-multi-line-text.html
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::apply): Use TypingCommand::insertText instead of InsertTextCommand
+        (WebCore::CompositeEditCommand::callsAppliedEditingInDoApply): Added. Returns false.
+        * editing/CompositeEditCommand.h:
+        (CompositeEditCommand):
+        * editing/SpellingCorrectionCommand.cpp:
+        (WebCore::SpellingCorrectionCommand::doApply):
+        (WebCore::SpellingCorrectionCommand::callsAppliedEditingInDoApply): Added. Returns true.
+        * editing/SpellingCorrectionCommand.h:
+        (SpellingCorrectionCommand):
+        * editing/TypingCommand.cpp:
+        (WebCore::TypingCommand::callsAppliedEditingInDoApply): Added. Returns true.
+        * editing/TypingCommand.h:
+        (TypingCommand):
+
 2013-04-12  Brendan Long  <b.long@cablelabs.com>
 
         Refactor TextTrack and TextTrackList to make it easier to add audio and video tracks
index 85745ae..a5e3a28 100644 (file)
@@ -216,7 +216,7 @@ void CompositeEditCommand::apply()
 
     // Only need to call appliedEditing for top-level commands,
     // and TypingCommands do it on their own (see TypingCommand::typingAddedToOpenCommand).
-    if (!isTypingCommand())
+    if (!callsAppliedEditingInDoApply())
         frame->editor()->appliedEditing(this);
     setShouldRetainAutocorrectionIndicator(false);
 }
@@ -246,6 +246,11 @@ bool CompositeEditCommand::isTypingCommand() const
     return false;
 }
 
+bool CompositeEditCommand::callsAppliedEditingInDoApply() const
+{
+    return false;
+}
+
 bool CompositeEditCommand::shouldRetainAutocorrectionIndicator() const
 {
     return false;
index 4c8c0b1..b19960c 100644 (file)
@@ -82,6 +82,7 @@ public:
 
     virtual bool isCreateLinkCommand() const;
     virtual bool isTypingCommand() const;
+    virtual bool callsAppliedEditingInDoApply() const;
     virtual bool isDictationCommand() const { return false; }
     virtual bool preservesTypingStyle() const;
     virtual bool shouldRetainAutocorrectionIndicator() const;
index 673b682..114edcc 100644 (file)
@@ -30,9 +30,9 @@
 #include "Document.h"
 #include "DocumentFragment.h"
 #include "Frame.h"
-#include "InsertTextCommand.h"
 #include "SetSelectionCommand.h"
 #include "TextIterator.h"
+#include "TypingCommand.h"
 #include "markup.h"
 
 namespace WebCore {
@@ -101,7 +101,7 @@ void SpellingCorrectionCommand::doApply()
 #if USE(AUTOCORRECTION_PANEL)
     applyCommandToComposite(SpellingCorrectionRecordUndoCommand::create(document(), m_corrected, m_correction));
 #endif
-    applyCommandToComposite(InsertTextCommand::create(document(), m_correction));
+    TypingCommand::insertText(document(), m_correction, TypingCommand::PreventSpellChecking);
 }
 
 bool SpellingCorrectionCommand::shouldRetainAutocorrectionIndicator() const
@@ -109,4 +109,9 @@ bool SpellingCorrectionCommand::shouldRetainAutocorrectionIndicator() const
     return true;
 }
 
+bool SpellingCorrectionCommand::callsAppliedEditingInDoApply() const
+{
+    return true;
+}
+
 } // namespace WebCore
index 6136366..5d7889d 100644 (file)
@@ -41,6 +41,7 @@ private:
     SpellingCorrectionCommand(PassRefPtr<Range> rangeToBeCorrected, const String& correction);
     virtual void doApply();
     virtual bool shouldRetainAutocorrectionIndicator() const;
+    virtual bool callsAppliedEditingInDoApply() const;
 
     RefPtr<Range> m_rangeToBeCorrected;
     VisibleSelection m_selectionToBeCorrected;
index 248b3c9..a9fba68 100644 (file)
@@ -645,4 +645,9 @@ bool TypingCommand::isTypingCommand() const
     return true;
 }
 
+bool TypingCommand::callsAppliedEditingInDoApply() const
+{
+    return true;
+}
+
 } // namespace WebCore
index 950eec8..19fbfee 100644 (file)
@@ -100,6 +100,7 @@ private:
     virtual void doApply();
     virtual EditAction editingAction() const;
     virtual bool isTypingCommand() const;
+    virtual bool callsAppliedEditingInDoApply() const;
     virtual bool preservesTypingStyle() const { return m_preservesTypingStyle; }
     virtual bool shouldRetainAutocorrectionIndicator() const
     {
index fcda8aa..e036f27 100644 (file)
@@ -1,3 +1,15 @@
+2013-04-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [Mac] REGRESSION: Auto substitution strips new lines
+        https://bugs.webkit.org/show_bug.cgi?id=114537
+
+        Reviewed by Enrica Casucci.
+
+        Add a rule to replace "helloleworld" by "hello\nworld" for testing purpose.
+
+        * DumpRenderTree/mac/DumpRenderTree.mm:
+        (resetDefaultsToConsistentValues):
+
 2013-04-09  Roger Fong  <roger_fong@apple.com>
 
         Re-enable WinEWS tests.
index eb65b6b..0a8cfe5 100644 (file)
@@ -606,7 +606,7 @@ static void resetDefaultsToConsistentValues()
         @"message", @"mesage",
         @"would", @"wouldn",
         @"welcome", @"wellcome",
-        @"uppercase", @"upper case",
+        @"hello\nworld", @"hellolfworld",
         nil] forKey:@"NSTestCorrectionDictionary"];
 #endif