Skip to content

Commit

Permalink
[Omnibox] Hide url actions when their width is needed to show UrlBar
Browse files Browse the repository at this point in the history 
The url action container's width detracts from the width of the url bar
when the url bar is focused. In the case of very narrow windows on
tablets (and potentially elsewhere) this leaves no width for the url bar
itself. Since a 0-width view can't be focused, this prevents omnibox
focus entirely. This CL hides/shows the action container
when it detects a transition to/away this state.

(cherry picked from commit 029155a)

Bug: 1383224, 1428914
Change-Id: I6bca4b4a4ad70b9cd867d1f9ce9c0c880c357ee6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4650854
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1164758}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4664534
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5845@{#310}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
Patrick Noland authored and Chromium LUCI CQ committed Jul 5, 2023
1 parent 71e578b commit 3331589
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 32 deletions.
51 changes: 35 additions & 16 deletions chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/LocationBarLayoutTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.mockito.Mockito.doReturn;

import android.view.View;
import android.view.View.MeasureSpec;
import android.view.ViewGroup.MarginLayoutParams;
import android.view.WindowManager;
import android.widget.ImageButton;
Expand Down Expand Up @@ -49,8 +50,8 @@
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.R;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.chrome.test.util.OmniboxTestUtils;
import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
import org.chromium.content_public.browser.test.util.ClickUtils;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.permissions.AndroidPermissionDelegate;
Expand All @@ -75,8 +76,6 @@ public class LocationBarLayoutTest {

@Mock
AndroidPermissionDelegate mAndroidPermissionDelegate;
@Mock
SearchEngineLogoUtils mSearchEngineLogoUtils;

private OmniboxTestUtils mOmnibox;

Expand Down Expand Up @@ -281,7 +280,7 @@ public void testUpdateLayoutParams() {
urlContainer.setLayoutParams(urlLayoutParams);

statusIcon.setVisibility(GONE);
locationBar.updateLayoutParams();
locationBar.updateLayoutParams(MeasureSpec.makeMeasureSpec(1000, MeasureSpec.EXACTLY));
urlLayoutParams = (MarginLayoutParams) urlContainer.getLayoutParams();
int endMarginNoIcon = MarginLayoutParamsCompat.getMarginEnd(urlLayoutParams);

Expand All @@ -290,7 +289,7 @@ public void testUpdateLayoutParams() {
urlContainer.setLayoutParams(urlLayoutParams);

statusIcon.setVisibility(VISIBLE);
locationBar.updateLayoutParams();
locationBar.updateLayoutParams(MeasureSpec.makeMeasureSpec(1000, MeasureSpec.EXACTLY));
urlLayoutParams = (MarginLayoutParams) urlContainer.getLayoutParams();
int endMarginWithIcon = MarginLayoutParamsCompat.getMarginEnd(urlLayoutParams);

Expand All @@ -299,19 +298,39 @@ public void testUpdateLayoutParams() {
});
}

/** Load a new URL and also update the location bar models. */
private Tab loadUrlInNewTabAndUpdateModels(String url, boolean incognito) {
Tab tab = mActivityTestRule.loadUrlInNewTab(url, incognito);
updateLocationBar();
return tab;
}
@Test
@MediumTest
public void testEnforceMinimumUrlBarWidth() {
setUrlBarTextAndFocus("");

private void updateLocationBar() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
LocationBarMediator mediator = getLocationBarMediator();
mediator.onPrimaryColorChanged();
mediator.onSecurityStateChanged();
mediator.onUrlChanged();
View urlBar = getUrlBar();
View locationBar = getLocationBar();

int constrainedWidth = ((MarginLayoutParams) urlBar.getLayoutParams()).getMarginStart()
+ locationBar.getResources().getDimensionPixelSize(
R.dimen.location_bar_min_url_width);
int urlContainerMarginEnd =
((MarginLayoutParams) urlBar.getLayoutParams()).getMarginEnd();

locationBar.measure(MeasureSpec.makeMeasureSpec(constrainedWidth, MeasureSpec.EXACTLY),
MeasureSpec.makeMeasureSpec(200, MeasureSpec.EXACTLY));
Assert.assertEquals(locationBar.findViewById(R.id.url_action_container).getVisibility(),
View.INVISIBLE);

locationBar.measure(
MeasureSpec.makeMeasureSpec(
constrainedWidth + urlContainerMarginEnd, MeasureSpec.EXACTLY),
MeasureSpec.makeMeasureSpec(200, MeasureSpec.EXACTLY));
Assert.assertEquals(locationBar.findViewById(R.id.url_action_container).getVisibility(),
View.VISIBLE);

locationBar.measure(
MeasureSpec.makeMeasureSpec(
constrainedWidth + urlContainerMarginEnd - 1, MeasureSpec.EXACTLY),
MeasureSpec.makeMeasureSpec(200, MeasureSpec.EXACTLY));
Assert.assertEquals(locationBar.findViewById(R.id.url_action_container).getVisibility(),
View.INVISIBLE);
});
}
}
51 changes: 35 additions & 16 deletions ...er/ui/android/omnibox/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public class LocationBarLayout extends FrameLayout {
protected StatusCoordinator mStatusCoordinator;

protected boolean mNativeInitialized;
protected boolean mHidingActionContainerForNarrowWindow;
protected int mMinimumUrlBarWidthPx;

protected LinearLayout mUrlActionContainer;

Expand All @@ -78,6 +80,8 @@ public LocationBarLayout(Context context, AttributeSet attrs, int layoutId) {
mUrlActionContainer = (LinearLayout) findViewById(R.id.url_action_container);
mStatusViewLeftSpace = findViewById(R.id.location_bar_status_view_left_space);
mStatusViewRightSpace = findViewById(R.id.location_bar_status_view_right_space);
mMinimumUrlBarWidthPx =
context.getResources().getDimensionPixelSize(R.dimen.location_bar_min_url_width);
}

/**
Expand All @@ -104,7 +108,7 @@ protected void onFinishInflate() {

@Override
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
updateLayoutParams();
updateLayoutParams(widthMeasureSpec);
super.onMeasure(widthMeasureSpec, heightMeasureSpec);
}

Expand Down Expand Up @@ -173,19 +177,20 @@ public View getSecurityIconView() {
}

/**
* @return The margin to be applied to the URL bar based on the buttons currently visible next
* to it, used to avoid text overlapping the buttons and vice versa.
* Returns the width of the url actions container, including its internal and external margins.
*/
private int getUrlContainerMarginEnd() {
private int getUrlActionContainerWidth() {
int urlContainerMarginEnd = 0;
for (View childView : getUrlContainerViewsForMargin()) {
ViewGroup.MarginLayoutParams childLayoutParams =
(ViewGroup.MarginLayoutParams) childView.getLayoutParams();
urlContainerMarginEnd += childLayoutParams.width
+ MarginLayoutParamsCompat.getMarginStart(childLayoutParams)
+ MarginLayoutParamsCompat.getMarginEnd(childLayoutParams);
}
if (mUrlActionContainer != null && mUrlActionContainer.getVisibility() == View.VISIBLE) {
// INVISIBLE views still take up space for the purpose of layout, so we consider the url
// action container's width unless it's GONE.
if (mUrlActionContainer != null && mUrlActionContainer.getVisibility() != View.GONE) {
for (View childView : getUrlContainerViewsForMargin()) {
ViewGroup.MarginLayoutParams childLayoutParams =
(ViewGroup.MarginLayoutParams) childView.getLayoutParams();
urlContainerMarginEnd += childLayoutParams.width
+ MarginLayoutParamsCompat.getMarginStart(childLayoutParams)
+ MarginLayoutParamsCompat.getMarginEnd(childLayoutParams);
}
ViewGroup.MarginLayoutParams urlActionContainerLayoutParams =
(ViewGroup.MarginLayoutParams) mUrlActionContainer.getLayoutParams();
urlContainerMarginEnd +=
Expand All @@ -208,7 +213,7 @@ private int getUrlContainerMarginEnd() {
* Updates the layout params for the location bar start aligned views.
*/
@VisibleForTesting
void updateLayoutParams() {
void updateLayoutParams(int parentWidthMeasureSpec) {
int startMargin = 0;
for (int i = 0; i < getChildCount(); i++) {
View childView = getChildAt(i);
Expand Down Expand Up @@ -247,10 +252,24 @@ void updateLayoutParams() {
}
}

int urlContainerMarginEnd = getUrlContainerMarginEnd();
int urlActionContainerWidth = getUrlActionContainerWidth();
int allocatedWidth = MeasureSpec.getSize(parentWidthMeasureSpec);
int availableWidth = allocatedWidth - startMargin - urlActionContainerWidth;
if (!mHidingActionContainerForNarrowWindow && availableWidth < mMinimumUrlBarWidthPx) {
mHidingActionContainerForNarrowWindow = true;
mUrlActionContainer.setVisibility(INVISIBLE);
} else if (mHidingActionContainerForNarrowWindow
&& mUrlActionContainer.getVisibility() != VISIBLE
&& availableWidth >= mMinimumUrlBarWidthPx) {
mHidingActionContainerForNarrowWindow = false;
mUrlActionContainer.setVisibility(VISIBLE);
}

int urlBarMarginEnd = mHidingActionContainerForNarrowWindow ? 0 : urlActionContainerWidth;

LayoutParams urlLayoutParams = (LayoutParams) mUrlBar.getLayoutParams();
if (MarginLayoutParamsCompat.getMarginEnd(urlLayoutParams) != urlContainerMarginEnd) {
MarginLayoutParamsCompat.setMarginEnd(urlLayoutParams, urlContainerMarginEnd);
if (MarginLayoutParamsCompat.getMarginEnd(urlLayoutParams) != urlBarMarginEnd) {
MarginLayoutParamsCompat.setMarginEnd(urlLayoutParams, urlBarMarginEnd);
mUrlBar.setLayoutParams(urlLayoutParams);
}
}
Expand Down

0 comments on commit 3331589

Please sign in to comment.