New issue
Advanced search Search tips

Issue 879170 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 11
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

[Duet] Home/Clear tab button needs to match top toolbar

Project Member Reported by mdjones@chromium.org, Aug 30

Issue description

The behavior of the home button in the bottom toolbar should match the behavior of when it is at the top.
 
Owner: amaralp@chromium.org
Status: Assigned (was: Available)
Pedro, do you have time to look at this? Hopefully matching the top button's behavior isn't that complicated.
What is the difference in behavior? I know we always use the same icon so I could have the icon change depending on what feature is enabled. As for the behavior I think we are basically doing the same thing. Currently we call ToolbarManager#openHomepage() which does some checks to see if we should clear the tab or go to the homepage. It looks like the only difference between ToolbarLayout's and ToolbarManager's openHomepage() is that ToolbarLayout hides the location bar's suggestions. I could move that hiding to ToolbarManager's method.
Also I don't think the accessibility string ever gets updated for clear tabs. At least ToolbarLayout#changeIconToNTPIcon() never changes the string.
We'd need to either navigate to the user/carrier's home page or the NTP depending on the state of the experiment. I'll try to find where that is implemented currently.
Labels: Merge-Request-70
Requesting merge into M70 as this would block a stable experiment.
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/287b73abb86bfb28a9086326a6fe940df87d1368

commit 287b73abb86bfb28a9086326a6fe940df87d1368
Author: Pedro Amaral <amaralp@chromium.org>
Date: Wed Sep 05 00:05:12 2018

Change home button icons in bottom bar depending on feature

Bug:  879170 
Change-Id: Ib256f52f2445f5a99b835b0a8682bda2b51f9848
Reviewed-on: https://chromium-review.googlesource.com/1198013
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588709}
[modify] https://crrev.com/287b73abb86bfb28a9086326a6fe940df87d1368/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarButtonInProductHelpController.java
[modify] https://crrev.com/287b73abb86bfb28a9086326a6fe940df87d1368/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarDataProvider.java
[modify] https://crrev.com/287b73abb86bfb28a9086326a6fe940df87d1368/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java
[modify] https://crrev.com/287b73abb86bfb28a9086326a6fe940df87d1368/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/287b73abb86bfb28a9086326a6fe940df87d1368/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModel.java
[modify] https://crrev.com/287b73abb86bfb28a9086326a6fe940df87d1368/chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java

Project Member

Comment 7 by sheriffbot@chromium.org, Sep 6

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 6

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/030b38f05a46e2ca35016fd55307be06d838ba6a

commit 030b38f05a46e2ca35016fd55307be06d838ba6a
Author: Pedro Amaral <amaralp@chromium.org>
Date: Thu Sep 06 18:04:08 2018

Change home button icons in bottom bar depending on feature

TBR=amaralp@chromium.org

(cherry picked from commit 287b73abb86bfb28a9086326a6fe940df87d1368)

Bug:  879170 
Change-Id: Ib256f52f2445f5a99b835b0a8682bda2b51f9848
Reviewed-on: https://chromium-review.googlesource.com/1198013
Commit-Queue: Pedro Amaral <amaralp@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588709}
Reviewed-on: https://chromium-review.googlesource.com/1211243
Reviewed-by: Pedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#91}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/030b38f05a46e2ca35016fd55307be06d838ba6a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarButtonInProductHelpController.java
[modify] https://crrev.com/030b38f05a46e2ca35016fd55307be06d838ba6a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarDataProvider.java
[modify] https://crrev.com/030b38f05a46e2ca35016fd55307be06d838ba6a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarLayout.java
[modify] https://crrev.com/030b38f05a46e2ca35016fd55307be06d838ba6a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarManager.java
[modify] https://crrev.com/030b38f05a46e2ca35016fd55307be06d838ba6a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModel.java
[modify] https://crrev.com/030b38f05a46e2ca35016fd55307be06d838ba6a/chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java

Status: Fixed (was: Assigned)
Merged into M70, marking as fixed.

Sign in to add a comment