Commit 899e647a authored by Troy Hildebrandt's avatar Troy Hildebrandt Committed by Commit Bot

Scroll to beginning of text when displaying search terms in omnibox.

The assumption is currently that the URL bar displays URLs, so we
always make a call to #scrollToTLD. This results in showing the end of
a query, which is inconsistent with common SRP behaviour.

This CL ensures that when we're displaying search terms, we scroll to
the start of the text in an RTL friendly way.

Bug: 776988
Change-Id: Ied926bd276f312a7e4ab18b9c45173d0ce15dbbe
Reviewed-on: https://chromium-review.googlesource.com/935452
Commit-Queue: Troy Hildebrandt <thildebr@chromium.org>
Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539505}
parent d6db2b70
...@@ -2090,6 +2090,13 @@ public class LocationBarLayout extends FrameLayout ...@@ -2090,6 +2090,13 @@ public class LocationBarLayout extends FrameLayout
return !mToolbarDataProvider.isDisplayingQueryTerms(); return !mToolbarDataProvider.isDisplayingQueryTerms();
} }
@Override
@UrlBar.ScrollType
public int getScrollType() {
return mToolbarDataProvider.isDisplayingQueryTerms() ? UrlBar.SCROLL_TO_BEGINNING
: UrlBar.SCROLL_TO_TLD;
}
/** /**
* @return Returns the original url of the page. * @return Returns the original url of the page.
*/ */
...@@ -2698,9 +2705,9 @@ public class LocationBarLayout extends FrameLayout ...@@ -2698,9 +2705,9 @@ public class LocationBarLayout extends FrameLayout
} }
/** /**
* Scroll to ensure the TLD is visible. * Scroll to ensure the TLD is visible, if necessary.
*/ */
public void scrollUrlBarToTld() { public void scrollUrlBarToTld() {
mUrlBar.scrollToTLD(); if (getScrollType() == UrlBar.SCROLL_TO_TLD) mUrlBar.scrollToTLD();
} }
} }
...@@ -15,6 +15,7 @@ import android.net.Uri; ...@@ -15,6 +15,7 @@ import android.net.Uri;
import android.os.Build; import android.os.Build;
import android.os.StrictMode; import android.os.StrictMode;
import android.os.SystemClock; import android.os.SystemClock;
import android.support.annotation.IntDef;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import android.support.v4.text.BidiFormatter; import android.support.v4.text.BidiFormatter;
import android.text.Editable; import android.text.Editable;
...@@ -97,7 +98,7 @@ public class UrlBar extends AutocompleteEditText { ...@@ -97,7 +98,7 @@ public class UrlBar extends AutocompleteEditText {
private MotionEvent mSuppressedTouchDownEvent; private MotionEvent mSuppressedTouchDownEvent;
private boolean mAllowFocus = true; private boolean mAllowFocus = true;
private boolean mPendingScrollTLD; private boolean mPendingScroll;
private int mPreviousWidth; private int mPreviousWidth;
private String mPreviousTldScrollText; private String mPreviousTldScrollText;
private int mPreviousTldScrollViewWidth; private int mPreviousTldScrollViewWidth;
...@@ -127,6 +128,14 @@ public class UrlBar extends AutocompleteEditText { ...@@ -127,6 +128,14 @@ public class UrlBar extends AutocompleteEditText {
/** The location of this view on the last ACTION_DOWN event. */ /** The location of this view on the last ACTION_DOWN event. */
private float mDownEventViewTop; private float mDownEventViewTop;
/** What scrolling action should be taken after the URL bar text changes. **/
@IntDef({NO_SCROLL, SCROLL_TO_TLD, SCROLL_TO_BEGINNING})
public @interface ScrollType {}
public static final int NO_SCROLL = 0;
public static final int SCROLL_TO_TLD = 1;
public static final int SCROLL_TO_BEGINNING = 2;
/** /**
* Implement this to get updates when the direction of the text in the URL bar changes. * Implement this to get updates when the direction of the text in the URL bar changes.
* E.g. If the user is typing a URL, then erases it and starts typing a query in Arabic, * E.g. If the user is typing a URL, then erases it and starts typing a query in Arabic,
...@@ -176,6 +185,12 @@ public class UrlBar extends AutocompleteEditText { ...@@ -176,6 +185,12 @@ public class UrlBar extends AutocompleteEditText {
* @return Whether or not we should force LTR text on the URL bar when unfocused. * @return Whether or not we should force LTR text on the URL bar when unfocused.
*/ */
boolean shouldForceLTR(); boolean shouldForceLTR();
/**
* @return What scrolling action should be performed after the URL text is modified.
*/
@ScrollType
int getScrollType();
} }
public UrlBar(Context context, AttributeSet attrs) { public UrlBar(Context context, AttributeSet attrs) {
...@@ -314,7 +329,7 @@ public class UrlBar extends AutocompleteEditText { ...@@ -314,7 +329,7 @@ public class UrlBar extends AutocompleteEditText {
if (focused) { if (focused) {
StartupMetrics.getInstance().recordFocusedOmnibox(); StartupMetrics.getInstance().recordFocusedOmnibox();
mPendingScrollTLD = false; mPendingScroll = false;
} }
fixupTextDirection(); fixupTextDirection();
...@@ -658,22 +673,42 @@ public class UrlBar extends AutocompleteEditText { ...@@ -658,22 +673,42 @@ public class UrlBar extends AutocompleteEditText {
setText(displayText); setText(displayText);
boolean textChanged = !TextUtils.equals(previousText, getEditableText()); boolean textChanged = !TextUtils.equals(previousText, getEditableText());
if (textChanged && !isFocused()) scrollToTLD(); if (textChanged && !isFocused()) scrollDisplayText();
return textChanged; return textChanged;
} }
/** public void scrollDisplayText() {
* Scroll to ensure the TLD is visible.
*/
public void scrollToTLD() {
if (isLayoutRequested()) { if (isLayoutRequested()) {
mPendingScrollTLD = true; if (mUrlBarDelegate.getScrollType() == NO_SCROLL) return;
mPendingScroll = true;
} else { } else {
scrollToTLDInternal(); scrollDisplayTextInternal();
} }
} }
private void scrollToTLDInternal() { public void scrollDisplayTextInternal() {
switch (mUrlBarDelegate.getScrollType()) {
case SCROLL_TO_TLD:
scrollToTLD();
break;
case SCROLL_TO_BEGINNING:
scrollToBeginning();
break;
default:
break;
}
}
private void scrollToBeginning() {
int scrollX = 0;
if (BidiFormatter.getInstance().isRtl(getTextWithAutocomplete())) {
int textWidth = (int) getLayout().getPaint().measureText(getTextWithAutocomplete());
scrollX = textWidth - getMeasuredWidth();
}
scrollTo(scrollX, getScrollY());
}
public void scrollToTLD() {
if (mFocused) return; if (mFocused) return;
// Ensure any selection from the focus state is cleared. // Ensure any selection from the focus state is cleared.
...@@ -757,11 +792,11 @@ public class UrlBar extends AutocompleteEditText { ...@@ -757,11 +792,11 @@ public class UrlBar extends AutocompleteEditText {
protected void onLayout(boolean changed, int left, int top, int right, int bottom) { protected void onLayout(boolean changed, int left, int top, int right, int bottom) {
super.onLayout(changed, left, top, right, bottom); super.onLayout(changed, left, top, right, bottom);
if (mPendingScrollTLD) { if (mPendingScroll) {
scrollToTLDInternal(); scrollDisplayTextInternal();
mPendingScrollTLD = false; mPendingScroll = false;
} else if (mPreviousWidth != (right - left)) { } else if (mPreviousWidth != (right - left)) {
scrollToTLDInternal(); scrollDisplayTextInternal();
mPreviousWidth = right - left; mPreviousWidth = right - left;
} }
} }
...@@ -771,7 +806,7 @@ public class UrlBar extends AutocompleteEditText { ...@@ -771,7 +806,7 @@ public class UrlBar extends AutocompleteEditText {
// TextView internally attempts to keep the selection visible, but in the unfocused state // TextView internally attempts to keep the selection visible, but in the unfocused state
// this class ensures that the TLD is visible. // this class ensures that the TLD is visible.
if (!mFocused) return false; if (!mFocused) return false;
assert !mPendingScrollTLD; assert !mPendingScroll;
return super.bringPointIntoView(offset); return super.bringPointIntoView(offset);
} }
......
...@@ -706,6 +706,12 @@ public class CustomTabToolbar extends ToolbarLayout implements LocationBar, ...@@ -706,6 +706,12 @@ public class CustomTabToolbar extends ToolbarLayout implements LocationBar,
return true; return true;
} }
@Override
@UrlBar.ScrollType
public int getScrollType() {
return UrlBar.SCROLL_TO_TLD;
}
@Override @Override
public void setUrlBarFocus(boolean shouldBeFocused) {} public void setUrlBarFocus(boolean shouldBeFocused) {}
......
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