shelf position can't be changed in unified desktop mode |
|||||||
Issue description55.0.2883.42 (Official Build) beta (64-bit) Platform 8872.44.0 (Official Build) beta-channel samus I believe this is regression.
,
Dec 6 2017
I can't repro this. It is possible to change the shelf position in UD. However, it probably makes sense to revisit the shelf behavior in UD at this point. - We currently limit the shelf to be in the first display (i.e. upper left display in the matrix). - Shelf position > Left, Bottom, Right are limited only in the first display. - There is a bug in the workarea bounds when the shelf position is set to right. It subtracts the shelf height as if the shelf is placed in the second display. [See attached].
,
Dec 7 2017
I was actually able to repro by chance. Looking into this...
,
Dec 8 2017
James & msw, I found the reason why this is broken. CL with suggested fix is here https://chromium-review.googlesource.com/c/chromium/src/+/816135. - All you need to do to repro this issue, is + Set a different shelf alignment when you're not in unified mode. + Enable Unified Mode. + Try to change the shelf alignment, and observe that it fails. - Unified Desktop has a single display with ID = display::kUnifiedDisplayId = -10. Because of this negative value, it is not possible to save a per display shelf alignment pref for the unified display [1]. - However, it is possible to change the pref value, since display ID is always equal to the primary display ID [2]. The handler for the pref change doesn't just use the new value to set the shelf alignment, instead it tries to figure out which value to set according to some rules [3]. - Since there had been a per-display shelf alignment before moving to unified mode, and no one for the unified display itself, |has_per_display_prefs| is true [4], local prefs are not recommended, and hence we return the default value which is the BOTTOM alignment. - So the result is, the pref value is changed, but it is never applied. [1]: https://cs.chromium.org/chromium/src/ash/public/cpp/shelf_prefs.cc?q=SetPerDisplayPref&sq=package:chromium&l=91 [2]: https://cs.chromium.org/chromium/src/ash/public/cpp/shelf_prefs.cc?q=SetShelfAlignmentPref&sq=package:chromium&l=225 [3]: https://cs.chromium.org/chromium/src/ash/public/cpp/shelf_prefs.cc?q=GetPerDisplayPref&sq=package:chromium&l=33 [4]: https://cs.chromium.org/chromium/src/ash/public/cpp/shelf_prefs.cc?q=GetPerDisplayPref&sq=package:chromium&l=72
,
Dec 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/30a718372e1c872ef6dff7b2e93a0add15277c7b commit 30a718372e1c872ef6dff7b2e93a0add15277c7b Author: Ahmed Fakhry <afakhry@google.com> Date: Fri Dec 08 18:00:24 2017 Fix not being able to change shelf alignment in Unified Desktop Mode Make it possible to set a per-display shelf pref setting for the Unified display. Add test coverage for the case when a per-display shelf settings was added prior to enabling Unified Mode, which used to result in not being able to set the shelf alignment after going into Unified Mode. BUG= 665696 TEST=Added test coverage Change-Id: I57c35801721c0e3e6b3d0c60237be26ef1c3abe0 Reviewed-on: https://chromium-review.googlesource.com/816135 Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org> Cr-Commit-Position: refs/heads/master@{#522819} [modify] https://crrev.com/30a718372e1c872ef6dff7b2e93a0add15277c7b/ash/public/cpp/shelf_prefs.cc [modify] https://crrev.com/30a718372e1c872ef6dff7b2e93a0add15277c7b/ash/shelf/shelf_controller_unittest.cc
,
Dec 8 2017
,
Jul 30
,
Oct 26
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by abodenha@chromium.org
, Dec 1 2017