Skip to content

Commit

Permalink
Bug 1339331 TextEventDispatcher should replace \r in composition stri…
Browse files Browse the repository at this point in the history 
…ng 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

--HG--
extra : rebase_source : 0644e5941a080583af8701546111fbf46ec646ec
  • Loading branch information
@masayuki-nakano
masayuki-nakano committed Mar 16, 2017
1 parent 0865aae commit c038f80
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 30 deletions.
2 changes: 1 addition & 1 deletion dom/events/TextComposition.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ RemoveControlCharactersFrom(nsAString& aStr, TextRangeArray* aRanges)
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;
Expand Down
7 changes: 5 additions & 2 deletions widget/TextEventDispatcher.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,11 @@ TextEventDispatcher::CommitComposition(nsEventStatus& aStatus,
}
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))) {
Expand Down Expand Up @@ -708,8 +710,9 @@ TextEventDispatcher::PendingComposition::ReplaceNativeLineBreakers()
}

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.
Expand Down
88 changes: 61 additions & 27 deletions widget/tests/window_composition_text_querycontent.xul
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6006,8 +6006,6 @@ function runNativeLineBreakerTest()
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" ];
Expand All @@ -6030,18 +6028,18 @@ function runNativeLineBreakerTest()
"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();
Expand All @@ -6066,18 +6064,54 @@ function runNativeLineBreakerTest()
"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);
Expand Down Expand Up @@ -6106,7 +6140,7 @@ function runControlCharTest()
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";
Expand All @@ -6130,7 +6164,7 @@ function runControlCharTest()
"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");
Expand Down Expand Up @@ -6162,14 +6196,14 @@ function runControlCharTest()
"caret": { "start": DIndex, "length": 0 }
});
checkSelection(DIndex - 1 + kLFLen, "", "runControlCharTest", "#3")
checkSelection(DIndex + (kLFLen - 1) * 2, "", "runControlCharTest", "#3")
is(result.compositionupdate, data, "runControlCharTest: control characters in event.data should not be removed in compositionupdate event #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");
Expand Down

0 comments on commit c038f80

Please sign in to comment.