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

Issue 870503 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 7
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Work area not reset after hiding accessibility keyboard.

Project Member Reported by shend@chromium.org, Aug 2

Issue description

Chrome Version: 70.0.3511.0
OS: ChromeOS Eve

What steps will reproduce the problem?
(1) Show accessibility keyboard
(2) Hide accessibility keyboard

What is the expected result?
The work area is reset.

What happens instead?
The work area does not reset, leaving a large gap on the screen.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 3

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

commit 8aff076a9bec981375116705a7a760b25b21952b
Author: Darren Shen <shend@chromium.org>
Date: Fri Aug 03 00:48:50 2018

[VK] Fix work area bug in shelf layout manager.

In a previous CL [1], we made changes to the shelf layout manager to
use occluded and displaced bounds. However, we introduced a bug where
the work area would not reset when we hide the accessibility keyboard,
leaving a large gap in the screen.

The code now makes more sense: we should be changing the work area
whenever the displaced bounds change.

We add a test for this. We also removed some calls to |LayoutShelf| in
existing tests; |LayoutShelf| doesn't get called in real situations.
Calling |LayoutShelf| manually will always change the work area,
which is not what happens in practice.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1146530

TBR=jamescook@chromium.org

Bug:  870503 
Change-Id: I80a6aa90f1bce8e59e6c7cde2e3f181c31fc6907
Reviewed-on: https://chromium-review.googlesource.com/1160827
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580394}
[modify] https://crrev.com/8aff076a9bec981375116705a7a760b25b21952b/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/8aff076a9bec981375116705a7a760b25b21952b/ash/shelf/shelf_layout_manager_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 7

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

commit f858e7a7908f59e291d8d16ad0874c632efa6ccf
Author: Darren Shen <shend@chromium.org>
Date: Tue Aug 07 02:24:03 2018

[ash] Remove |change_work_area| from ShelfLayoutManager.

|change_work_area| is used to determine whether to change the work area
or not. It is currently set to true when the displaced keyboard bounds
changes and false when the displaced bounds is the same. However, we can
actually set it to true even when the displaced bounds is the same,
because changing the work area to its current value should be a no-op
anyway.

If we make that change, then |change_work_area| is always true, so we
can just remove it from the code.

Bug:  870503 
Change-Id: Ia4359211f35f43996efcbd0152b3f4bfa6517caa
Reviewed-on: https://chromium-review.googlesource.com/1160828
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581098}
[modify] https://crrev.com/f858e7a7908f59e291d8d16ad0874c632efa6ccf/ash/shelf/shelf_layout_manager.cc
[modify] https://crrev.com/f858e7a7908f59e291d8d16ad0874c632efa6ccf/ash/shelf/shelf_layout_manager.h

Status: Fixed (was: Started)

Sign in to add a comment