Commit ff89d6eb authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

Fix RTL hint text positioning in the UrlBar.

The MVC refactor for UrlBar.java:
https://chromium-review.googlesource.com/1148680
converted empty text strings to pass in SCROLL_BEGIN.

The logic for properly scrolling empty RTL text existed
only within SCROLL_TO_TLD.  This moves and unifies the
logic within the SCROLL_BEGIN handling.

This also fixes some issues with resetting the previous
scroll state too aggressively so that multiple calls
would need to recalculate.

BUG=871147

Change-Id: I8cbe07a4765b991fa39999dd92ae8dcc8f08538a
Reviewed-on: https://chromium-review.googlesource.com/1164477
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581253}
parent d0f13af5
...@@ -36,6 +36,7 @@ import org.chromium.base.Log; ...@@ -36,6 +36,7 @@ import org.chromium.base.Log;
import org.chromium.base.SysUtils; import org.chromium.base.SysUtils;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.chrome.browser.WindowDelegate; import org.chromium.chrome.browser.WindowDelegate;
import org.chromium.chrome.browser.omnibox.UrlBar.ScrollType;
import org.chromium.chrome.browser.toolbar.ToolbarManager; import org.chromium.chrome.browser.toolbar.ToolbarManager;
import org.chromium.ui.UiUtils; import org.chromium.ui.UiUtils;
...@@ -87,10 +88,13 @@ public class UrlBar extends AutocompleteEditText { ...@@ -87,10 +88,13 @@ public class UrlBar extends AutocompleteEditText {
private boolean mPendingScroll; private boolean mPendingScroll;
private int mPreviousWidth; private int mPreviousWidth;
private String mPreviousTldScrollText;
private int mPreviousTldScrollViewWidth; @ScrollType
private int mPreviousTldScrollResultXPosition; private int mPreviousScrollType;
private float mPreviousFontSize; private String mPreviousScrollText;
private int mPreviousScrollViewWidth;
private int mPreviousScrollResultXPosition;
private float mPreviousScrollFontSize;
// Used as a hint to indicate the text may contain an ellipsize span. This will be true if an // Used as a hint to indicate the text may contain an ellipsize span. This will be true if an
// ellispize span was applied the last time the text changed. A true value here does not // ellispize span was applied the last time the text changed. A true value here does not
...@@ -616,6 +620,25 @@ public class UrlBar extends AutocompleteEditText { ...@@ -616,6 +620,25 @@ public class UrlBar extends AutocompleteEditText {
*/ */
private void scrollDisplayTextInternal(@ScrollType int scrollType) { private void scrollDisplayTextInternal(@ScrollType int scrollType) {
mPendingScroll = false; mPendingScroll = false;
if (mFocused) return;
Editable text = getText();
if (TextUtils.isEmpty(text)) scrollType = ScrollType.SCROLL_TO_BEGINNING;
// Ensure any selection from the focus state is cleared.
setSelection(0);
int measuredWidth = getMeasuredWidth() - (getPaddingLeft() + getPaddingRight());
if (scrollType == mPreviousScrollType && TextUtils.equals(text, mPreviousScrollText)
&& measuredWidth == mPreviousScrollViewWidth
// Font size is float but it changes in discrete range (eg small font, big font),
// therefore false negative using regular equality is unlikely.
&& getTextSize() == mPreviousScrollFontSize) {
scrollTo(mPreviousScrollResultXPosition, getScrollY());
return;
}
switch (scrollType) { switch (scrollType) {
case ScrollType.SCROLL_TO_TLD: case ScrollType.SCROLL_TO_TLD:
scrollToTLD(); scrollToTLD();
...@@ -624,21 +647,32 @@ public class UrlBar extends AutocompleteEditText { ...@@ -624,21 +647,32 @@ public class UrlBar extends AutocompleteEditText {
scrollToBeginning(); scrollToBeginning();
break; break;
default: default:
break; // Intentional return to avoid clearing scroll state when no scroll was applied.
return;
} }
mPreviousScrollType = scrollType;
mPreviousScrollText = text.toString();
mPreviousScrollViewWidth = measuredWidth;
mPreviousScrollFontSize = getTextSize();
mPreviousScrollResultXPosition = getScrollX();
} }
/** /**
* Scrolls the omnibox text to show the very beginning of the text entered. * Scrolls the omnibox text to show the very beginning of the text entered.
*/ */
private void scrollToBeginning() { private void scrollToBeginning() {
if (mFocused) return;
setSelection(0);
Editable text = getText(); Editable text = getText();
float scrollPos = 0f; float scrollPos = 0f;
if (BidiFormatter.getInstance().isRtl(text)) { if (TextUtils.isEmpty(text)) {
if (ApiCompatibilityUtils.isLayoutRtl(this)
&& BidiFormatter.getInstance().isRtl(getHint())) {
// Compared to below that uses getPrimaryHorizontal(1) due to 0 returning an
// invalid value, if the text is empty, getPrimaryHorizontal(0) returns the actual
// max scroll amount.
scrollPos = (int) getLayout().getPrimaryHorizontal(0) - getMeasuredWidth();
}
} else if (BidiFormatter.getInstance().isRtl(text)) {
// RTL. // RTL.
float endPointX = getLayout().getPrimaryHorizontal(text.length()); float endPointX = getLayout().getPrimaryHorizontal(text.length());
int measuredWidth = getMeasuredWidth(); int measuredWidth = getMeasuredWidth();
...@@ -652,42 +686,8 @@ public class UrlBar extends AutocompleteEditText { ...@@ -652,42 +686,8 @@ public class UrlBar extends AutocompleteEditText {
* Scrolls the omnibox text to bring the TLD into view. * Scrolls the omnibox text to bring the TLD into view.
*/ */
private void scrollToTLD() { private void scrollToTLD() {
if (mFocused) return;
// Ensure any selection from the focus state is cleared.
setSelection(0);
String previousTldScrollText = mPreviousTldScrollText;
int previousTldScrollViewWidth = mPreviousTldScrollViewWidth;
int previousTldScrollResultXPosition = mPreviousTldScrollResultXPosition;
mPreviousTldScrollText = null;
mPreviousTldScrollViewWidth = 0;
mPreviousTldScrollResultXPosition = 0;
Editable url = getText(); Editable url = getText();
if (url == null || url.length() < 1) {
int scrollX = 0;
if (ApiCompatibilityUtils.isLayoutRtl(this)
&& BidiFormatter.getInstance().isRtl(getHint())) {
// Compared to below that uses getPrimaryHorizontal(1) due to 0 returning an
// invalid value, if the text is empty, getPrimaryHorizontal(0) returns the actual
// max scroll amount.
scrollX = (int) getLayout().getPrimaryHorizontal(0) - getMeasuredWidth();
}
scrollTo(scrollX, getScrollY());
return;
}
int measuredWidth = getMeasuredWidth() - (getPaddingLeft() + getPaddingRight()); int measuredWidth = getMeasuredWidth() - (getPaddingLeft() + getPaddingRight());
if (TextUtils.equals(url, previousTldScrollText)
&& measuredWidth == previousTldScrollViewWidth
// Font size is float but it changes in discrete range (eg small font, big font),
// therefore false negative using regular equality is unlikely.
&& getTextSize() == mPreviousFontSize) {
scrollTo(previousTldScrollResultXPosition, getScrollY());
return;
}
assert getLayout().getLineCount() == 1; assert getLayout().getLineCount() == 1;
float endPointX = getLayout().getPrimaryHorizontal(mOriginEndIndex); float endPointX = getLayout().getPrimaryHorizontal(mOriginEndIndex);
...@@ -710,11 +710,6 @@ public class UrlBar extends AutocompleteEditText { ...@@ -710,11 +710,6 @@ public class UrlBar extends AutocompleteEditText {
} }
} }
scrollTo((int) scrollPos, getScrollY()); scrollTo((int) scrollPos, getScrollY());
mPreviousTldScrollText = url.toString();
mPreviousTldScrollViewWidth = measuredWidth;
mPreviousFontSize = getTextSize();
mPreviousTldScrollResultXPosition = (int) scrollPos;
} }
@Override @Override
......
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