Title: [179569] trunk
Revision
179569
Author
[email protected]
Date
2015-02-03 13:52:49 -0800 (Tue, 03 Feb 2015)

Log Message

Smart quoting could move the caret backwards in some configurations
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:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (179568 => 179569)


--- trunk/LayoutTests/ChangeLog	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/LayoutTests/ChangeLog	2015-02-03 21:52:49 UTC (rev 179569)
@@ -1,3 +1,21 @@
+2015-02-03  Ryosuke Niwa  <[email protected]>
+
+        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  <[email protected]>
 
         Additional emoji support.

Added: trunk/LayoutTests/editing/inserting/smart-quote-with-all-configurations.html (0 => 179569)


--- trunk/LayoutTests/editing/inserting/smart-quote-with-all-configurations.html	                        (rev 0)
+++ trunk/LayoutTests/editing/inserting/smart-quote-with-all-configurations.html	2015-02-03 21:52:49 UTC (rev 179569)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="editor" contenteditable><br></div>
+<script src=""
+<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>

Modified: trunk/LayoutTests/platform/efl/TestExpectations (179568 => 179569)


--- trunk/LayoutTests/platform/efl/TestExpectations	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/LayoutTests/platform/efl/TestExpectations	2015-02-03 21:52:49 UTC (rev 179569)
@@ -1321,6 +1321,7 @@
 # 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 ]

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (179568 => 179569)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2015-02-03 21:52:49 UTC (rev 179569)
@@ -203,7 +203,8 @@
 
 # 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 ]

Modified: trunk/LayoutTests/platform/ios-simulator-wk2/TestExpectations (179568 => 179569)


--- trunk/LayoutTests/platform/ios-simulator-wk2/TestExpectations	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/LayoutTests/platform/ios-simulator-wk2/TestExpectations	2015-02-03 21:52:49 UTC (rev 179569)
@@ -1587,6 +1587,7 @@
 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 ]

Modified: trunk/LayoutTests/platform/win/TestExpectations (179568 => 179569)


--- trunk/LayoutTests/platform/win/TestExpectations	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/LayoutTests/platform/win/TestExpectations	2015-02-03 21:52:49 UTC (rev 179569)
@@ -1137,6 +1137,7 @@
 # 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.

Modified: trunk/LayoutTests/platform/wk2/TestExpectations (179568 => 179569)


--- trunk/LayoutTests/platform/wk2/TestExpectations	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/LayoutTests/platform/wk2/TestExpectations	2015-02-03 21:52:49 UTC (rev 179569)
@@ -353,6 +353,7 @@
 # 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.

Modified: trunk/Source/WebCore/ChangeLog (179568 => 179569)


--- trunk/Source/WebCore/ChangeLog	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/Source/WebCore/ChangeLog	2015-02-03 21:52:49 UTC (rev 179569)
@@ -1,3 +1,35 @@
+2015-02-03  Ryosuke Niwa  <[email protected]>
+
+        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  <[email protected]>
 
         Additional emoji support.

Modified: trunk/Source/WebCore/editing/Editor.cpp (179568 => 179569)


--- trunk/Source/WebCore/editing/Editor.cpp	2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/Source/WebCore/editing/Editor.cpp	2015-02-03 21:52:49 UTC (rev 179569)
@@ -2418,7 +2418,7 @@
     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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to