From 2f7f6257b66fc1ed19b600f3d55902fd0de2e338 Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Mon, 24 Jun 2013 19:43:03 +0900 Subject: [PATCH] Ignore spans that are not the right size. Bug: 8839763 Bug: 8862327 Change-Id: I5d49c86edcbc0fc1b2b147856437cfbadd678377 --- .../android/inputmethod/latin/LatinIME.java | 25 ++-- .../latin/RichInputConnection.java | 63 +++++++++ ...RichInputConnectionAndTextRangeTests.java} | 124 +++++++++++++++++- 3 files changed, 193 insertions(+), 19 deletions(-) rename tests/src/com/android/inputmethod/latin/{RichInputConnectionTests.java => RichInputConnectionAndTextRangeTests.java} (53%) diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index c867436e5..824752fcb 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/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; @@ -2522,21 +2521,15 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen final int numberOfCharsInWordBeforeCursor = range.getNumberOfCharsInWordBeforeCursor(); if (numberOfCharsInWordBeforeCursor > mLastSelectionStart) return; final ArrayList suggestions = CollectionUtils.newArrayList(); - final CharSequence word = range.mWord; - 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(), - SuggestionSpan.class)) { - SuggestionSpan span = (SuggestionSpan)object; - for (String s : span.getSuggestions()) { - ++i; - if (!TextUtils.equals(s, typedWord)) { - suggestions.add(new SuggestedWordInfo(s, - SuggestionStripView.MAX_SUGGESTIONS - i, - SuggestedWordInfo.KIND_RESUMED, Dictionary.TYPE_RESUMED)); - } + final String typedWord = range.mWord.toString(); + int i = 0; + for (final SuggestionSpan span : range.getSuggestionSpansAtWord()) { + for (final String s : span.getSuggestions()) { + ++i; + if (!TextUtils.equals(s, typedWord)) { + suggestions.add(new SuggestedWordInfo(s, + SuggestionStripView.MAX_SUGGESTIONS - i, + SuggestedWordInfo.KIND_RESUMED, Dictionary.TYPE_RESUMED)); } } } diff --git a/java/src/com/android/inputmethod/latin/RichInputConnection.java b/java/src/com/android/inputmethod/latin/RichInputConnection.java index 5391b1303..6b22cb12e 100644 --- a/java/src/com/android/inputmethod/latin/RichInputConnection.java +++ b/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 diff --git a/tests/src/com/android/inputmethod/latin/RichInputConnectionTests.java b/tests/src/com/android/inputmethod/latin/RichInputConnectionAndTextRangeTests.java similarity index 53% rename from tests/src/com/android/inputmethod/latin/RichInputConnectionTests.java rename to tests/src/com/android/inputmethod/latin/RichInputConnectionAndTextRangeTests.java index aacd60f4d..0e077bbbe 100644 --- a/tests/src/com/android/inputmethod/latin/RichInputConnectionTests.java +++ b/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 String mTextAfter; + final CharSequence mTextBefore; + 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); + } }