Shelf indicator is not correct for Settings |
|||||||||||
Issue descriptionChrome Version: (copy from chrome://version) OS: (e.g. Win10, MacOS 10.12, etc...) What steps will reproduce the problem? (1) Open Chrome (2) Open Settings (3) What is the expected result? Settings icon should have the "focused" indicator What happens instead? It has the "active" indicator. Video: https://photos.app.goo.gl/Ukrzm9j44aJfeUqQA Please use labels and text to provide additional information. If this is a regression (i.e., worked before), please consider using the bisect tool (https://www.chromium.org/developers/bisect-builds-py) to help us identify the root cause and more rapidly triage the issue. For graphics-related bugs, please copy/paste the contents of the about:gpu page at the end of this report.
,
Sep 5
Nope, only saw for Settings.
,
Sep 5
I'm going to look into this, but ccing msw@ who had some great suggestions on how to implement this, and raised some concerns about why the previous approach wouldn't work (e.g. for the keyboard shortcut help panel). Michael, any idea what is special about the Settings app? It seems to me like this is the only app where the indicators don't work as expected.
,
Sep 5
Note that this bug only happens the first time you launch Settings. If you then switch around between Settings and other apps, it works fine again. Given this restriction and the fact that only Settings is impacted, demoting to P2. Let me know if you disagree.
,
Sep 5
,
Sep 8
,
Sep 10
The Settings app window's shelf item is handled by ShelfWindowWatcher in Ash instead of ChromeLauncherController. I wonder if this is related to the delayed ShelfID assignment by SettingsWindowObserver::OnNewSettingsWindow: https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/settings_window_observer.cc?rcl=3fe22318a2be8cb2b058bc53f6896477db39a4fc&l=53 Perhaps the item on the shelf has a different ShelfID than the window (at least for a short period of time?) Maybe Settings is given a browser ShelfID at first? I don't think Mash's temp ShelfID should be used in the default (non-Mash) config. https://cs.chromium.org/chromium/src/ash/shelf/shelf_window_watcher.cc?rcl=3fe22318a2be8cb2b058bc53f6896477db39a4fc&l=43 https://cs.chromium.org/chromium/src/ash/shelf/shelf_window_watcher.cc?rcl=3fe22318a2be8cb2b058bc53f6896477db39a4fc&l=122 I would record calls to SetActiveShelfID; we may need to call that in some additional codepath when ShelfID window property values change?
,
Sep 11
Actually, the settings app window's shelf item might be handled by InternalAppWindowShelfController in classic ash... https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/internal_app_window_shelf_controller.cc?rcl=6e7d8014ea79c0c11957fd962ae857b0af490697&l=82 https://cs.chromium.org/chromium/src/chrome/browser/ui/app_list/internal_app/internal_app_metadata.cc?sq=package:chromium&dr=C&g=0&l=78
,
Sep 16
Punting to M-71.
,
Oct 15
Bulk moving all M-71 <P-1's to M-72.
,
Oct 15
,
Dec 3
Bulk moving <p-1's to the next milestone because we branched to M-73.
,
Dec 18
,
Dec 18
+CC mukai@, this issue is related to Mash shelf item handling |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by manucornet@chromium.org
, Sep 5