[Duet] Home/Clear tab button needs to match top toolbar |
|||||
Issue descriptionThe behavior of the home button in the bottom toolbar should match the behavior of when it is at the top.
,
Aug 30
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.
,
Aug 30
Also I don't think the accessibility string ever gets updated for clear tabs. At least ToolbarLayout#changeIconToNTPIcon() never changes the string.
,
Aug 30
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.
,
Sep 5
Requesting merge into M70 as this would block a stable experiment.
,
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
,
Sep 6
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
,
Sep 6
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
,
Sep 11
Merged into M70, marking as fixed. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by mdjones@chromium.org
, Aug 30Status: Assigned (was: Available)