Bug 1339331 TextEventDispatcher should replace \r in composition string with \n and TextComposition should allow to input \n with composition events r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 16 Mar 2017 16:26:43 +0900
changeset 499819 d57b3f876d3f90a8785fb631527fcb34108734a8
parent 499691 5c137c718208dadc5a5c29ce512c4ebe959e5ad6
child 499868 85d25bc1aa612837b34521b85269edc6a213d4a0
push id49544
push usermasayuki@d-toybox.com
push dateThu, 16 Mar 2017 07:28:15 +0000
reviewersm_kato
bugs1339331
milestone55.0a1
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);
 }