Merge "Ignore spans that are not the right size."

java/src/com/android/inputmethod/latin/LatinIME.java

@@ -43,7 +43,6 @@ import android.os.Message;
 import android.os.SystemClock;
 import android.preference.PreferenceManager;
 import android.text.InputType;
-import android.text.Spanned;
 import android.text.TextUtils;
 import android.text.style.SuggestionSpan;
 import android.util.Log;
@@ -2523,15 +2522,10 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
         final int numberOfCharsInWordBeforeCursor = range.getNumberOfCharsInWordBeforeCursor();
         if (numberOfCharsInWordBeforeCursor > mLastSelectionStart) return;
         final ArrayList<SuggestedWordInfo> suggestions = CollectionUtils.newArrayList();
-        final CharSequence word = range.mWord;
+        final String typedWord = range.mWord.toString();
-        final String typedWord = word.toString();
-        if (word instanceof Spanned) {
-            final Spanned spanned = (Spanned)word;
         int i = 0;
-            for (Object object : spanned.getSpans(0, spanned.length(),
+        for (final SuggestionSpan span : range.getSuggestionSpansAtWord()) {
-                    SuggestionSpan.class)) {
+            for (final String s : span.getSuggestions()) {
-                SuggestionSpan span = (SuggestionSpan)object;
-                for (String s : span.getSuggestions()) {
                 ++i;
                 if (!TextUtils.equals(s, typedWord)) {
                     suggestions.add(new SuggestedWordInfo(s,
@@ -2540,7 +2534,6 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
                 }
             }
         }
-        }
         mWordComposer.setComposingWord(typedWord, mKeyboardSwitcher.getKeyboard());
         mWordComposer.setCursorPositionWithinWord(numberOfCharsInWordBeforeCursor);
         mConnection.setComposingRegion(

java/src/com/android/inputmethod/latin/RichInputConnection.java

@@ -17,7 +17,9 @@
 package com.android.inputmethod.latin;
 
 import android.inputmethodservice.InputMethodService;
+import android.text.Spanned;
 import android.text.TextUtils;
+import android.text.style.SuggestionSpan;
 import android.util.Log;
 import android.view.KeyEvent;
 import android.view.inputmethod.CompletionInfo;
@@ -32,6 +34,7 @@ import com.android.inputmethod.latin.utils.DebugLogUtils;
 import com.android.inputmethod.latin.utils.StringUtils;
 import com.android.inputmethod.research.ResearchLogger;
 
+import java.util.Arrays;
 import java.util.Locale;
 import java.util.regex.Pattern;
 
@@ -457,6 +460,66 @@ public final class RichInputConnection {
             return mWordAtCursorEndIndex - mCursorIndex;
         }
 
+        /**
+         * Gets the suggestion spans that are put squarely on the word, with the exact start
+         * and end of the span matching the boundaries of the word.
+         * @return the list of spans.
+         */
+        public SuggestionSpan[] getSuggestionSpansAtWord() {
+            if (!(mTextAtCursor instanceof Spanned && mWord instanceof Spanned)) {
+                return new SuggestionSpan[0];
+            }
+            final Spanned text = (Spanned)mTextAtCursor;
+            // Note: it's fine to pass indices negative or greater than the length of the string
+            // to the #getSpans() method. The reason we need to get from -1 to +1 is that, the
+            // spans were cut at the cursor position, and #getSpans(start, end) does not return
+            // spans that end at `start' or begin at `end'. Consider the following case:
+            //              this| is          (The | symbolizes the cursor position
+            //              ---- ---
+            // In this case, the cursor is in position 4, so the 0~7 span has been split into
+            // a 0~4 part and a 4~7 part.
+            // If we called #getSpans(0, 4) in this case, we would only get the part from 0 to 4
+            // of the span, and not the part from 4 to 7, so we would not realize the span actually
+            // extends from 0 to 7. But if we call #getSpans(-1, 5) we'll get both the 0~4 and
+            // the 4~7 spans and we can merge them accordingly.
+            // Any span starting more than 1 char away from the word boundaries in any direction
+            // does not touch the word, so we don't need to consider it. That's why requesting
+            // -1 ~ +1 is enough.
+            // Of course this is only relevant if the cursor is at one end of the word. If it's
+            // in the middle, the -1 and +1 are not necessary, but they are harmless.
+            final SuggestionSpan[] spans = text.getSpans(mWordAtCursorStartIndex - 1,
+                    mWordAtCursorEndIndex + 1, SuggestionSpan.class);
+            int readIndex = 0;
+            int writeIndex = 0;
+            for (; readIndex < spans.length; ++readIndex) {
+                final SuggestionSpan span = spans[readIndex];
+                // The span may be null, as we null them when we find duplicates. Cf a few lines
+                // down.
+                if (null == span) continue;
+                // Tentative span start and end. This may be modified later if we realize the
+                // same span is also applied to other parts of the string.
+                int spanStart = text.getSpanStart(span);
+                int spanEnd = text.getSpanEnd(span);
+                for (int i = readIndex + 1; i < spans.length; ++i) {
+                    if (span.equals(spans[i])) {
+                        // We found the same span somewhere else. Read the new extent of this
+                        // span, and adjust our values accordingly.
+                        spanStart = Math.min(spanStart, text.getSpanStart(spans[i]));
+                        spanEnd = Math.max(spanEnd, text.getSpanEnd(spans[i]));
+                        // ...and mark the span as processed.
+                        spans[i] = null;
+                    }
+                }
+                if (spanStart == mWordAtCursorStartIndex && spanEnd == mWordAtCursorEndIndex) {
+                    // If the span does not start and stop here, we ignore it. It probably extends
+                    // past the start or end of the word, as happens in missing space correction
+                    // or EasyEditSpans put by voice input.
+                    spans[writeIndex++] = spans[readIndex];
+                }
+            }
+            return writeIndex == readIndex ? spans : Arrays.copyOfRange(spans, 0, writeIndex);
+        }
+
         public Range(final CharSequence textAtCursor, final int wordAtCursorStartIndex,
                 final int wordAtCursorEndIndex, final int cursorIndex) {
             if (wordAtCursorStartIndex < 0 || cursorIndex < wordAtCursorStartIndex

tests/src/com/android/inputmethod/latin/RichInputConnectionAndTextRangeTests.java

@@ -17,9 +17,14 @@
 package com.android.inputmethod.latin;
 
 import android.inputmethodservice.InputMethodService;
+import android.os.Parcel;
 import android.test.AndroidTestCase;
+import android.test.MoreAsserts;
 import android.test.suitebuilder.annotation.SmallTest;
+import android.text.SpannableString;
+import android.text.Spanned;
 import android.text.TextUtils;
+import android.text.style.SuggestionSpan;
 import android.view.inputmethod.ExtractedText;
 import android.view.inputmethod.ExtractedTextRequest;
 import android.view.inputmethod.InputConnection;
@@ -27,8 +32,10 @@ import android.view.inputmethod.InputConnectionWrapper;
 
 import com.android.inputmethod.latin.RichInputConnection.Range;
 
+import java.util.Locale;
+
 @SmallTest
-public class RichInputConnectionTests extends AndroidTestCase {
+public class RichInputConnectionAndTextRangeTests extends AndroidTestCase {
 
     // The following is meant to be a reasonable default for
     // the "word_separators" resource.
@@ -40,10 +47,30 @@ public class RichInputConnectionTests extends AndroidTestCase {
     }
 
     private class MockConnection extends InputConnectionWrapper {
-        final String mTextBefore;
+        final CharSequence mTextBefore;
-        final String mTextAfter;
+        final CharSequence mTextAfter;
         final ExtractedText mExtractedText;
 
+        public MockConnection(final CharSequence text, final int cursorPosition) {
+            super(null, false);
+            // Interaction of spans with Parcels is completely non-trivial, but in the actual case
+            // the CharSequences do go through Parcels because they go through IPC. There
+            // are some significant differences between the behavior of Spanned objects that
+            // have and that have not gone through parceling, so it's much easier to simulate
+            // the environment with Parcels than try to emulate things by hand.
+            final Parcel p = Parcel.obtain();
+            TextUtils.writeToParcel(text.subSequence(0, cursorPosition), p, 0 /* flags */);
+            TextUtils.writeToParcel(text.subSequence(cursorPosition, text.length()), p,
+                    0 /* flags */);
+            final byte[] marshalled = p.marshall();
+            p.unmarshall(marshalled, 0, marshalled.length);
+            p.setDataPosition(0);
+            mTextBefore = TextUtils.CHAR_SEQUENCE_CREATOR.createFromParcel(p);
+            mTextAfter = TextUtils.CHAR_SEQUENCE_CREATOR.createFromParcel(p);
+            mExtractedText = null;
+            p.recycle();
+        }
+
         public MockConnection(String textBefore, String textAfter, ExtractedText extractedText) {
             super(null, false);
             mTextBefore = textBefore;
@@ -191,4 +218,95 @@ public class RichInputConnectionTests extends AndroidTestCase {
         ic.endBatchEdit();
         assertTrue(TextUtils.equals("word", r.mWord));
     }
+
+    /**
+     * Test logic in getting the word range at the cursor.
+     */
+    public void testGetSuggestionSpansAtWord() {
+        helpTestGetSuggestionSpansAtWord(10);
+        helpTestGetSuggestionSpansAtWord(12);
+        helpTestGetSuggestionSpansAtWord(15);
+        helpTestGetSuggestionSpansAtWord(16);
+    }
+
+    private void helpTestGetSuggestionSpansAtWord(final int cursorPos) {
+        final MockInputMethodService mockInputMethodService = new MockInputMethodService();
+        final RichInputConnection ic = new RichInputConnection(mockInputMethodService);
+
+        final String[] SUGGESTIONS1 = { "swing", "strong" };
+        final String[] SUGGESTIONS2 = { "storing", "strung" };
+
+        // Test the usual case.
+        SpannableString text = new SpannableString("This is a string for test");
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS1, 0 /* flags */),
+                10 /* start */, 16 /* end */, 0 /* flags */);
+        mockInputMethodService.setInputConnection(new MockConnection(text, cursorPos));
+        Range r;
+        SuggestionSpan[] suggestions;
+
+        r = ic.getWordRangeAtCursor(" ", 0);
+        suggestions = r.getSuggestionSpansAtWord();
+        assertEquals(suggestions.length, 1);
+        MoreAsserts.assertEquals(suggestions[0].getSuggestions(), SUGGESTIONS1);
+
+        // Test the case with 2 suggestion spans in the same place.
+        text = new SpannableString("This is a string for test");
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS1, 0 /* flags */),
+                10 /* start */, 16 /* end */, 0 /* flags */);
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS2, 0 /* flags */),
+                10 /* start */, 16 /* end */, 0 /* flags */);
+        mockInputMethodService.setInputConnection(new MockConnection(text, cursorPos));
+        r = ic.getWordRangeAtCursor(" ", 0);
+        suggestions = r.getSuggestionSpansAtWord();
+        assertEquals(suggestions.length, 2);
+        MoreAsserts.assertEquals(suggestions[0].getSuggestions(), SUGGESTIONS1);
+        MoreAsserts.assertEquals(suggestions[1].getSuggestions(), SUGGESTIONS2);
+
+        // Test a case with overlapping spans, 2nd extending past the start of the word
+        text = new SpannableString("This is a string for test");
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS1, 0 /* flags */),
+                10 /* start */, 16 /* end */, 0 /* flags */);
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS2, 0 /* flags */),
+                5 /* start */, 16 /* end */, 0 /* flags */);
+        mockInputMethodService.setInputConnection(new MockConnection(text, cursorPos));
+        r = ic.getWordRangeAtCursor(" ", 0);
+        suggestions = r.getSuggestionSpansAtWord();
+        assertEquals(suggestions.length, 1);
+        MoreAsserts.assertEquals(suggestions[0].getSuggestions(), SUGGESTIONS1);
+
+        // Test a case with overlapping spans, 2nd extending past the end of the word
+        text = new SpannableString("This is a string for test");
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS1, 0 /* flags */),
+                10 /* start */, 16 /* end */, 0 /* flags */);
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS2, 0 /* flags */),
+                10 /* start */, 20 /* end */, 0 /* flags */);
+        mockInputMethodService.setInputConnection(new MockConnection(text, cursorPos));
+        r = ic.getWordRangeAtCursor(" ", 0);
+        suggestions = r.getSuggestionSpansAtWord();
+        assertEquals(suggestions.length, 1);
+        MoreAsserts.assertEquals(suggestions[0].getSuggestions(), SUGGESTIONS1);
+
+        // Test a case with overlapping spans, 2nd extending past both ends of the word
+        text = new SpannableString("This is a string for test");
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS1, 0 /* flags */),
+                10 /* start */, 16 /* end */, 0 /* flags */);
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS2, 0 /* flags */),
+                5 /* start */, 20 /* end */, 0 /* flags */);
+        mockInputMethodService.setInputConnection(new MockConnection(text, cursorPos));
+        r = ic.getWordRangeAtCursor(" ", 0);
+        suggestions = r.getSuggestionSpansAtWord();
+        assertEquals(suggestions.length, 1);
+        MoreAsserts.assertEquals(suggestions[0].getSuggestions(), SUGGESTIONS1);
+
+        // Test a case with overlapping spans, none right on the word
+        text = new SpannableString("This is a string for test");
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS1, 0 /* flags */),
+                5 /* start */, 16 /* end */, 0 /* flags */);
+        text.setSpan(new SuggestionSpan(Locale.ENGLISH, SUGGESTIONS2, 0 /* flags */),
+                5 /* start */, 20 /* end */, 0 /* flags */);
+        mockInputMethodService.setInputConnection(new MockConnection(text, cursorPos));
+        r = ic.getWordRangeAtCursor(" ", 0);
+        suggestions = r.getSuggestionSpansAtWord();
+        assertEquals(suggestions.length, 0);
+    }
 }