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

Issue 893856 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Feature


Participants' hotlists:
vk-cujo-bugs
vk-cujo-bugs-M71


Sign in to add a comment

Fix keyboard hide/show behavior when using app switcher

Project Member Reported by pcovell@chromium.org, Oct 9

Issue description

* If open when the app switcher is used, hide the VK
* If an app with a selected text box input is chosen, open the VK

https://photos.app.goo.gl/3pPDyzjsxR5YMm6F8
 
Owner: shend@chromium.org
It need to be fixed in OS side.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 15

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

commit a643a13e0bc16fd0cab3a143ba841567efa7c67c
Author: Darren Shen <shend@chromium.org>
Date: Mon Oct 15 03:52:32 2018

[VK] Hide the virtual keyboard when we enter window overview mode.

When we enter window overview mode (e.g. by dragging the top of an app),
we need to hide the virtual keyboard because:

1) It blocks the overview mode UI for no reason.
2) The previews of windows have blank space at the bottom because that's
   where the keyboard is.

Bug:  893856 
Change-Id: I4f79f6bf603fd074a62b7ba29366ec5af18bbab2
Reviewed-on: https://chromium-review.googlesource.com/c/1278565
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599561}
[modify] https://crrev.com/a643a13e0bc16fd0cab3a143ba841567efa7c67c/ash/BUILD.gn
[modify] https://crrev.com/a643a13e0bc16fd0cab3a143ba841567efa7c67c/ash/wm/overview/window_selector_controller.cc
[add] https://crrev.com/a643a13e0bc16fd0cab3a143ba841567efa7c67c/ash/wm/overview/window_selector_controller_unittest.cc
[modify] https://crrev.com/a643a13e0bc16fd0cab3a143ba841567efa7c67c/testing/buildbot/filters/chromeos.single_process_mash.ash_unittests.filter
[modify] https://crrev.com/a643a13e0bc16fd0cab3a143ba841567efa7c67c/ui/keyboard/keyboard_controller.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 15

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

commit 1892e3dba74335b6df195fc606d5857b54d3f44a
Author: Darren Shen <shend@chromium.org>
Date: Mon Oct 15 07:27:42 2018

Revert "[VK] Hide the virtual keyboard when we enter window overview mode."

This reverts commit a643a13e0bc16fd0cab3a143ba841567efa7c67c.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [VK] Hide the virtual keyboard when we enter window overview mode.
> 
> When we enter window overview mode (e.g. by dragging the top of an app),
> we need to hide the virtual keyboard because:
> 
> 1) It blocks the overview mode UI for no reason.
> 2) The previews of windows have blank space at the bottom because that's
>    where the keyboard is.
> 
> Bug:  893856 
> Change-Id: I4f79f6bf603fd074a62b7ba29366ec5af18bbab2
> Reviewed-on: https://chromium-review.googlesource.com/c/1278565
> Commit-Queue: Darren Shen <shend@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599561}

TBR=jamescook@chromium.org,xdai@chromium.org,shend@chromium.org

Change-Id: Ie5ff6ccd70f24b40ba861c08de1467f87f7fcd62
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  893856 
Reviewed-on: https://chromium-review.googlesource.com/c/1280093
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599581}
[modify] https://crrev.com/1892e3dba74335b6df195fc606d5857b54d3f44a/ash/BUILD.gn
[modify] https://crrev.com/1892e3dba74335b6df195fc606d5857b54d3f44a/ash/wm/overview/window_selector_controller.cc
[delete] https://crrev.com/2bf0abb3e843c0b37e936e5be2f21d904222a9a9/ash/wm/overview/window_selector_controller_unittest.cc
[modify] https://crrev.com/1892e3dba74335b6df195fc606d5857b54d3f44a/testing/buildbot/filters/chromeos.single_process_mash.ash_unittests.filter
[modify] https://crrev.com/1892e3dba74335b6df195fc606d5857b54d3f44a/ui/keyboard/keyboard_controller.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 15

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

commit 8bc834f93ba33d416dc61fb9841b2aa92f091bbb
Author: Darren Shen <shend@chromium.org>
Date: Mon Oct 15 09:48:24 2018

Reland "[VK] Hide the virtual keyboard when we enter window overview mode."

This is a reland of a643a13e0bc16fd0cab3a143ba841567efa7c67c

Changed it to use HideKeyboardByUser instead of
HideKeyboardImplicitlyBySystem as we do not want to reshow the VK
if there's focus (see description of HideKeyboardImplicitlyBySystem).

A "proper" fix is to actually allow clients to specify whether an input
field should bring up the VK (like inputmode="none" in HTML). Then we
can just remove this special case code.

TBR=jamescook@chromium.org, xdai@chromium.org

Original change's description:
> [VK] Hide the virtual keyboard when we enter window overview mode.
>
> When we enter window overview mode (e.g. by dragging the top of an app),
> we need to hide the virtual keyboard because:
>
> 1) It blocks the overview mode UI for no reason.
> 2) The previews of windows have blank space at the bottom because that's
>    where the keyboard is.
>
> Bug:  893856 
> Change-Id: I4f79f6bf603fd074a62b7ba29366ec5af18bbab2
> Reviewed-on: https://chromium-review.googlesource.com/c/1278565
> Commit-Queue: Darren Shen <shend@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599561}

Bug:  893856 
Change-Id: Ibe9a1873cda3972520c75338db1b108d6274e005
Reviewed-on: https://chromium-review.googlesource.com/c/1280098
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599603}
[modify] https://crrev.com/8bc834f93ba33d416dc61fb9841b2aa92f091bbb/ash/BUILD.gn
[modify] https://crrev.com/8bc834f93ba33d416dc61fb9841b2aa92f091bbb/ash/wm/overview/window_selector_controller.cc
[add] https://crrev.com/8bc834f93ba33d416dc61fb9841b2aa92f091bbb/ash/wm/overview/window_selector_controller_unittest.cc
[modify] https://crrev.com/8bc834f93ba33d416dc61fb9841b2aa92f091bbb/testing/buildbot/filters/chromeos.single_process_mash.ash_unittests.filter
[modify] https://crrev.com/8bc834f93ba33d416dc61fb9841b2aa92f091bbb/ui/keyboard/keyboard_controller.cc

Status: Assigned (was: Available)
Hi Paul, I've fixed the first point you mentioned (hide the keyboard). The second part seems a bit more difficult. Can the second part be punted to a later milestone?
Labels: Merge-Request-71
Requesting merge to M71 for c#4. Thanks!
Project Member

Comment 7 by sheriffbot@chromium.org, Oct 16

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Reached out to kbleicher in email
Did #4 get testing prior to merge approval?
I tested locally and added unit tests.
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 Chrome OS.

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 17

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c7ff63d9e5c877cea49a178b2806269a93d3fba

commit 5c7ff63d9e5c877cea49a178b2806269a93d3fba
Author: Darren Shen <shend@chromium.org>
Date: Wed Oct 17 23:06:04 2018

[Merge M71] Reland "[VK] Hide the virtual keyboard when we enter window overview mode."

This is a reland of a643a13e0bc16fd0cab3a143ba841567efa7c67c

Changed it to use HideKeyboardByUser instead of
HideKeyboardImplicitlyBySystem as we do not want to reshow the VK
if there's focus (see description of HideKeyboardImplicitlyBySystem).

A "proper" fix is to actually allow clients to specify whether an input
field should bring up the VK (like inputmode="none" in HTML). Then we
can just remove this special case code.

TBR=jamescook@chromium.org, xdai@chromium.org

Original change's description:
> [VK] Hide the virtual keyboard when we enter window overview mode.
>
> When we enter window overview mode (e.g. by dragging the top of an app),
> we need to hide the virtual keyboard because:
>
> 1) It blocks the overview mode UI for no reason.
> 2) The previews of windows have blank space at the bottom because that's
>    where the keyboard is.
>
> Bug:  893856 
> Change-Id: I4f79f6bf603fd074a62b7ba29366ec5af18bbab2
> Reviewed-on: https://chromium-review.googlesource.com/c/1278565
> Commit-Queue: Darren Shen <shend@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599561}

Bug:  893856 
Change-Id: Ibe9a1873cda3972520c75338db1b108d6274e005
Reviewed-on: https://chromium-review.googlesource.com/c/1280098
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599603}(cherry picked from commit 8bc834f93ba33d416dc61fb9841b2aa92f091bbb)
Reviewed-on: https://chromium-review.googlesource.com/c/1287329
Cr-Commit-Position: refs/branch-heads/3578@{#107}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/5c7ff63d9e5c877cea49a178b2806269a93d3fba/ash/BUILD.gn
[modify] https://crrev.com/5c7ff63d9e5c877cea49a178b2806269a93d3fba/ash/wm/overview/window_selector_controller.cc
[add] https://crrev.com/5c7ff63d9e5c877cea49a178b2806269a93d3fba/ash/wm/overview/window_selector_controller_unittest.cc
[modify] https://crrev.com/5c7ff63d9e5c877cea49a178b2806269a93d3fba/testing/buildbot/filters/chromeos.single_process_mash.ash_unittests.filter
[modify] https://crrev.com/5c7ff63d9e5c877cea49a178b2806269a93d3fba/ui/keyboard/keyboard_controller.cc

Status: Fixed (was: Assigned)
Thanks!
Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/5c7ff63d9e5c877cea49a178b2806269a93d3fba

Commit: 5c7ff63d9e5c877cea49a178b2806269a93d3fba
Author: shend@chromium.org
Commiter: shend@chromium.org
Date: 2018-10-17 23:06:04 +0000 UTC

[Merge M71] Reland "[VK] Hide the virtual keyboard when we enter window overview mode."

This is a reland of a643a13e0bc16fd0cab3a143ba841567efa7c67c

Changed it to use HideKeyboardByUser instead of
HideKeyboardImplicitlyBySystem as we do not want to reshow the VK
if there's focus (see description of HideKeyboardImplicitlyBySystem).

A "proper" fix is to actually allow clients to specify whether an input
field should bring up the VK (like inputmode="none" in HTML). Then we
can just remove this special case code.

TBR=jamescook@chromium.org, xdai@chromium.org

Original change's description:
> [VK] Hide the virtual keyboard when we enter window overview mode.
>
> When we enter window overview mode (e.g. by dragging the top of an app),
> we need to hide the virtual keyboard because:
>
> 1) It blocks the overview mode UI for no reason.
> 2) The previews of windows have blank space at the bottom because that's
>    where the keyboard is.
>
> Bug:  893856 
> Change-Id: I4f79f6bf603fd074a62b7ba29366ec5af18bbab2
> Reviewed-on: https://chromium-review.googlesource.com/c/1278565
> Commit-Queue: Darren Shen <shend@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599561}

Bug:  893856 
Change-Id: Ibe9a1873cda3972520c75338db1b108d6274e005
Reviewed-on: https://chromium-review.googlesource.com/c/1280098
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599603}(cherry picked from commit 8bc834f93ba33d416dc61fb9841b2aa92f091bbb)
Reviewed-on: https://chromium-review.googlesource.com/c/1287329
Cr-Commit-Position: refs/branch-heads/3578@{#107}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment