Bug 1339331 TextEventDispatcher should replace \r in composition string with \n and TextComposition should allow to input \n with composition events r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 16 Mar 2017 16:26:43 +0900
changeset 398554 8e72178c3893c377972209cccd2e561e1ec06c7d
parent 398553 cacda47d7c8186a94f6db272713a36baaf67bd44
child 398555 4503ff6c5820e361c8acd497f7b93775f0a6dd74
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1339331
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1339331 TextEventDispatcher should replace \r in composition string with \n and TextComposition should allow to input \n with composition events r=m_kato According to ATOK's behavior, IME may send different line breaker from its platform's standard. Therefore, we should treat \r as \n too. Additionally, currently, TextComposition doesn't allow to input \n with composition. However, this was added for preventing to see odd control characters as boxes with code point. Therefore, we should allow \n for IMEs. (It was allowed, this limitation is unexpected when I reviewed the patch to reject control characters in TextComposition.) MozReview-Commit-ID: DzGSMgp89Av
dom/events/TextComposition.cpp
widget/TextEventDispatcher.cpp
widget/tests/window_composition_text_querycontent.xul
--- a/dom/events/TextComposition.cpp
+++ b/dom/events/TextComposition.cpp
@@ -220,17 +220,17 @@ RemoveControlCharactersFrom(nsAString& a
   if (NS_WARN_IF(!dest)) {
     return;
   }
 
   char16_t* curDest = dest + firstControlCharOffset;
   size_t i = firstControlCharOffset;
   for (const char16_t* source = sourceBegin + firstControlCharOffset;
        source < sourceEnd; ++source) {
-    if (*source == '\t' || !IsControlChar(*source)) {
+    if (*source == '\t' || *source == '\n' || !IsControlChar(*source)) {
       *curDest = *source;
       ++curDest;
       ++i;
     } else if (aRanges) {
       aRanges->RemoveCharacter(i);
     }
   }
 
--- a/widget/TextEventDispatcher.cpp
+++ b/widget/TextEventDispatcher.cpp
@@ -314,19 +314,21 @@ TextEventDispatcher::CommitComposition(n
                                          eCompositionCommitAsIs;
   WidgetCompositionEvent compositionCommitEvent(true, message, widget);
   InitEvent(compositionCommitEvent);
   if (aEventTime) {
     compositionCommitEvent.AssignEventTime(*aEventTime);
   }
   if (message == eCompositionCommit) {
     compositionCommitEvent.mData = *aCommitString;
-    // Don't send CRLF, replace it with LF here.
+    // Don't send CRLF nor CR, replace it with LF here.
     compositionCommitEvent.mData.ReplaceSubstring(NS_LITERAL_STRING("\r\n"),
                                                   NS_LITERAL_STRING("\n"));
+    compositionCommitEvent.mData.ReplaceSubstring(NS_LITERAL_STRING("\r"),
+                                                  NS_LITERAL_STRING("\n"));
   }
   rv = DispatchEvent(widget, compositionCommitEvent, aStatus);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   return NS_OK;
 }
@@ -703,18 +705,19 @@ TextEventDispatcher::PendingComposition:
   mReplacedNativeLineBreakers = true;
 
   // If the composition string is empty, we don't need to do anything.
   if (mString.IsEmpty()) {
     return;
   }
 
   nsAutoString nativeString(mString);
-  // Don't expose CRLF to web contents, instead, use LF.
+  // Don't expose CRLF nor CR to web contents, instead, use LF.
   mString.ReplaceSubstring(NS_LITERAL_STRING("\r\n"), NS_LITERAL_STRING("\n"));
+  mString.ReplaceSubstring(NS_LITERAL_STRING("\r"), NS_LITERAL_STRING("\n"));
 
   // If the length isn't changed, we don't need to adjust any offset and length
   // of mClauses nor mCaret.
   if (nativeString.Length() == mString.Length()) {
     return;
   }
 
   if (mClauses) {
--- a/widget/tests/window_composition_text_querycontent.xul
+++ b/widget/tests/window_composition_text_querycontent.xul
@@ -6001,18 +6001,16 @@ function runNativeLineBreakerTest()
   }
 
   SpecialPowers.setBoolPref("dom.compositionevent.allow_control_characters", false);
 
   textarea.addEventListener("compositionupdate", handler, true);
   textarea.addEventListener("compositionend", handler, true);
 
   // '\n' in composition string shouldn't be changed.
-  // XXX Currently, \n isn't accepted by TextComposition.  So,these test checks
-  //     without the length of \n (bug 1339331).
   clearResult();
   textarea.value = "";
   var clauses = [ "abc\n", "def\n\ng", "hi\n", "\njkl" ];
   var caret = clauses[0] + clauses[1] + clauses[2];
   synthesizeCompositionChange(
     { "composition":
       { "string": clauses.join(''),
         "clauses":
@@ -6025,28 +6023,28 @@ function runNativeLineBreakerTest()
             "attr": COMPOSITION_ATTR_CONVERTED_CLAUSE },
           { "length": clauses[3].length,
             "attr": COMPOSITION_ATTR_SELECTED_CLAUSE },
         ]
       },
       "caret": { "start": caret.length, "length": 0 }
     });
 
-  checkSelection(caret.replace(/\n/g, "").length, "", "runNativeLineBreakerTest", "#1");
-  checkIMESelection("RawClause", true, 0, clauses[0].replace(/\n/g, ""), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
-  checkIMESelection("SelectedRawClause", true, clauses[0].replace(/\n/g, "").length, clauses[1].replace(/\n/g, ""), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
-  checkIMESelection("ConvertedClause", true, (clauses[0] + clauses[1]).replace(/\n/g, "").length, clauses[2].replace(/\n/g, ""), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
-  checkIMESelection("SelectedClause", true, (clauses[0] + clauses[1] + clauses[2]).replace(/\n/g, "").length, clauses[3].replace(/\n/g, ""), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
-  is(result.compositionupdate, clauses.join('').replace(/\n/g, ""), "runNativeLineBreakerTest: \\n in compositionupdate.data shouldn't be removed nor replaced with other characters #1");
-  is(textarea.value, clauses.join('').replace(/\n/g, ""), "runNativeLineBreakerTest: \\n in textarea.value shouldn't be removed nor replaced with other characters #1");
+  checkSelection(caret.replace(/\n/g, "\n").length + (kLFLen - 1) * 4, "", "runNativeLineBreakerTest", "#1");
+  checkIMESelection("RawClause", true, 0, clauses[0].replace(/\n/g, kLF), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
+  checkIMESelection("SelectedRawClause", true, clauses[0].replace(/\n/g, kLF).length, clauses[1].replace(/\n/g, kLF), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
+  checkIMESelection("ConvertedClause", true, (clauses[0] + clauses[1]).replace(/\n/g, kLF).length, clauses[2].replace(/\n/g, kLF), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
+  checkIMESelection("SelectedClause", true, (clauses[0] + clauses[1] + clauses[2]).replace(/\n/g, kLF).length, clauses[3].replace(/\n/g, kLF), "runNativeLineBreakerTest: \\n shouldn't be replaced with any character #1");
+  is(result.compositionupdate, clauses.join('').replace(/\n/g, "\n"), "runNativeLineBreakerTest: \\n in compositionupdate.data shouldn't be removed nor replaced with other characters #1");
+  is(textarea.value, clauses.join('').replace(/\n/g, "\n"), "runNativeLineBreakerTest: \\n in textarea.value shouldn't be removed nor replaced with other characters #1");
 
   synthesizeComposition({ type: "compositioncommit", data: clauses.join('') });
-  checkSelection(clauses.join('').replace(/\n/g, "").length, "", "runNativeLineBreakerTest", "#2");
-  is(result.compositionend, clauses.join('').replace(/\n/g, ""), "runNativeLineBreakerTest: \\n in compositionend.data shouldn't be removed nor replaced with other characters #2");
-  is(textarea.value, clauses.join('').replace(/\n/g, ""), "runNativeLineBreakerTest: \\n in textarea.value shouldn't be removed nor replaced with other characters #2");
+  checkSelection(clauses.join('').replace(/\n/g, "\n").length + (kLFLen - 1) * 5, "", "runNativeLineBreakerTest", "#2");
+  is(result.compositionend, clauses.join('').replace(/\n/g, "\n"), "runNativeLineBreakerTest: \\n in compositionend.data shouldn't be removed nor replaced with other characters #2");
+  is(textarea.value, clauses.join('').replace(/\n/g, "\n"), "runNativeLineBreakerTest: \\n in textarea.value shouldn't be removed nor replaced with other characters #2");
 
   // \r\n in composition string should be replaced with \n.
   clearResult();
   textarea.value = "";
   clauses = [ "abc\r\n", "def\r\n\r\ng", "hi\r\n", "\r\njkl" ];
   caret = clauses[0] + clauses[1] + clauses[2];
   synthesizeCompositionChange(
     { "composition":
@@ -6061,28 +6059,64 @@ function runNativeLineBreakerTest()
             "attr": COMPOSITION_ATTR_CONVERTED_CLAUSE },
           { "length": clauses[3].length,
             "attr": COMPOSITION_ATTR_SELECTED_CLAUSE },
         ]
       },
       "caret": { "start": caret.length, "length": 0 }
     });
 
-  checkSelection(caret.replace(/\r\n/g, "").length, "", "runNativeLineBreakerTest", "#3");
-  checkIMESelection("RawClause", true, 0, clauses[0].replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
-  checkIMESelection("SelectedRawClause", true, clauses[0].replace(/\r\n/g, "").length, clauses[1].replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
-  checkIMESelection("ConvertedClause", true, (clauses[0] + clauses[1]).replace(/\r\n/g, "").length, clauses[2].replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
-  checkIMESelection("SelectedClause", true, (clauses[0] + clauses[1] + clauses[2]).replace(/\r\n/g, "").length, clauses[3].replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
-  is(result.compositionupdate, clauses.join('').replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n in compositionudpate.data should be replaced with \\n #3");
-  is(textarea.value, clauses.join('').replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n in textarea.value should be replaced with \\n #3");
+  checkSelection(caret.replace(/\r\n/g, "\n").length + (kLFLen - 1) * 4, "", "runNativeLineBreakerTest", "#3");
+  checkIMESelection("RawClause", true, 0, clauses[0].replace(/\r\n/g, kLF), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
+  checkIMESelection("SelectedRawClause", true, clauses[0].replace(/\r\n/g, kLF).length, clauses[1].replace(/\r\n/g, kLF), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
+  checkIMESelection("ConvertedClause", true, (clauses[0] + clauses[1]).replace(/\r\n/g, kLF).length, clauses[2].replace(/\r\n/g, kLF), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
+  checkIMESelection("SelectedClause", true, (clauses[0] + clauses[1] + clauses[2]).replace(/\r\n/g, kLF).length, clauses[3].replace(/\r\n/g, kLF), "runNativeLineBreakerTest: \\r\\n should be replaced with \\n #3");
+  is(result.compositionupdate, clauses.join('').replace(/\r\n/g, "\n"), "runNativeLineBreakerTest: \\r\\n in compositionudpate.data should be replaced with \\n #3");
+  is(textarea.value, clauses.join('').replace(/\r\n/g, "\n"), "runNativeLineBreakerTest: \\r\\n in textarea.value should be replaced with \\n #3");
 
   synthesizeComposition({ type: "compositioncommit", data: clauses.join('') });
-  checkSelection(clauses.join('').replace(/\r\n/g, "").length, "", "runNativeLineBreakerTest", "#4");
-  is(result.compositionend, clauses.join('').replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n in compositionend.data should be replaced with \\n #4");
-  is(textarea.value, clauses.join('').replace(/\r\n/g, ""), "runNativeLineBreakerTest: \\r\\n in textarea.value should be replaced with \\n #4");
+  checkSelection(clauses.join('').replace(/\r\n/g, "\n").length + (kLFLen - 1) * 5, "", "runNativeLineBreakerTest", "#4");
+  is(result.compositionend, clauses.join('').replace(/\r\n/g, "\n"), "runNativeLineBreakerTest: \\r\\n in compositionend.data should be replaced with \\n #4");
+  is(textarea.value, clauses.join('').replace(/\r\n/g, "\n"), "runNativeLineBreakerTest: \\r\\n in textarea.value should be replaced with \\n #4");
+
+  // \r (not followed by \n) in composition string should be replaced with \n.
+  clearResult();
+  textarea.value = "";
+  clauses = [ "abc\r", "def\r\rg", "hi\r", "\rjkl" ];
+  caret = clauses[0] + clauses[1] + clauses[2];
+  synthesizeCompositionChange(
+    { "composition":
+      { "string": clauses.join(''),
+        "clauses":
+        [
+          { "length": clauses[0].length,
+            "attr": COMPOSITION_ATTR_RAW_CLAUSE },
+          { "length": clauses[1].length,
+            "attr": COMPOSITION_ATTR_SELECTED_RAW_CLAUSE },
+          { "length": clauses[2].length,
+            "attr": COMPOSITION_ATTR_CONVERTED_CLAUSE },
+          { "length": clauses[3].length,
+            "attr": COMPOSITION_ATTR_SELECTED_CLAUSE },
+        ]
+      },
+      "caret": { "start": caret.length, "length": 0 }
+    });
+
+  checkSelection(caret.replace(/\r/g, "\n").length + (kLFLen - 1) * 4, "", "runNativeLineBreakerTest", "#5");
+  checkIMESelection("RawClause", true, 0, clauses[0].replace(/\r/g, kLF), "runNativeLineBreakerTest: \\r should be replaced with \\n #5");
+  checkIMESelection("SelectedRawClause", true, clauses[0].replace(/\r/g, kLF).length, clauses[1].replace(/\r/g, kLF), "runNativeLineBreakerTest: \\r should be replaced with \\n #5");
+  checkIMESelection("ConvertedClause", true, (clauses[0] + clauses[1]).replace(/\r/g, kLF).length, clauses[2].replace(/\r/g, kLF), "runNativeLineBreakerTest: \\r should be replaced with \\n #5");
+  checkIMESelection("SelectedClause", true, (clauses[0] + clauses[1] + clauses[2]).replace(/\r/g, kLF).length, clauses[3].replace(/\r/g, kLF), "runNativeLineBreakerTest: \\r should be replaced with \\n #5");
+  is(result.compositionupdate, clauses.join('').replace(/\r/g, "\n"), "runNativeLineBreakerTest: \\r in compositionupdate.data should be replaced with \\n #5");
+  is(textarea.value, clauses.join('').replace(/\r/g, "\n"), "runNativeLineBreakerTest: \\r in textarea.value should be replaced with \\n #5");
+
+  synthesizeComposition({ type: "compositioncommit", data: clauses.join('') });
+  checkSelection(clauses.join('').replace(/\r/g, "\n").length + (kLFLen - 1) * 5, "", "runNativeLineBreakerTest", "#6");
+  is(result.compositionend, clauses.join('').replace(/\r/g, "\n"), "runNativeLineBreakerTest: \\r in compositionend.data should be replaced with \\n #6");
+  is(textarea.value, clauses.join('').replace(/\r/g, "\n"), "runNativeLineBreakerTest: \\r in textarea.value should be replaced with \\n #6");
 
   textarea.removeEventListener("compositionupdate", handler, true);
   textarea.removeEventListener("compositionend", handler, true);
 
   SpecialPowers.clearUserPref("dom.compositionevent.allow_control_characters");
 }
 
 function runControlCharTest()
@@ -6101,17 +6135,17 @@ function runControlCharTest()
   }
 
   textarea.addEventListener("compositionupdate", handler, true);
   textarea.addEventListener("compositionend", handler, true);
 
   textarea.value = "";
 
   var controlChars = String.fromCharCode.apply(null, Object.keys(Array.from({length:0x20}))) + "\x7F";
-  var allowedChars = "\t";
+  var allowedChars = "\t\n\n";
 
   var data = "AB" + controlChars + "CD" + controlChars + "EF";
   var removedData = "AB" + allowedChars + "CD" + allowedChars + "EF";
 
   var DIndex = data.indexOf("D");
   var removedDIndex = removedData.indexOf("D");
 
   // input string contains control characters
@@ -6125,17 +6159,17 @@ function runControlCharTest()
             "attr": COMPOSITION_ATTR_SELECTED_CLAUSE },
           { "length": data.length - DIndex,
             "attr": COMPOSITION_ATTR_CONVERTED_CLAUSE }
         ]
       },
       "caret": { "start": DIndex, "length": 0 }
     });
 
-  checkSelection(removedDIndex, "", "runControlCharTest", "#1")
+  checkSelection(removedDIndex + (kLFLen - 1) * 2, "", "runControlCharTest", "#1")
 
   is(result.compositionupdate, removedData, "runControlCharTest: control characters in event.data should be removed in compositionupdate event #1");
   is(textarea.value, removedData, "runControlCharTest: control characters should not appear in textarea #1");
 
   synthesizeComposition({ type: "compositioncommit", data: data });
 
   is(result.compositionend, removedData, "runControlCharTest: control characters in event.data should be removed in compositionend event #2");
   is(textarea.value, removedData, "runControlCharTest: control characters should not appear in textarea #2");
@@ -6157,24 +6191,24 @@ function runControlCharTest()
             "attr": COMPOSITION_ATTR_SELECTED_CLAUSE },
           { "length": data.length - DIndex,
             "attr": COMPOSITION_ATTR_CONVERTED_CLAUSE }
         ]
       },
       "caret": { "start": DIndex, "length": 0 }
     });
 
-  checkSelection(DIndex - 1 + kLFLen, "", "runControlCharTest", "#3")
-
-  is(result.compositionupdate, data, "runControlCharTest: control characters in event.data should not be removed in compositionupdate event #3");
+  checkSelection(DIndex + (kLFLen - 1) * 2, "", "runControlCharTest", "#3")
+
+  is(result.compositionupdate, data.replace(/\r/g, "\n"), "runControlCharTest: control characters in event.data should not be removed in compositionupdate event #3");
   is(textarea.value, data.replace(/\r/g, "\n"), "runControlCharTest: control characters should appear in textarea #3");
 
   synthesizeComposition({ type: "compositioncommit", data: data });
 
-  is(result.compositionend, data, "runControlCharTest: control characters in event.data should not be removed in compositionend event #4");
+  is(result.compositionend, data.replace(/\r/g, "\n"), "runControlCharTest: control characters in event.data should not be removed in compositionend event #4");
   is(textarea.value, data.replace(/\r/g, "\n"), "runControlCharTest: control characters should appear in textarea #4");
 
   SpecialPowers.clearUserPref("dom.compositionevent.allow_control_characters");
 
   textarea.removeEventListener("compositionupdate", handler, true);
   textarea.removeEventListener("compositionend", handler, true);
 }