Smart quoting could move the caret backwards in some configurations
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Feb 2015 21:52:49 +0000 (21:52 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 3 Feb 2015 21:52:49 +0000 (21:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=141203
Source/WebCore:

<rdar://problem/17452543>

Reviewed by Enrica Casucci.

The bug was caused by markAndReplaceFor not running the code to preserve the selection after
text replacement only when smart quote is enabled. Furthermore, when smart link was disabled,
we never applied smart quote due to the following condition at line 2502:

if (!(shouldPerformReplacement || shouldCheckForCorrection || shouldMarkLink) || !doReplacement)
    continue;

This condition prevented the code to apply smart quote from running when both continuous
spellchecking, smart link, and text replacement are disabled.

Fixed the bug by treating smart quotes and smart dashes like any other text replacement and set
shouldPerformReplacement to true whenever either one of those text checking options are present.

Smart link didn't have this issue due to the explicit check for shouldMarkLink.

Smart dashes didn't suffer this problem either because dashes replacement happens only once
the caret has moved past the dashes but his patch makes go through the same code path to preserve
the selection as well for consistency.

Test: editing/inserting/smart-quote-with-all-configurations.html

* editing/Editor.cpp:
(WebCore::Editor::markAndReplaceFor):

LayoutTests:

Reviewed by Enrica Casucci.

Added a regression test for smart quote under all combinations of
spellchecking and substitution configurations.

* editing/inserting/smart-quote-with-all-configurations-expected.txt: Added.
* editing/inserting/smart-quote-with-all-configurations.html: Added.
* platform/efl/TestExpectations:
* platform/gtk/TestExpectations:
* platform/ios-simulator-wk2/TestExpectations:
* platform/win/TestExpectations:
* platform/wk2/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/editing/inserting/smart-quote-with-all-configurations.html [new file with mode: 0644]
LayoutTests/platform/efl/TestExpectations
LayoutTests/platform/gtk/TestExpectations
LayoutTests/platform/ios-simulator-wk2/TestExpectations
LayoutTests/platform/win/TestExpectations
LayoutTests/platform/wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp

index 597381059263a7b08703c69b85cda0688a8b1d49..00abcf179f017dae4dac414f251b16215603f995 100644 (file)
@@ -1,3 +1,21 @@
+2015-02-03  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Smart quoting could move the caret backwards in some configurations
+        https://bugs.webkit.org/show_bug.cgi?id=141203
+
+        Reviewed by Enrica Casucci.
+
+        Added a regression test for smart quote under all combinations of
+        spellchecking and substitution configurations.
+
+        * editing/inserting/smart-quote-with-all-configurations-expected.txt: Added.
+        * editing/inserting/smart-quote-with-all-configurations.html: Added.
+        * platform/efl/TestExpectations:
+        * platform/gtk/TestExpectations:
+        * platform/ios-simulator-wk2/TestExpectations:
+        * platform/win/TestExpectations:
+        * platform/wk2/TestExpectations:
+
 2015-02-02  Enrica Casucci  <enrica@apple.com>
 
         Additional emoji support.
diff --git a/LayoutTests/editing/inserting/smart-quote-with-all-configurations.html b/LayoutTests/editing/inserting/smart-quote-with-all-configurations.html
new file mode 100644 (file)
index 0000000..75ee9c3
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="editor" contenteditable><br></div>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+function config(config) {
+    editor.textContent = '';
+    internals.setContinuousSpellCheckingEnabled(config.continuousSpellchecking);
+    internals.setAutomaticQuoteSubstitutionEnabled(config.smartQuote);
+    internals.setAutomaticLinkDetectionEnabled(config.smartLink);
+    internals.setAutomaticDashSubstitutionEnabled(config.smartDash);
+    internals.setAutomaticTextReplacementEnabled(config.textReplacement);
+    internals.setAutomaticSpellingCorrectionEnabled(config.autocorrection);
+}
+
+function type(text) {
+    document.execCommand('InsertText', false, text);
+}
+
+function tryAllCombinations(configOptions, config) {
+    if (!configOptions.length) {
+        debug('');
+        evalAndLog('config(' + JSON.stringify(config) + ')');
+        shouldBe('type("We\'re"); type(" "); type("good"); editor.textContent', '"We\u2019re good"');
+        return;
+    }
+    var firstConfigOption = configOptions[0];
+    var remainingOptions = configOptions.slice(1);
+    config[firstConfigOption] = true;
+    tryAllCombinations(remainingOptions, config);
+
+    config[firstConfigOption] = false;
+    tryAllCombinations(remainingOptions, config);
+}
+
+var editor = document.getElementById('editor');
+editor.focus();
+
+if (!window.internals)
+    testFailed("This test requires internals to be ran manually. To test manually, type \"We're good\" with all combinations of spellchecking and substitutions options.");
+else
+    tryAllCombinations(['continuousSpellchecking', 'smartLink', 'smartDash', 'textReplacement', 'autocorrection'], {smartQuote: true});
+
+</script>
+</body>
+</html>
index d2f40f734d2622165b4f94dcbbab501a89595c61..73e349c34fa56192803079614afba95a3497e747 100644 (file)
@@ -1321,6 +1321,7 @@ webkit.org/b/83704 media/event-attributes.html [ Failure Crash ]
 # EFL's LayoutTestController does not implement setAutomaticLinkDetectionEnabled
 webkit.org/b/85463 editing/inserting/typing-space-to-trigger-smart-link.html
 webkit.org/b/85463 editing/inserting/smart-link-when-caret-is-moved-before-URL.html
+editing/inserting/smart-quote-with-all-configurations.html
 
 # Tests failing due to rounding problems in colors inside pixman
 webkit.org/b/49964 fast/canvas/canvas-fillPath-shadow.html [ Failure ]
index e61a05f85080852690fbb28175fbbc2e6d6857b5..614f0603664ad2efda38fe1d2c6b6d636e993ae5 100644 (file)
@@ -203,7 +203,8 @@ webkit.org/b/99068 editing/pasteboard/clipboard-customData.html [ Failure ]
 
 # setAutomaticLinkDetectionEnabled is not yet implemented.
 webkit.org/b/99069 editing/inserting/typing-space-to-trigger-smart-link.html [ Failure ]
-webkit.org/b/85463 editing/inserting/smart-link-when-caret-is-moved-before-URL.html [ Failure ]
+webkit.org/b/99069 editing/inserting/smart-link-when-caret-is-moved-before-URL.html [ Failure ]
+webkit.org/b/99069 editing/inserting/smart-quote-with-all-configurations.html [ Failure ]
 
 # Quota API is not supported.
 webkit.org/b/98942 storage/storageinfo-missing-arguments.html [ Failure ]
index efeb99ebe25f2421ce10c32bf9a4d93fa5a988f9..c487bc739d63519864e23edca1acc5d8168626ba 100644 (file)
@@ -1587,6 +1587,7 @@ editing/inserting/insert-tab-002.html [ Failure ]
 editing/inserting/insert-tab-004.html [ Failure ]
 editing/inserting/paragraph-outside-nested-divs.html [ Failure ]
 editing/inserting/smart-link-when-caret-is-moved-before-URL.html [ Failure ]
+editing/inserting/smart-quote-with-all-configurations.html [ Failure ]
 editing/inserting/typing-001.html [ Failure ]
 editing/inserting/typing-003.html [ Failure ]
 editing/inserting/typing-around-br-001.html [ Failure ]
index b4486db05c373cc124a4fe8fc145e20260ffe191..819ec8078d4921c388410c41a0ec355993d41706 100644 (file)
@@ -1137,6 +1137,7 @@ editing/execCommand/find-after-replace.html [ Failure ]
 # TODO testRunner::setAutomaticLinkDetectionEnabled isn't implemented
 editing/inserting/typing-space-to-trigger-smart-link.html [ Skip ]
 editing/inserting/smart-link-when-caret-is-moved-before-URL.html [ Skip ]
+editing/inserting/smart-quote-with-all-configurations.html [ Skip ]
 
 ###### Text Iterator
 # Plain text controller currently in Mac DumpRenderTree only.
index 66246c9a10a471f8c8ecaaeff356c814f2a9e4d6..4c544bca3e8bc92afec9407b3d455a697c687ca0 100644 (file)
@@ -353,6 +353,7 @@ fast/block/lineboxcontain/font.html
 # WTR needs an implementation of setAutomaticLinkDetectionEnabled.
 # https://bugs.webkit.org/show_bug.cgi?id=87162
 editing/inserting/smart-link-when-caret-is-moved-before-URL.html
+editing/inserting/smart-quote-with-all-configurations.html
 editing/inserting/typing-space-to-trigger-smart-link.html
 
 # No CORS support for media elements is implemented yet.
index 701f98550514ce40e7f840e8dca57203442958a2..6d5dd8217f15a454febbb775532e929be575983e 100644 (file)
@@ -1,3 +1,35 @@
+2015-02-03  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Smart quoting could move the caret backwards in some configurations
+        https://bugs.webkit.org/show_bug.cgi?id=141203
+        <rdar://problem/17452543>
+
+        Reviewed by Enrica Casucci.
+
+        The bug was caused by markAndReplaceFor not running the code to preserve the selection after
+        text replacement only when smart quote is enabled. Furthermore, when smart link was disabled,
+        we never applied smart quote due to the following condition at line 2502:
+
+        if (!(shouldPerformReplacement || shouldCheckForCorrection || shouldMarkLink) || !doReplacement)
+            continue;
+
+        This condition prevented the code to apply smart quote from running when both continuous
+        spellchecking, smart link, and text replacement are disabled.
+
+        Fixed the bug by treating smart quotes and smart dashes like any other text replacement and set
+        shouldPerformReplacement to true whenever either one of those text checking options are present.
+
+        Smart link didn't have this issue due to the explicit check for shouldMarkLink.
+
+        Smart dashes didn't suffer this problem either because dashes replacement happens only once
+        the caret has moved past the dashes but his patch makes go through the same code path to preserve
+        the selection as well for consistency.
+
+        Test: editing/inserting/smart-quote-with-all-configurations.html
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::markAndReplaceFor):
+
 2015-02-02  Enrica Casucci  <enrica@apple.com>
 
         Additional emoji support.
index 15d9b494ba27e9cc125c183a04a58892a75635bc..4f05b4ebb131c2e33a69ba9b3c11474df2024d2a 100644 (file)
@@ -2418,7 +2418,7 @@ void Editor::markAndReplaceFor(PassRefPtr<SpellCheckRequest> request, const Vect
     const bool shouldMarkSpelling = textCheckingOptions & TextCheckingTypeSpelling;
     const bool shouldMarkGrammar = textCheckingOptions & TextCheckingTypeGrammar;
     const bool shouldMarkLink = textCheckingOptions & TextCheckingTypeLink;
-    const bool shouldPerformReplacement = textCheckingOptions & TextCheckingTypeReplacement;
+    const bool shouldPerformReplacement = textCheckingOptions & (TextCheckingTypeQuote | TextCheckingTypeDash | TextCheckingTypeReplacement);
     const bool shouldShowCorrectionPanel = textCheckingOptions & TextCheckingTypeShowCorrectionPanel;
     const bool shouldCheckForCorrection = shouldShowCorrectionPanel || (textCheckingOptions & TextCheckingTypeCorrection);
 #if !USE(AUTOCORRECTION_PANEL)