Bug 1043310 - AutoCompletion doesn't take capitalization from address book entry, can leave angle brackets characters >> in field, when loosing focus by clicking outside (not enter/tab). r=mak, a=sledru
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Sat, 10 Jan 2015 13:48:23 +0200
changeset 242846 1d406b3f20db
parent 242845 06bb4d89e2bf
child 242847 e076d58d5b10
push id4321
push userryanvm@gmail.com
push date2015-01-14 15:04 +0000
treeherdermozilla-beta@a78eb4dd84f0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak, sledru
bugs1043310
milestone36.0
Bug 1043310 - AutoCompletion doesn't take capitalization from address book entry, can leave angle brackets characters >> in field, when loosing focus by clicking outside (not enter/tab). r=mak, a=sledru The fix has two aspects: - for onblur we need to complete to the actual suggestion not to leave addresses uncapitalized or having " >> " as value - once we have a valid selection that should not be changed to default because EnterMatch is called again (which it is with blur if the value was confirmed by enter, or mouse selection already)
toolkit/components/autocomplete/nsAutoCompleteController.cpp
toolkit/content/tests/chrome/test_autocomplete4.xul
toolkit/content/widgets/autocomplete.xml
--- a/toolkit/components/autocomplete/nsAutoCompleteController.cpp
+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
@@ -495,26 +495,37 @@ nsAutoCompleteController::HandleKeyNavig
         if (NS_SUCCEEDED(GetResultValueAt(selectedIndex, false, value))) {
           input->SetTextValue(value);
           input->SelectTextRange(value.Length(), value.Length());
         }
       }
       else if (shouldComplete) {
         // We usually try to preserve the casing of what user has typed, but
         // if he wants to autocomplete, we will replace the value with the
-        // actual autocomplete result.
+        // actual autocomplete result. Note that the autocomplete input can also
+        // be showing e.g. "bar >> foo bar" if the search matched "bar", a
+        // word not at the start of the full value "foo bar".
         // The user wants explicitely to use that result, so this ensures
         // association of the result with the autocompleted text.
         nsAutoString value;
         nsAutoString inputValue;
         input->GetTextValue(inputValue);
-        if (NS_SUCCEEDED(GetDefaultCompleteValue(-1, false, value)) &&
-            value.Equals(inputValue, nsCaseInsensitiveStringComparator())) {
-          input->SetTextValue(value);
-          input->SelectTextRange(value.Length(), value.Length());
+        if (NS_SUCCEEDED(GetDefaultCompleteValue(-1, false, value))) {
+          nsAutoString suggestedValue;
+          int32_t pos = inputValue.Find(" >> ");
+          if (pos > 0) {
+            inputValue.Right(suggestedValue, inputValue.Length() - pos - 4);
+          } else {
+            suggestedValue = inputValue;
+          }
+
+          if (value.Equals(suggestedValue, nsCaseInsensitiveStringComparator())) {
+            input->SetTextValue(value);
+            input->SelectTextRange(value.Length(), value.Length());
+          }
         }
       }
       // Close the pop-up even if nothing was selected
       ClearSearchTimer();
       ClosePopup();
     }
     // Update last-searched string to the current input, since the input may
     // have changed.  Without this, subsequent backspaces look like text
@@ -1280,30 +1291,63 @@ nsAutoCompleteController::EnterMatch(boo
       // The user wants explicitely to use that result, so this ensures
       // association of the result with the autocompleted text.
       nsAutoString defaultIndexValue;
       if (NS_SUCCEEDED(GetFinalDefaultCompleteValue(defaultIndexValue)))
         value = defaultIndexValue;
     }
 
     if (forceComplete && value.IsEmpty()) {
-      // Since nothing was selected, and forceComplete is specified, that means
-      // we have to find the first default match and enter it instead
+      // See if inputValue is one of the autocomplete results. It can be an
+      // identical value, or if it matched the middle of a result it can be
+      // something like "bar >> foobar" (user entered bar and foobar is
+      // the result value).
+      // If the current search matches one of the autocomplete results, we
+      // should use that result, and not overwrite it with the default value.
+      // It's indeed possible EnterMatch gets called a second time (for example
+      // by the blur handler) and it should not overwrite the current match.
+      nsAutoString inputValue;
+      input->GetTextValue(inputValue);
+      nsAutoString suggestedValue;
+      int32_t pos = inputValue.Find(" >> ");
+      if (pos > 0) {
+        inputValue.Right(suggestedValue, inputValue.Length() - pos - 4);
+      } else {
+        suggestedValue = inputValue;
+      }
+
+      nsAutoString defaultValue;
       for (uint32_t i = 0; i < mResults.Length(); ++i) {
         nsIAutoCompleteResult *result = mResults[i];
+        if (result) {
+          if (defaultValue.IsEmpty()) {
+            int32_t defaultIndex;
+            result->GetDefaultIndex(&defaultIndex);
+            if (defaultIndex >= 0) {
+              result->GetFinalCompleteValueAt(defaultIndex, defaultValue);
+            }
+          }
 
-        if (result) {
-          int32_t defaultIndex;
-          result->GetDefaultIndex(&defaultIndex);
-          if (defaultIndex >= 0) {
-            result->GetFinalCompleteValueAt(defaultIndex, value);
-            break;
+          uint32_t matchCount = 0;
+          result->GetMatchCount(&matchCount);
+          for (uint32_t j = 0; j < matchCount; ++j) {
+            nsAutoString matchValue;
+            result->GetFinalCompleteValueAt(j, matchValue);
+            if (suggestedValue.Equals(matchValue, nsCaseInsensitiveStringComparator())) {
+              value = matchValue;
+              break;
+            }
           }
         }
       }
+      if (value.IsEmpty()) {
+        // Since nothing was selected, and forceComplete is specified, that means
+        // we have to enter the first default match instead.
+        value = defaultValue;
+      }
     }
   }
 
   nsCOMPtr<nsIObserverService> obsSvc =
     mozilla::services::GetObserverService();
   NS_ENSURE_STATE(obsSvc);
   obsSvc->NotifyObservers(input, "autocomplete-will-enter-text", nullptr);
 
--- a/toolkit/content/tests/chrome/test_autocomplete4.xul
+++ b/toolkit/content/tests/chrome/test_autocomplete4.xul
@@ -146,16 +146,24 @@ const tests = [
   },
   { desc: "TAB key should confirm suggestion when forcecomplete is set",
     key: "VK_TAB",
     removeSelection: false,
     forceComplete: true,
     result: "Result",
     start: 6, end: 6
   },
+
+  { desc: "RIGHT key complete from middle",
+    key: "VK_RIGHT",
+    forceComplete: true,
+    completeFromMiddle: true,
+    result: "Result",
+    start: 6, end: 6
+  },
 ];
 
 function nextTest() {
   if (!tests.length) {
     // No more tests to run, finish.
     setTimeout(function() {
       // Unregister the factory so that we don't get in the way of other tests
       componentManager.unregisterFactory(autoCompleteSimpleID, autoCompleteSimple);
@@ -172,30 +180,53 @@ function nextTest() {
   if (currentTest.key == "VK_HOME" && navigator.platform.indexOf("Mac") != -1)
     nextTest();
   else
     setTimeout(runCurrentTest, 0);
 }
 
 function runCurrentTest() {
   var autocomplete = $("autocomplete");
-
   autocomplete.focus();
 
-  synthesizeKey("r", {});
-  synthesizeKey("e", {});
+  if (!currentTest.completeFromMiddle) {
+    synthesizeKey("r", {});
+    synthesizeKey("e", {});
+  }
+  else {
+    synthesizeKey("l", {});
+    synthesizeKey("t", {});
+  }
 }
 
 function searchComplete() {
   var autocomplete = $("autocomplete");
+  autocomplete.setAttribute("forcecomplete", currentTest.forceComplete ? true : false);
+
+  if (currentTest.completeFromMiddle) {
+    if (!currentTest.forceComplete) {
+      synthesizeKey(currentTest.key, {});
+    }
+    else if (!/ >> /.test(autocomplete.value)) {
+      // At this point we should have a value like "lt >> Result" showing.
+      throw new Error("Expected an middle-completed value, got " + autocomplete.value);
+    }
+
+    // For forceComplete a blur should cause a value from the results to get
+    // completed to. E.g. "lt >> Result" will turn into "Result".
+    if (currentTest.forceComplete)
+      autocomplete.blur();
+
+    checkResult();
+    return;
+  }
+
   is(autocomplete.value, "result",
      "Test '" + currentTest.desc + "': autocomplete.value should equal 'result'");
 
-  autocomplete.setAttribute("forcecomplete", currentTest.forceComplete ? true : false);
-
   if (autocomplete.selectionStart == 2) {  // Finished inserting "re" string.
     if (currentTest.removeSelection) {
       // remove current selection
       synthesizeKey("VK_DELETE", {});
     }
 
     synthesizeKey(currentTest.key, {});
 
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -331,22 +331,18 @@
       <method name="attachController">
         <body><![CDATA[
           this.mController.input = this;
         ]]></body>
       </method>
 
       <method name="detachController">
         <body><![CDATA[
-          try {
-            if  (this.mController.input == this)
+          if (this.mController.input == this)
             this.mController.input = null;
-          } catch (ex) {
-            // nothing really to do.
-          }
         ]]></body>
       </method>
 
       <!-- ::::::::::::: popup opening ::::::::::::: -->
 
       <method name="openPopup">
         <body><![CDATA[
           this.popup.openAutocompletePopup(this, this);
@@ -589,18 +585,23 @@
                action="if (this.mController.input == this) this.mController.handleStartComposition();"/>
 
       <handler event="compositionend" phase="capturing"
                action="if (this.mController.input == this) this.mController.handleEndComposition();"/>
 
       <handler event="focus" phase="capturing"
                action="this.attachController();"/>
 
-      <handler event="blur" phase="capturing"
-               action="if (!this._dontBlur) this.detachController();"/>
+      <handler event="blur" phase="capturing"><![CDATA[
+        if (!this._dontBlur) {
+          if (this.forceComplete && this.mController.matchCount >= 1)
+            this.mController.handleEnter(false);
+          this.detachController();
+        }
+      ]]></handler>
     </handlers>
   </binding>
 
   <binding id="autocomplete-result-popup" extends="chrome://global/content/bindings/autocomplete.xml#autocomplete-base-popup">
     <resources>
       <stylesheet src="chrome://global/content/autocomplete.css"/>
       <stylesheet src="chrome://global/skin/tree.css"/>
       <stylesheet src="chrome://global/skin/autocomplete.css"/>