New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 614042 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Taskbar is permanently relocated due to lock but should not be

Project Member Reported by kuscher@google.com, May 23 2016

Issue description

samus on beta channel (51.0.2704.55)

If I accidentally tap the power button to lock the screen, but not for long enough to lock the screen, the task bar moves to the bottom.  You have to lock the screen and unlock it to move it back.

Can be reproduced reliably. 
 

Comment 1 by nhendin@google.com, May 23 2016

I have made a video, and put it at this link (it's too big to fit as an attachment)

https://drive.google.com/open?id=0B4VWh7-bXG-pRkNUY3FKN0t0OEk

You may need to download the video till Drive recompresses it.

You can see in the video the task bar starts on the left, a quick tap of the power button (not enough to lock the screen) moves the taskbar to the bottom, and selecting the location back to the left again does not work (that part came out fuzzy on the camera video). 

Locking the screen fully and then unlocking works to fix the issue. It's as if when you hit the power button, the task bar moves to it's lock screen location before the lock screen is actually engaged (that is a total guess however).


Cc: abodenha@chromium.org jamescook@chromium.org glevin@chromium.org
Components: UI>Shell UI>Shell>Shelf UI>Shell>MultipleMonitor
Labels: -Pri-3 Pri-2

Comment 3 by osh...@chromium.org, May 23 2016

Owner: msw@chromium.org
Status: Assigned (was: Unconfirmed)
Could be related to recent mastash refactoring. msw@,can you look into this?

Comment 4 by msw@chromium.org, May 24 2016

Labels: M-51
Status: Started (was: Assigned)
Yeah, this is likely related to https://codereview.chromium.org/1877543002
I'll take a look; but fixing this in time for M-51 may not be possible.

Comment 5 by msw@chromium.org, May 25 2016

I have a WIP fix, but would appreciate some help testing it: https://codereview.chromium.org/2013933002/

With that CL, the alignment doesn't lock when tapping lock/power buttons (so it fixes that part), but I just get crashes (with or without the CL) when I hold either button (using --login-manager on my linux-chromeos desktop build). I'd like to make sure the shelf does apply bottom alignment at the right moment when holding the power/lock button, and the custom alignment is restored correctly on unlock. I made Alt+Shift+P = lock and Ctrl+Alt+Shift+P = power for testing.
Project Member

Comment 6 by bugdroid1@chromium.org, May 27 2016

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

commit dbd05211a0cbffa1bcd40888503c2cffeeb0d8ea
Author: msw <msw@chromium.org>
Date: Fri May 27 23:37:57 2016

Fix shelf alignment locking for power button.

Only lock the shelf once the lock animation starts.
(do not lock earlier, on the pre-lock animation start)
Add Alt-L (Lock) and Alt-P (Power) --ash-debug-shortcuts.

BUG= 614042 
TEST=Quickly tapping power/lock doesn't lock the shelf.
R=oshima@chromium.org

Review-Url: https://codereview.chromium.org/2013933002
Cr-Commit-Position: refs/heads/master@{#396599}

[modify] https://crrev.com/dbd05211a0cbffa1bcd40888503c2cffeeb0d8ea/ash/accelerators/accelerator_table.cc
[modify] https://crrev.com/dbd05211a0cbffa1bcd40888503c2cffeeb0d8ea/ash/shelf/shelf_locking_manager.cc

Comment 7 by msw@chromium.org, Jun 1 2016

Labels: Merge-Request-52 M-52
Requesting merge to M-52 (I'm assuming that it's too late for M-51).
(verified fixed on local linux-chromeos build on ToT@#397187)

Comment 8 by tin...@google.com, Jun 1 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 1 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/136633328b3c7b2f46be0fe238644191922a80b7

commit 136633328b3c7b2f46be0fe238644191922a80b7
Author: Michael Wasserman <msw@chromium.org>
Date: Wed Jun 01 22:14:32 2016

Fix shelf alignment locking for power button.

Only lock the shelf once the lock animation starts.
(do not lock earlier, on the pre-lock animation start)
Add Alt-L (Lock) and Alt-P (Power) --ash-debug-shortcuts.

BUG= 614042 
TEST=Quickly tapping power/lock doesn't lock the shelf.
R=oshima@chromium.org

Review-Url: https://codereview.chromium.org/2013933002
Cr-Commit-Position: refs/heads/master@{#396599}
(cherry picked from commit dbd05211a0cbffa1bcd40888503c2cffeeb0d8ea)

Review URL: https://codereview.chromium.org/2035483002 .

Cr-Commit-Position: refs/branch-heads/2743@{#175}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/136633328b3c7b2f46be0fe238644191922a80b7/ash/accelerators/accelerator_table.cc
[modify] https://crrev.com/136633328b3c7b2f46be0fe238644191922a80b7/ash/shelf/shelf_locking_manager.cc

Comment 10 by msw@chromium.org, Jun 1 2016

Cc: bhthompson@chromium.org keta...@chromium.org
Labels: Merge-Request-51
Requesting merge to M-51 on the off-chance we want this fix sooner; bhthompson@, oshima@, WDYT?
This is a very visible (but mostly benign) defect for users with left/right-aligned shelves in Chrome OS.
(if you tap the lock button, the shelf switches to bottom alignment, and can't be changed until re-login)
Project Member

Comment 11 by sheriffbot@chromium.org, Jun 2 2016

Labels: Hotlist-Google

Comment 12 by tin...@google.com, Jun 2 2016

Labels: -Merge-Request-51 Merge-Review-51 Hotlist-Merge-Review
[Automated comment] Request affecting a post-stable build (M51), manual review required.
Labels: -Merge-Review-51 Merge-Rejected-51
I think at this point we should skip this for 51, we should get it in 52. 
Labels: -Hotlist-Merge-review

Comment 15 by msw@chromium.org, Jun 14 2016

Status: Fixed (was: Started)
Okay, then this is fixed (already merged to M-52).
Status: Verified (was: Fixed)
Verified on 8350.36.0, 52.0.2743.46 beta and TOT

Comment 17 by msw@chromium.org, Aug 3 2016

 Issue 634100  has been merged into this issue.

Sign in to add a comment