New issue
Advanced search Search tips

Issue 826411 link

Starred by 1 user
Status: Fixed
Owner:
est...@chromium.org
Closed: Apr 2018
Cc:
afakhry@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 678705



Sign in to add a comment

mash: Remove ash accelerator_table.h access in chrome

Project Member Reported by jamescook@chromium.org, Mar 27 2018 
It's used for 3 things:

* To look up the keyboard shortcut for restore tab (bizarre, we should just hard-code Ctrl-Shift-T)
* To show shortcut text on menus (see chrome/browser/ui/views/accelerator_table.cc)
* Support an extension API for keyboard shortcuts

Either these need to be refactored away or we need a mojo API to query accelerator data from the ash process. The former would be better, I think.

Comment 1 by jamescook@chromium.org, Mar 27 2018

Blocking: 678705

Comment 2 by jamescook@chromium.org, Mar 27 2018 

Also used to support "ProcessAcceleratorWhileMenuShowing" in ChromeViewsDelegate

ui/views/chrome_views_delegate_chromeos.cc:#include "ash/accelerators/accelerator_controller.h"
ui/views/chrome_views_delegate_chromeos.cc:#include "ash/shell.h"

Comment 3 by est...@chromium.org, Apr 4 2018

Owner: est...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 12 2018 

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

commit db110f78b92a0f802b0c90e55555dd9120a35a07
Author: Evan Stade <estade@chromium.org>
Date: Thu Apr 12 19:36:24 2018

Remove references to ash/accelerators/accelerator_table.h from chrome/.

Move the accelerator data to ash/public for those that need it and remove
references that are redundant.

Bug:  826411 
Change-Id: I3c20bc590dd9b3c1920dde65431dad09cdc59da4
Reviewed-on: https://chromium-review.googlesource.com/996473
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550312}
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/accelerators/accelerator_table.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/accelerators/accelerator_table.h
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/content/keyboard_overlay/keyboard_overlay_view_unittest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/public/cpp/BUILD.gn
[add] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/public/cpp/accelerators.cc
[add] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/public/cpp/accelerators.h
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/chromeos/accessibility/spoken_feedback_browsertest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/extensions/api/commands/command_service.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/accelerator_utils.h
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/ash/ksv/DEPS
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_metadata_unittest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/cocoa/accelerator_utils_cocoa.mm
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/extensions/accelerator_priority.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/accelerator_table.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/accelerator_table_unittest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/accelerator_utils_aura.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/webui/chromeos/DEPS
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/webui/chromeos/keyboard_overlay_ui_browsertest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/db110f78b92a0f802b0c90e55555dd9120a35a07

commit db110f78b92a0f802b0c90e55555dd9120a35a07
Author: Evan Stade <estade@chromium.org>
Date: Thu Apr 12 19:36:24 2018

Remove references to ash/accelerators/accelerator_table.h from chrome/.

Move the accelerator data to ash/public for those that need it and remove
references that are redundant.

Bug:  826411 
Change-Id: I3c20bc590dd9b3c1920dde65431dad09cdc59da4
Reviewed-on: https://chromium-review.googlesource.com/996473
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550312}
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/accelerators/accelerator_table.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/accelerators/accelerator_table.h
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/content/keyboard_overlay/keyboard_overlay_view_unittest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/public/cpp/BUILD.gn
[add] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/public/cpp/accelerators.cc
[add] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/ash/public/cpp/accelerators.h
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/chromeos/accessibility/spoken_feedback_browsertest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/extensions/api/commands/command_service.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/accelerator_utils.h
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/ash/ksv/DEPS
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/ash/ksv/keyboard_shortcut_viewer_metadata_unittest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/bookmarks/enhanced_bookmark_key_service.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/browser_commands.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/cocoa/accelerator_utils_cocoa.mm
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/extensions/accelerator_priority.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/toolbar/recent_tabs_sub_menu_model.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/accelerator_table.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/accelerator_table_unittest.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/accelerator_utils_aura.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/webui/chromeos/DEPS
[modify] https://crrev.com/db110f78b92a0f802b0c90e55555dd9120a35a07/chrome/browser/ui/webui/chromeos/keyboard_overlay_ui_browsertest.cc

Comment 6 by est...@chromium.org, Apr 18 2018

Status: Fixed (was: Started)
►

Sign in to add a comment