<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[179569] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/179569">179569</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2015-02-03 13:52:49 -0800 (Tue, 03 Feb 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>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:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsplatformeflTestExpectations">trunk/LayoutTests/platform/efl/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformgtkTestExpectations">trunk/LayoutTests/platform/gtk/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformiossimulatorwk2TestExpectations">trunk/LayoutTests/platform/ios-simulator-wk2/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformwinTestExpectations">trunk/LayoutTests/platform/win/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformwk2TestExpectations">trunk/LayoutTests/platform/wk2/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreeditingEditorcpp">trunk/Source/WebCore/editing/Editor.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestseditinginsertingsmartquotewithallconfigurationshtml">trunk/LayoutTests/editing/inserting/smart-quote-with-all-configurations.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-02-03 21:40:24 UTC (rev 179568)
+++ trunk/LayoutTests/ChangeLog 2015-02-03 21:52:49 UTC (rev 179569)
</span><span class="lines">@@ -1,3 +1,21 @@
</span><ins>+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:
+
</ins><span class="cx"> 2015-02-02 Enrica Casucci <enrica@apple.com>
</span><span class="cx">
</span><span class="cx"> Additional emoji support.
</span></span></pre></div>
<a id="trunkLayoutTestseditinginsertingsmartquotewithallconfigurationshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/editing/inserting/smart-quote-with-all-configurations.html (0 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,50 @@
</span><ins>+<!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>
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformeflTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/efl/TestExpectations (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1321,6 +1321,7 @@
</span><span class="cx"> # EFL's LayoutTestController does not implement setAutomaticLinkDetectionEnabled
</span><span class="cx"> webkit.org/b/85463 editing/inserting/typing-space-to-trigger-smart-link.html
</span><span class="cx"> webkit.org/b/85463 editing/inserting/smart-link-when-caret-is-moved-before-URL.html
</span><ins>+editing/inserting/smart-quote-with-all-configurations.html
</ins><span class="cx">
</span><span class="cx"> # Tests failing due to rounding problems in colors inside pixman
</span><span class="cx"> webkit.org/b/49964 fast/canvas/canvas-fillPath-shadow.html [ Failure ]
</span></span></pre></div>
<a id="trunkLayoutTestsplatformgtkTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/gtk/TestExpectations (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -203,7 +203,8 @@
</span><span class="cx">
</span><span class="cx"> # setAutomaticLinkDetectionEnabled is not yet implemented.
</span><span class="cx"> webkit.org/b/99069 editing/inserting/typing-space-to-trigger-smart-link.html [ Failure ]
</span><del>-webkit.org/b/85463 editing/inserting/smart-link-when-caret-is-moved-before-URL.html [ Failure ]
</del><ins>+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 ]
</ins><span class="cx">
</span><span class="cx"> # Quota API is not supported.
</span><span class="cx"> webkit.org/b/98942 storage/storageinfo-missing-arguments.html [ Failure ]
</span></span></pre></div>
<a id="trunkLayoutTestsplatformiossimulatorwk2TestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/ios-simulator-wk2/TestExpectations (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1587,6 +1587,7 @@
</span><span class="cx"> editing/inserting/insert-tab-004.html [ Failure ]
</span><span class="cx"> editing/inserting/paragraph-outside-nested-divs.html [ Failure ]
</span><span class="cx"> editing/inserting/smart-link-when-caret-is-moved-before-URL.html [ Failure ]
</span><ins>+editing/inserting/smart-quote-with-all-configurations.html [ Failure ]
</ins><span class="cx"> editing/inserting/typing-001.html [ Failure ]
</span><span class="cx"> editing/inserting/typing-003.html [ Failure ]
</span><span class="cx"> editing/inserting/typing-around-br-001.html [ Failure ]
</span></span></pre></div>
<a id="trunkLayoutTestsplatformwinTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/win/TestExpectations (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1137,6 +1137,7 @@
</span><span class="cx"> # TODO testRunner::setAutomaticLinkDetectionEnabled isn't implemented
</span><span class="cx"> editing/inserting/typing-space-to-trigger-smart-link.html [ Skip ]
</span><span class="cx"> editing/inserting/smart-link-when-caret-is-moved-before-URL.html [ Skip ]
</span><ins>+editing/inserting/smart-quote-with-all-configurations.html [ Skip ]
</ins><span class="cx">
</span><span class="cx"> ###### Text Iterator
</span><span class="cx"> # Plain text controller currently in Mac DumpRenderTree only.
</span></span></pre></div>
<a id="trunkLayoutTestsplatformwk2TestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/wk2/TestExpectations (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -353,6 +353,7 @@
</span><span class="cx"> # WTR needs an implementation of setAutomaticLinkDetectionEnabled.
</span><span class="cx"> # https://bugs.webkit.org/show_bug.cgi?id=87162
</span><span class="cx"> editing/inserting/smart-link-when-caret-is-moved-before-URL.html
</span><ins>+editing/inserting/smart-quote-with-all-configurations.html
</ins><span class="cx"> editing/inserting/typing-space-to-trigger-smart-link.html
</span><span class="cx">
</span><span class="cx"> # No CORS support for media elements is implemented yet.
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,35 @@
</span><ins>+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):
+
</ins><span class="cx"> 2015-02-02 Enrica Casucci <enrica@apple.com>
</span><span class="cx">
</span><span class="cx"> Additional emoji support.
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingEditorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/Editor.cpp (179568 => 179569)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -2418,7 +2418,7 @@
</span><span class="cx"> const bool shouldMarkSpelling = textCheckingOptions & TextCheckingTypeSpelling;
</span><span class="cx"> const bool shouldMarkGrammar = textCheckingOptions & TextCheckingTypeGrammar;
</span><span class="cx"> const bool shouldMarkLink = textCheckingOptions & TextCheckingTypeLink;
</span><del>- const bool shouldPerformReplacement = textCheckingOptions & TextCheckingTypeReplacement;
</del><ins>+ const bool shouldPerformReplacement = textCheckingOptions & (TextCheckingTypeQuote | TextCheckingTypeDash | TextCheckingTypeReplacement);
</ins><span class="cx"> const bool shouldShowCorrectionPanel = textCheckingOptions & TextCheckingTypeShowCorrectionPanel;
</span><span class="cx"> const bool shouldCheckForCorrection = shouldShowCorrectionPanel || (textCheckingOptions & TextCheckingTypeCorrection);
</span><span class="cx"> #if !USE(AUTOCORRECTION_PANEL)
</span></span></pre>
</div>
</div>
</body>
</html>