Bug 1528715 - Work around problems with forcing the autocomplete result's capitalisation. r=geckoview-reviewers,m_kato
authorJan Henning <jh+bugzilla@buttercookie.de>
Fri, 22 Feb 2019 13:11:32 +0000
changeset 518473 cb52999afb25d616f2e98cbd720737c370a01686
parent 518472 c57170b2bc4ffc8e24f8ee741fd78ef7aaabaf3c
child 518474 d73453c15f883b7b4e63719e65b50271aae533ff
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, m_kato
bugs1528715
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1528715 - Work around problems with forcing the autocomplete result's capitalisation. r=geckoview-reviewers,m_kato 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;
 }