Commit 098695f5 authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

Fix paste not triggering suggestions on the NTP.

The issue is that this refactor:
https://chromium-review.googlesource.com/1148680
updated all text updates to the URL bar from the location bar to
be ignored for autocomplete purposes.

But in the old code, there were cases where it used setText/setUrl
directly and those would have triggered autocomplete.  setUrlBarText
did the current behavior in the old code, so those paths do not
need updating.

Instead of implicit triggers for autocomplete, this uses the consistent
path of never triggering autocomplete implicitly, but instead makes
it explicit in the cases we want.

Also fixes a trivial case where "" text updates (i.e. from the NTP)
were spamming the URL bar where it didn't need to.

BUG=871130

Change-Id: I26664bf3aa065176830f60b250d57a64de1c10f9
Reviewed-on: https://chromium-review.googlesource.com/1163992
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581406}
parent ce296d4e
......@@ -272,12 +272,10 @@ public class LocationBarLayout extends FrameLayout
OmniboxResultItem selectedItem =
(OmniboxResultItem) mSuggestionListAdapter.getItem(
mSuggestionList.getSelectedItemPosition());
// Set the UrlBar text to empty, so that it will trigger a text change when we
// set the text to the suggestion again.
setUrlBarTextEmpty();
setUrlBarText(
UrlBarData.forNonUrlText(selectedItem.getSuggestion().getFillIntoEdit()),
UrlBar.ScrollType.NO_SCROLL, SelectionState.SELECT_END);
onTextChangedForAutocomplete();
mSuggestionList.setSelection(0);
return true;
} else if (KeyNavigationUtil.isEnter(event)
......@@ -931,6 +929,7 @@ public class LocationBarLayout extends FrameLayout
// This must be happen after requestUrlFocus(), which changes the selection.
mUrlCoordinator.setUrlBarData(UrlBarData.forNonUrlText(pastedText),
UrlBar.ScrollType.NO_SCROLL, UrlBarCoordinator.SelectionState.SELECT_END);
onTextChangedForAutocomplete();
} else {
ToolbarManager.recordOmniboxFocusReason(ToolbarManager.OmniboxFocusReason.FAKE_BOX_TAP);
}
......@@ -1346,6 +1345,7 @@ public class LocationBarLayout extends FrameLayout
mUrlCoordinator.setUrlBarData(UrlBarData.forNonUrlText(refineText),
UrlBar.ScrollType.NO_SCROLL, UrlBarCoordinator.SelectionState.SELECT_END);
onTextChangedForAutocomplete();
if (isUrlSuggestion) {
RecordUserAction.record("MobileOmniboxRefineSuggestion.Url");
} else {
......
......@@ -81,7 +81,8 @@ class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate {
}
}
if (isNewTextEquivalentToExistingText(mUrlBarData, data) && mScrollType == scrollType) {
if (!mHasFocus && isNewTextEquivalentToExistingText(mUrlBarData, data)
&& mScrollType == scrollType) {
return false;
}
mUrlBarData = data;
......@@ -119,6 +120,10 @@ class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate {
// Regardless of focus state, ensure the text content is the same.
if (!TextUtils.equals(existingCharSequence, newCharSequence)) return false;
// If both existing and new text is empty, then treat them equal regardless of their
// spanned state.
if (TextUtils.isEmpty(newCharSequence)) return true;
// When not focused, compare the emphasis spans applied to the text to determine
// equality. Internally, TextView applies many additional spans that need to be
// ignored for this comparison to be useful, so this is scoped to only the span types
......
......@@ -82,6 +82,11 @@ public class UrlBarMediatorUnitTest {
public void urlDataComparison_equals() {
Assert.assertTrue(UrlBarMediator.isNewTextEquivalentToExistingText(null, null));
// Empty display text, regardless of spanned state.
Assert.assertTrue(UrlBarMediator.isNewTextEquivalentToExistingText(
UrlBarData.create(null, spannable(""), 0, 0, null),
UrlBarData.create(null, "", 0, 0, null)));
// No editing text, equal display text
Assert.assertTrue(UrlBarMediator.isNewTextEquivalentToExistingText(
UrlBarData.create(null, spannable("Test"), 0, 0, null),
......
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