New issue
Advanced search Search tips

Issue 742613 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug-Regression



Sign in to add a comment

Caps Lock remapping setting not shown on Chromebooks with built-in Caps Lock keys

Project Member Reported by derat@chromium.org, Jul 13 2017

Issue description

There's a legacy_keyboard USE flag that we set for Chromeboxes, Chromebases, and Chromebooks with non-standard keyboards (parrot, stout, etc.). session_manager's chrome_setup.cc passes a --has-chromeos-keyboard flag to Chrome if legacy_keyboard is unset.

chrome/browser/resources/options/chromeos/keyboard_overlay.html still has a reference to the flag; we used to show a Caps Lock remapping setting when the flag was missing. https://codereview.chromium.org/1269793005 removed Chrome's handling of the flag for  issue 501121 , but I'm not sure of the reason why.

https://crrev.com/1188693002 made Chrome only show the Caps Lock setting when an external keyboard is connected for  issue 167237  (which was initially talking about Chromebooks with legacy keyboards, not external keyboards). Were Chromebooks with built-in Caps Lock keys overlooked when this change was made?

I think we should show the Caps Lock remapping when an external keyboard is connected *or* when --has-chromeos-keyboard was not passed. Am I missing something?
 
You're right. I'm not sure why wanted to remove the --has-chromeos-keyboard handling. My memory is quite foggy regarding this issue, but by looking at the proposal in https://bugs.chromium.org/p/chromium/issues/detail?id=167237#c17, I think what we did was:
- Assume --has-chromeos-keyboard is true on all devices, except devices such as chromeboxes ... etc, which are likely to have external keyboards, for which we display the capslock remapping setting. So for that, we decided to remove the flag and just detect the presence of an external keyboard.

Comment 2 by derat@chromium.org, Jul 13 2017

Status: Started (was: Assigned)
I'm going to make the change described above.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18 2017

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

commit 4dff8652057a92baab5770904c002840f7a6b00d
Author: Daniel Erat <derat@chromium.org>
Date: Tue Jul 18 00:06:17 2017

chromeos: Display Caps Lock setting for non-CrOS keyboards.

Let the user remap the Caps Lock key when
--has-chromeos-keyboard isn't provided (i.e. the internal
keyboard has a Caps Lock key in place of Search). Formerly,
the setting was only shown when an external keyboard was
connected.

Also add unit tests for KeyboardHandler and delete some
unused variables. Unrelatedly, drop an accidentally-repeated
call in PowerHandlerTest.

BUG= 742613 

Change-Id: I56d7ccb0bd5e054b9e815fb4d8d457c0729ada0c
Reviewed-on: https://chromium-review.googlesource.com/572209
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487298}
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chrome/browser/chromeos/login/chrome_restart_request.cc
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chrome/browser/ui/webui/settings/chromeos/device_keyboard_handler.cc
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chrome/browser/ui/webui/settings/chromeos/device_keyboard_handler.h
[add] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chrome/browser/ui/webui/settings/chromeos/device_keyboard_handler_unittest.cc
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chrome/browser/ui/webui/settings/chromeos/device_power_handler_unittest.cc
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chromeos/chromeos_switches.cc
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/chromeos/chromeos_switches.h
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/ui/events/test/device_data_manager_test_api.h
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/ui/events/test/device_data_manager_test_api_impl.cc
[modify] https://crrev.com/4dff8652057a92baab5770904c002840f7a6b00d/ui/events/test/device_data_manager_test_api_stub.cc

Comment 4 by derat@chromium.org, Jul 18 2017

Labels: M-61
Status: Fixed (was: Started)
Fix should go out in M61.

Comment 5 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment