Bug 1528715 - Work around problems with forcing the autocomplete result's capitalisation. r=geckoview-reviewers,m_kato a=lizzard
authorJan Henning <jh+bugzilla@buttercookie.de>
Fri, 22 Feb 2019 13:11:32 +0000
changeset 516179 ab901cb2baa5ef1983a18a2c91d4a9b78ea8c313
parent 516178 2c312025ce11e2d86c9c431ad84332de8b4816df
child 516180 fc92378333dc0669c98fc3d11a8ec2bb0b5119a1
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, m_kato, lizzard
bugs1528715
milestone66.0
Bug 1528715 - Work around problems with forcing the autocomplete result's capitalisation. r=geckoview-reviewers,m_kato a=lizzard Even though using text.replace() is supposed to keep the existing spans intact, in practice the composition spans still get lost, even when the replacement text is identical (because there was in fact no capitalisation difference between user-entered text and autocomplete result). One approach to fix this would be to manually save the composition spans and subsequently restore them afterwards, like we already do this a few lines further down below, in the other major code path through onAutocomplete(). However while trying to generalise that approach, the most natural approach for the caller to specify which spans it wanted to save was to pass a Predicate lambda to the state saving function, which for some reason led to strange "Didn't find class" errors. So instead, we just restart input for affected IMEs (i.e. Sony's keyboard) to get them back into sync with the new contents of the EditText. To avoid unnecessarily calling restartInput(), though, we only do this if the user-entered text (which at this stage is known to correspond to the auto- complete result when compared case-*insensitively*) actually differs from the autocomplete result. Differential Revision: https://phabricator.services.mozilla.com/D20487
mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarEditText.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/InputMethods.java
--- a/mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarEditText.java
+++ b/mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarEditText.java
@@ -316,19 +316,28 @@ public class ToolbarEditText extends Cus
             // the result is stale and we should wait for the another result to come in.
             final String userText = text.toString().substring(0, autoCompleteStart);
             if (!StringUtils.caseInsensitiveStartsWith(result, userText)) {
                 return;
             }
 
             beginSettingAutocomplete();
 
-            // Should we force capitalisation from result
-            if (pathStart != -1) {
+            // If we're autocompleting the path part of an URL, force using the autocomplete
+            // result's capitalisation.
+            // Because replacing the text can mess up the composition spans and confuse certain
+            // IMEs, requiring further workarounds afterwards, we only do this if we actually have
+            // to fix up the capitalisation.
+            if (pathStart != -1 &&
+                    !TextUtils.regionMatches(text, pathStart,
+                            result, pathStart, autoCompleteStart - pathStart)) {
                 text.replace(pathStart, autoCompleteStart, result, pathStart, autoCompleteStart);
+                if (InputMethods.needsRestartOnReplaceRemove(mContext)) {
+                    InputMethods.restartInput(mContext, this);
+                }
             }
 
             // Replace the existing autocomplete text with new one.
             // replace() preserves the autocomplete spans that we set before.
             text.replace(autoCompleteStart, textLength, result, autoCompleteStart, resultLength);
 
             // Reshow the cursor if there is no longer any autocomplete text.
             if (autoCompleteStart == resultLength) {
@@ -484,17 +493,17 @@ public class ToolbarEditText extends Cus
                     return false;
                 }
                 return super.commitText(text, newCursorPosition);
             }
 
             @Override
             public boolean setComposingText(final CharSequence text, final int newCursorPosition) {
                 if (removeAutocompleteOnComposing(text)) {
-                    if (InputMethods.needsRemoveAutocompleteHack(mContext)) {
+                    if (InputMethods.needsRestartOnReplaceRemove(mContext)) {
                         InputMethods.restartInput(mContext, ToolbarEditText.this);
                     }
                     return false;
                 }
                 return super.setComposingText(text, newCursorPosition);
             }
         };
     }
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/InputMethods.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/InputMethods.java
@@ -65,16 +65,16 @@ final public class InputMethods {
                (METHOD_ANDROID_LATINIME.equals(inputMethod) ||
                 METHOD_GOOGLE_LATINIME.equals(inputMethod));
     }
 
     public static boolean shouldCommitCharAsKey(String inputMethod) {
         return METHOD_HTC_TOUCH_INPUT.equals(inputMethod);
     }
 
-    public static boolean needsRemoveAutocompleteHack(Context context) {
+    public static boolean needsRestartOnReplaceRemove(Context context) {
         String inputMethod = getCurrentInputMethod(context);
         return METHOD_SONY.equals(inputMethod);
     }
 
     // TODO: Replace usages by definition in EditorInfoCompat once available (bug 1385726).
     public static final int IME_FLAG_NO_PERSONALIZED_LEARNING = 0x1000000;
 }