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

Issue 867775 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 31
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Web page is cut off at docked keyboard shadow

Project Member Reported by shend@chromium.org, Jul 26

Issue description

Chrome Version: 70.0.3502.0
OS: ChromeOS Eve

What steps will reproduce the problem?
(1) Launch the new UI docked virtual keyboard.

What is the expected result?
The shadows are semi-transparent; you should be able to see parts of the window behind the shadow.

What happens instead?
The shadows are opaque.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 26

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

commit c7dcd70e402ec71ee59b431ea02849e58243c85e
Author: Darren Shen <shend@chromium.org>
Date: Thu Jul 26 07:28:03 2018

[VK] Allow occluded bounds to be set for docked keyboard as well.

Before, we only allowed IME to set occluded bounds for fullscreen
keyboard because the other container behaviors (floating and docked)
knew what their occluded bounds were.

However, with recent changes, docked keyboard was made transparent so
that shadows could be drawn by IME. This means that the occluded bounds
for docked keyboard is not longer the same as the size of the window.
IME needs to tell the keyboard what parts are actually occluded so that
Chrome don't think shadows are also occluding the page.

We make SetOccludedBounds a method on the base ContainerBehavior class
and override it in docked keyboard. We also had to change
SetOccludedBounds to use coordinates relative to the window (since that
is what IME uses [*]).

Note that the occluded bounds for docked default to the keyboard window
bounds, so there are no behavioural changes if IME doesn't call
SetOccludedBounds.

[*] This didn't matter for fullscreen keyboard because the keyboard
bounds was same as screen bounds.

Bug:  867775 
Change-Id: I4eec3ecc281aad71781568f988cad2a2c3272ad2
Reviewed-on: https://chromium-review.googlesource.com/1151030
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578228}
[modify] https://crrev.com/c7dcd70e402ec71ee59b431ea02849e58243c85e/ui/keyboard/container_behavior.h
[modify] https://crrev.com/c7dcd70e402ec71ee59b431ea02849e58243c85e/ui/keyboard/container_full_width_behavior.cc
[modify] https://crrev.com/c7dcd70e402ec71ee59b431ea02849e58243c85e/ui/keyboard/container_full_width_behavior.h
[modify] https://crrev.com/c7dcd70e402ec71ee59b431ea02849e58243c85e/ui/keyboard/container_fullscreen_behavior.h
[modify] https://crrev.com/c7dcd70e402ec71ee59b431ea02849e58243c85e/ui/keyboard/keyboard_controller.cc
[modify] https://crrev.com/c7dcd70e402ec71ee59b431ea02849e58243c85e/ui/keyboard/keyboard_controller.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 30

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

commit 60b9c2fffb00aabd81da961035c22e381c64659e
Author: Darren Shen <shend@chromium.org>
Date: Mon Jul 30 05:21:31 2018

[VK] Draw shadows for docked keyboard.

In a previous patch, we removed shadows for both docked and floating
keyboard so that IME can draw them. Unfortunately, this means that
KeyboardController no longer knows which area of the keyboard is the
shadow and which is the actual UI, which breaks things like overscrolling.

A fix would be to make IME indicate what the actual occluded bounds are
through setOccludedBounds. Unfortunately, setOccludedBounds has several
problems (e.g. what happens when occluded bounds and visual bounds go
out of sync, e.g. when the screen is rotating). This requires some
additional thought.

So in the meantime, we just draw shadows only for docked mode. This will
at least unblock the new UI while we find a permanent solution.

Bug:  867775 
Change-Id: Iebd8f5edfeac5643fe837b672bf65dab5bfcdb4d
Reviewed-on: https://chromium-review.googlesource.com/1152328
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578979}
[modify] https://crrev.com/60b9c2fffb00aabd81da961035c22e381c64659e/chrome/browser/ui/ash/chrome_keyboard_ui.cc
[modify] https://crrev.com/60b9c2fffb00aabd81da961035c22e381c64659e/chrome/browser/ui/ash/chrome_keyboard_ui_unittest.cc
[modify] https://crrev.com/60b9c2fffb00aabd81da961035c22e381c64659e/chrome/browser/ui/ash/keyboard_controller_browsertest.cc

Labels: Merge-Request-69
Requesting M69 merge for c#2. Thanks!
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 31

Labels: -Merge-Request-69 Hotlist-Merge-Approved Merge-Approved-69
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 31

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9b5cfaae0e9cc00906e21e764f1863b7462301f

commit d9b5cfaae0e9cc00906e21e764f1863b7462301f
Author: Darren Shen <shend@chromium.org>
Date: Tue Jul 31 06:51:22 2018

[VK] Draw shadows for docked keyboard.

In a previous patch, we removed shadows for both docked and floating
keyboard so that IME can draw them. Unfortunately, this means that
KeyboardController no longer knows which area of the keyboard is the
shadow and which is the actual UI, which breaks things like overscrolling.

A fix would be to make IME indicate what the actual occluded bounds are
through setOccludedBounds. Unfortunately, setOccludedBounds has several
problems (e.g. what happens when occluded bounds and visual bounds go
out of sync, e.g. when the screen is rotating). This requires some
additional thought.

So in the meantime, we just draw shadows only for docked mode. This will
at least unblock the new UI while we find a permanent solution.

Bug:  867775 
Change-Id: Iebd8f5edfeac5643fe837b672bf65dab5bfcdb4d
Reviewed-on: https://chromium-review.googlesource.com/1152328
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#578979}(cherry picked from commit 60b9c2fffb00aabd81da961035c22e381c64659e)
Reviewed-on: https://chromium-review.googlesource.com/1156204
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#261}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/d9b5cfaae0e9cc00906e21e764f1863b7462301f/chrome/browser/ui/ash/chrome_keyboard_ui.cc
[modify] https://crrev.com/d9b5cfaae0e9cc00906e21e764f1863b7462301f/chrome/browser/ui/ash/chrome_keyboard_ui_unittest.cc
[modify] https://crrev.com/d9b5cfaae0e9cc00906e21e764f1863b7462301f/chrome/browser/ui/ash/keyboard_controller_browsertest.cc

Status: Fixed (was: Started)

Sign in to add a comment