Merge pull request #24373 from comoyo/bug1053647

Bug 1053647 - delete space when reverting an autocorrection and don't insert space when user selects a word suggestion. r=janjongboom

apps/keyboard/js/imes/latin/latin.js

@@ -84,7 +84,7 @@
   var autoCorrection;     // Correction to make if next input is space
   var revertTo;           // Revert to this on backspace after autocorrect
   var revertFrom;         // Revert away from this on backspace
-  var justAutoCorrected;  // Was last change an auto correction?
+  var disableOnRevert;    // Do we disable auto correction when reverting?
   var correctionDisabled; // Temporarily diabled after reverting?
 
   // Terminate the worker when the keyboard is inactive for this long.
@@ -208,7 +208,7 @@
     lastSpaceTimestamp = 0;
     autoCorrection = null;
     revertTo = revertFrom = '';
-    justAutoCorrected = false;
+    disableOnRevert = false;
     correctionDisabled = false;
 
     // The keyboard isn't idle anymore, so clear the timer
@@ -368,7 +368,7 @@
       // previous changes that we would otherwise revert.
       if (keyCode !== BACKSPACE) {
         revertTo = revertFrom = '';
-        justAutoCorrected = false;
+        disableOnRevert = false;
       }
 
       var handler;
@@ -511,12 +511,12 @@
       return replaceBeforeCursor(revertFrom, revertTo).then(function() {
         // If the change we just reverted was an auto-correction then
         // temporarily disable auto correction until the next space
-        if (justAutoCorrected) {
+        if (disableOnRevert) {
           correctionDisabled = true;
         }
 
         revertFrom = revertTo = '';
-        justAutoCorrected = false;
+        disableOnRevert = false;
       });
     }
     else {
@@ -559,11 +559,10 @@
       // user types backspace
       revertTo = currentWord;
       revertFrom = newWord;
-      justAutoCorrected = true;
+      disableOnRevert = true;
     }).then(function() {
-      // Send the keycode as seperate key event because it may get canceled
+      // Send the keycode as separate key event because it may get canceled
       return handleKey(keycode).then(function() {
-        revertTo += String.fromCharCode(keycode);
         revertFrom += String.fromCharCode(keycode);
       });
     });
@@ -616,7 +615,7 @@
           // Remember this change so we can revert it on backspace
           revertTo = ' ' + String.fromCharCode(revertToKeycode || keycode);
           revertFrom = newtext;
-          justAutoCorrected = false;
+          disableOnRevert = false;
         });
     }
   }
@@ -713,33 +712,42 @@
     keyboard.sendCandidates(words);
   }
 
+  //
   // If the user selects one of the suggestions offered by this input method
   // the keyboard calls this method to tell us it has been selected.
-  // We have to backspace over the current word, insert this new word, and
+  // We have to delete the current word, insert this new word, and
   // update our internal state to match.
   //   word: the text displayed as the suggestion, might contain ellipsis
   //   data: the actual data we need to output
+  // In the past we would automatically insert a space after the selected
+  // word, but that, combined with the revert-on-backspace behavior made
+  // it impossible to add a suffix to the selected word.
+  //
   function select(word, data) {
     var oldWord = wordBeforeCursor();
+    var newWord = data;
 
-    // Replace the current word with the selected suggestion plus space
-    var newWord = data += ' ';
+    // The user has selected a suggestion, so we don't need to display
+    // them anymore. Note that calling this function resets all the
+    // autocorrect state. We'll reset much of that state below after
+    // the word has been replace with the new one.
+    dismissSuggestions();
 
     return replaceBeforeCursor(oldWord, newWord).then(function() {
       // Remember the change we just made so we can revert it if the
-      // next key is a backspace. Note that it is not an autocorrection
-      // so we don't need to disable corrections.
+      // next key is a backspace. If the word is reverted we disable
+      // autocorrection for this word.
       revertFrom = newWord;
       revertTo = oldWord;
-      justAutoCorrected = false;
-
-      // We inserted a space after the selected word, so we're beginning
-      // a new word here, which means that if auto-correction was disabled
-      // we can re-enable it now.
-      correctionDisabled = false;
 
-      // Clear the suggestions
-      keyboard.sendCandidates([]);
+      // Given that the user has selected this word, we don't ever
+      // want to autocorrect the word because if they keep typing to
+      // add a suffix to the word, we don't want to modify the
+      // original. This also means that if they revert the selection
+      // autocorrect will still be disabled which is what we want.
+      // Note, however, that most often the user will type a space
+      // after this selection and autocorrect will be enabled again.
+      correctionDisabled = true;
 
       // And update the keyboard capitalization state, if necessary
       updateCapitalization();
@@ -755,7 +763,7 @@
     lastSpaceTimestamp = 0;
     autoCorrection = null;
     revertTo = revertFrom = '';
-    justAutoCorrected = false;
+    disableOnRevert = false;
     correctionDisabled = false;
   }
 

tests/python/gaia-ui-tests/gaiatest/apps/keyboard/app.py

@@ -338,6 +338,15 @@ def tap_first_predictive_word(self):
         self.wait_for_element_displayed(*self._predicted_word_locator)
         self.marionette.find_element(*self._predicted_word_locator).tap()
         self.apps.switch_to_displayed_app()
+
+    def tap_suggestion(self, word):
+        self.switch_to_keyboard()
+
+        # find the requested suggestion
+        selector = (By.CSS_SELECTOR, ".suggestions-container span[data-data=\"%s\"]" %(word))
+        self.wait_for_element_displayed(*selector)
+        self.marionette.find_element(*selector).tap()
+        self.apps.switch_to_displayed_app()
 
     # Accessibility related properties and methods
 

tests/python/gaia-ui-tests/gaiatest/tests/accessibility/keyboard/test_a11y_keyboard_word_suggestions.py

@@ -26,7 +26,7 @@ def test_a11y_keyboard_word_suggestions(self):
         keyboard = keyboard_page.tap_text_input()
 
         # type first 7 letters of the expected word
-        expected_word = 'keyboard '
+        expected_word = 'keyboard'
         keyboard.send(expected_word[:7])
 
         keyboard.switch_to_keyboard()

tests/python/gaia-ui-tests/gaiatest/tests/functional/keyboard/test_keyboard_predictive_key.py

@@ -26,7 +26,7 @@ def test_keyboard_predictive_key(self):
         keyboard = keyboard_page.tap_text_input()
 
         # type first 7 letters of the expected word
-        expected_word = 'keyboard '
+        expected_word = 'keyboard'
         keyboard.send(expected_word[:7])
 
         # tap the first predictive word
@@ -37,3 +37,37 @@ def test_keyboard_predictive_key(self):
         # check if the word in the input field is the same as the expected word
         typed_word = keyboard_page.text_input
         self.assertEqual(typed_word, expected_word)
+
+        ## TEST 2, tap second suggestion, then press space
+        keyboard.send(' ')
+        keyboard_page.switch_to_frame()
+
+        # type some misspelled word
+        keyboard.send('Tes')
+        keyboard_page.switch_to_frame()
+        self.assertEqual(keyboard_page.text_input, 'keyboard Tes')
+
+        # tap second predictive word (tea)
+        keyboard.tap_suggestion('Tea')
+        self.apps.switch_to_displayed_app()
+
+        # Send space
+        keyboard.send(' ')
+        keyboard_page.switch_to_frame()
+
+        # Output should be 'Tea '
+        self.assertEqual(keyboard_page.text_input, 'keyboard Tea ')
+
+        ## TEST 3 - type something with autocorrect and press space
+        keyboard.send('ye ')
+        keyboard_page.switch_to_frame()
+        self.assertEqual(keyboard_page.text_input, 'keyboard Tea yes ')
+
+        # TEST 4 - autocorrect, dot and backspace
+        keyboard.send('wot.')
+        keyboard_page.switch_to_frame()
+        self.assertEqual(keyboard_page.text_input, 'keyboard Tea yes wit.')
+
+        keyboard.tap_backspace()
+        keyboard_page.switch_to_frame()
+        self.assertEqual(keyboard_page.text_input, 'keyboard Tea yes wot')