Commit 1cfb8825 authored by tedchoc's avatar tedchoc Committed by Commit bot

[Android] Fix overzealous autocomplete removal in the omnibox.

To work around crbug.com/273763, we were previously removing
all text after the selection end if the selection range was
entirely before the autocomplete text.

On certain keyboards, this could result in deleting new user
text that differs from the previously autcompleted text.  Now,
we only clear the text if it matches the autocompleted text.

I verified that this fix did not regress the HTC issue (tested
on 4.2.2 w/ the Sense keyboard), and it also address the
Japanese IME issue mentioned in the new bug.

BUG=569144

Review-Url: https://codereview.chromium.org/2617493003
Cr-Commit-Position: refs/heads/master@{#441495}
parent a50c106b
......@@ -32,6 +32,7 @@ import android.view.inputmethod.InputConnection;
import android.view.inputmethod.InputConnectionWrapper;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.Log;
import org.chromium.base.SysUtils;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.R;
......@@ -52,7 +53,9 @@ import java.net.URL;
* The URL text entry view for the Omnibox.
*/
public class UrlBar extends VerticallyFixedEditText {
private static final String TAG = "UrlBar";
private static final String TAG = "cr_UrlBar";
private static final boolean DEBUG = false;
// TextView becomes very slow on long strings, so we limit maximum length
// of what is displayed to the user, see limitDisplayableLength().
......@@ -349,6 +352,7 @@ public class UrlBar extends VerticallyFixedEditText {
@Override
public void onBeginBatchEdit() {
if (DEBUG) Log.i(TAG, "onBeginBatchEdit");
mBeforeBatchEditAutocompleteIndex = getText().getSpanStart(mAutocompleteSpan);
mBeforeBatchEditFullText = getText().toString();
......@@ -359,6 +363,7 @@ public class UrlBar extends VerticallyFixedEditText {
@Override
public void onEndBatchEdit() {
if (DEBUG) Log.i(TAG, "onEndBatchEdit");
super.onEndBatchEdit();
mInBatchEditMode = false;
limitDisplayableLength();
......@@ -391,6 +396,7 @@ public class UrlBar extends VerticallyFixedEditText {
@Override
protected void onSelectionChanged(int selStart, int selEnd) {
if (DEBUG) Log.i(TAG, "onSelectionChanged -- selStart: %d, selEnd: %d", selStart, selEnd);
if (!mInBatchEditMode) {
int beforeTextLength = getText().length();
if (validateSelection(selStart, selEnd)) {
......@@ -414,7 +420,15 @@ public class UrlBar extends VerticallyFixedEditText {
private boolean validateSelection(int selStart, int selEnd) {
int spanStart = getText().getSpanStart(mAutocompleteSpan);
int spanEnd = getText().getSpanEnd(mAutocompleteSpan);
if (DEBUG) {
Log.i(TAG, "validateSelection -- selStart: %d, selEnd: %d, spanStart: %d, spanEnd: %d",
selStart, selEnd, spanStart, spanEnd);
}
if (spanStart >= 0 && (spanStart != selStart || spanEnd != selEnd)) {
CharSequence previousAutocompleteText = mAutocompleteSpan.mAutocompleteText;
// On selection changes, the autocomplete text has been accepted by the user or needs
// to be deleted below.
mAutocompleteSpan.clearSpan();
......@@ -426,7 +440,10 @@ public class UrlBar extends VerticallyFixedEditText {
// character appears, Chrome may decide to append some autocomplete, but the keyboard
// will then remove this temporary character only while leaving the autocomplete text
// alone. See crbug/273763 for more details.
if (selEnd <= spanStart) getText().delete(spanStart, getText().length());
if (selEnd <= spanStart && TextUtils.equals(previousAutocompleteText,
getText().subSequence(spanStart, getText().length()))) {
getText().delete(spanStart, getText().length());
}
return true;
}
return false;
......@@ -758,6 +775,10 @@ public class UrlBar extends VerticallyFixedEditText {
* @param inlineAutocompleteText The suggested autocompletion for the user's text.
*/
public void setAutocompleteText(CharSequence userText, CharSequence inlineAutocompleteText) {
if (DEBUG) {
Log.i(TAG, "setAutocompleteText -- userText: %s, inlineAutocompleteText: %s",
userText, inlineAutocompleteText);
}
boolean emptyAutocomplete = TextUtils.isEmpty(inlineAutocompleteText);
if (!emptyAutocomplete) mDisableTextScrollingFromAutocomplete = true;
......@@ -842,6 +863,11 @@ public class UrlBar extends VerticallyFixedEditText {
@Override
protected void onTextChanged(CharSequence text, int start, int lengthBefore, int lengthAfter) {
if (DEBUG) {
Log.i(TAG, "onTextChanged -- text: %s, start: %d, lengthBefore: %d, lengthAfter: %d",
text, start, lengthBefore, lengthAfter);
}
super.onTextChanged(text, start, lengthBefore, lengthAfter);
if (!mInBatchEditMode) {
limitDisplayableLength();
......@@ -854,6 +880,8 @@ public class UrlBar extends VerticallyFixedEditText {
@Override
public void setText(CharSequence text, BufferType type) {
if (DEBUG) Log.i(TAG, "setText -- text: %s", text);
mDisableTextScrollingFromAutocomplete = false;
// Avoid setting the same text to the URL bar as it will mess up the scroll/cursor
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment