Bug 1773802 - Ignore empty strings when spellchecking; r=smaug a=pascalc
authorDan Minor <dminor@mozilla.com>
Thu, 23 Jun 2022 15:34:20 +0000 (2022-06-23)
changeset 694687 d1fc5f75c5010b6142502c041e8f9c31f238a679
parent 694686 83b3d07ae63139b55e15cd84ef719850209a30c2
child 694688 de4b32c5baa61e99426273377885409c1d064e91
push id2886
push userpchevrel@mozilla.com
push dateMon, 04 Jul 2022 12:42:47 +0000 (2022-07-04)
treeherdermozilla-release@9d5ec3b156bd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, pascalc
bugs1773802
milestone102.0.1
Bug 1773802 - Ignore empty strings when spellchecking; r=smaug a=pascalc This adds a check to see if the encoded word is not empty and does not start with the null character. Hunspell accepts C-style strings and marks the empty string as correctly spelled. This prevents other dictionaries from detecting misspelled strings in languages that use a different charset, which has lead to problems when using the en-US dictionary packaged with Firefox and Greek, Hebrew and Russian dictionaries, where misspellings are not detected in the non-English language. Differential Revision: https://phabricator.services.mozilla.com/D149899
editor/spellchecker/tests/mochitest.ini
editor/spellchecker/tests/ru-RU/ru_RU.aff
editor/spellchecker/tests/ru-RU/ru_RU.dic
editor/spellchecker/tests/test_bug1773802.html
extensions/spellcheck/hunspell/glue/mozHunspell.cpp
--- a/editor/spellchecker/tests/mochitest.ini
+++ b/editor/spellchecker/tests/mochitest.ini
@@ -12,16 +12,18 @@ support-files =
   bug1204147_subframe2.html
   multiple_content_languages_subframe.html
   en-GB/en_GB.dic
   en-GB/en_GB.aff
   en-AU/en_AU.dic
   en-AU/en_AU.aff
   de-DE/de_DE.dic
   de-DE/de_DE.aff
+  ru-RU/ru_RU.dic
+  ru-RU/ru_RU.aff
   spellcheck.js
 
 [test_async_UpdateCurrentDictionary.html]
 [test_bug366682.html]
 [test_bug432225.html]
 [test_bug484181.html]
 [test_bug596333.html]
 [test_bug636465.html]
@@ -38,12 +40,13 @@ support-files =
 skip-if = e10s
 [test_bug1365383.html]
 [test_bug1368544.html]
 [test_bug1402822.html]
 [test_bug1418629.html]
 [test_bug1497480.html]
 [test_bug1602526.html]
 [test_bug1761273.html]
+[test_bug1773802.html]
 [test_multiple_content_languages.html]
 [test_spellcheck_after_edit.html]
 [test_spellcheck_after_pressing_navigation_key.html]
 [test_suggest.html]
new file mode 100644
--- /dev/null
+++ b/editor/spellchecker/tests/ru-RU/ru_RU.aff
@@ -0,0 +1,1 @@
+SET KOI8-R
new file mode 100644
--- /dev/null
+++ b/editor/spellchecker/tests/ru-RU/ru_RU.dic
@@ -0,0 +1,2 @@
+1
+����������
new file mode 100644
--- /dev/null
+++ b/editor/spellchecker/tests/test_bug1773802.html
@@ -0,0 +1,96 @@
+<!DOCTYPE html>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1773802
+-->
+<head>
+  <title>Test for Bug 1773802</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" href="/tests/SimpleTest/test.css">
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1773802">Mozilla Bug 1773802</a>
+<p id="display"></p>
+</div>
+
+<textarea id="editor">correct правильный, incarrect непровильный</textarea>
+
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+const Ci = SpecialPowers.Ci;
+
+let { maybeOnSpellCheck } = SpecialPowers.ChromeUtils.import(
+  "resource://testing-common/AsyncSpellCheckTestHelper.jsm"
+);
+
+function getMisspelledWords(editor) {
+  return editor.selectionController.getSelection(Ci.nsISelectionController.SELECTION_SPELLCHECK).toString();
+}
+
+/** Test for Bug 1773802 **/
+SimpleTest.waitForExplicitFinish();
+addLoadEvent(start);
+async function start() {
+  /* global actorParent */
+  /* eslint-env mozilla/frame-script */
+  let script = SpecialPowers.loadChromeScript(() => {
+    // eslint-disable-next-line mozilla/use-services
+    let dir = Cc["@mozilla.org/file/directory_service;1"]
+                .getService(Ci.nsIProperties)
+                .get("CurWorkD", Ci.nsIFile);
+    dir.append("tests");
+    dir.append("editor");
+    dir.append("spellchecker");
+    dir.append("tests");
+
+    let hunspell = Cc["@mozilla.org/spellchecker/engine;1"]
+                     .getService(Ci.mozISpellCheckingEngine);
+
+    // Install ru-RU dictionary.
+    let ru_RU = dir.clone();
+    ru_RU.append("ru-RU");
+    hunspell.addDirectory(ru_RU);
+
+    addMessageListener("destroy", () => hunspell.removeDirectory(ru_RU));
+    addMessageListener("ru_RU-exists", () => ru_RU.exists());
+  });
+  is(await script.sendQuery("ru_RU-exists"), true,
+     "true expected (ru_RU directory should exist)");
+
+  let textarea = document.getElementById("editor");
+  let editor = SpecialPowers.wrap(textarea).editor;
+  textarea.focus();
+
+  maybeOnSpellCheck(textarea, () => {
+    let isc = SpecialPowers.wrap(textarea).editor.getInlineSpellChecker(false);
+    ok(isc, "Inline spell checker should exist after focus and spell check");
+    let spellchecker = isc.spellChecker;
+
+    spellchecker.setCurrentDictionaries(["en-US", "ru-RU"]).then(async () => {
+      textarea.blur();
+      textarea.focus();
+      maybeOnSpellCheck(textarea, async function() {
+        let currentDictionaries = spellchecker.getCurrentDictionaries();
+
+        is(currentDictionaries.length, 2, "expected two dictionaries");
+        is(currentDictionaries[0], "en-US", "expected en-US");
+        is(currentDictionaries[1], "ru-RU", "expected ru-RU");
+        is(getMisspelledWords(editor), "incarrectнепровильный", "some misspelled words expected: incarrect непровильный");
+
+        // Remove the fake ru_RU dictionary again.
+        await script.sendQuery("destroy");
+
+        // This will clear the content preferences and reset "spellchecker.dictionary".
+        spellchecker.setCurrentDictionaries([]).then(() => {
+          SimpleTest.finish();
+        });
+      });
+    });
+  });
+}
+</script>
+</pre>
+</body>
+</html>
--- a/extensions/spellcheck/hunspell/glue/mozHunspell.cpp
+++ b/extensions/spellcheck/hunspell/glue/mozHunspell.cpp
@@ -448,29 +448,41 @@ mozHunspell::Check(const nsAString& aWor
   if (NS_WARN_IF(!aResult)) {
     return NS_ERROR_INVALID_ARG;
   }
 
   if (NS_WARN_IF(mHunspells.IsEmpty())) {
     return NS_ERROR_FAILURE;
   }
 
+  *aResult = true;
   for (auto iter = mHunspells.Iter(); !iter.Done(); iter.Next()) {
     if (!iter.Data().mEnabled) {
       continue;
     }
 
     nsresult rv = iter.Data().LoadIfNecessary();
     if (NS_FAILED(rv)) {
       continue;
     }
 
     std::string charsetWord;
     rv = iter.Data().ConvertCharset(aWord, charsetWord);
-    NS_ENSURE_SUCCESS(rv, rv);
+    if (NS_FAILED(rv)) {
+      continue;
+    }
+
+    // Depending upon the encoding, we might end up with a string that begins
+    // with the null byte. Since the hunspell interface uses C-style strings,
+    // this appears like an empty string, and hunspell marks empty strings as
+    // spelled correctly. Skip these cases to allow another dictionary to have
+    // the chance to spellcheck them.
+    if (charsetWord.empty() || charsetWord[0] == 0) {
+      continue;
+    }
 
     *aResult = iter.Data().mHunspell->spell(charsetWord);
     if (*aResult) {
       break;
     }
   }
 
   if (!*aResult && mPersonalDictionary) {