ExclusiveAccessBubbleViews requires |context| to provide an accelerator, but sometimes ignores it |
|||
Issue description
Chrome Version : 63.0.3213.3
ExclusiveAccessBubbleViews does
// Create the contents view.
ui::Accelerator accelerator(ui::VKEY_UNKNOWN, ui::EF_NONE);
bool got_accelerator =
bubble_view_context_->GetAcceleratorProvider()
->GetAcceleratorForCommandId(IDC_FULLSCREEN, &accelerator);
DCHECK(got_accelerator);
view_ = new SubtleNotificationView(this);
browser_fullscreen_exit_accelerator_ = accelerator.GetShortcutText();
So it requires
- GetAcceleratorProvider() to return non-null, and
- that provider to provide an accelerator to populate |browser_fullscreen_exit_accelerator_|
But later on, |browser_fullscreen_exit_accelerator_| may be ignored:
if (bubble_type ==
EXCLUSIVE_ACCESS_BUBBLE_TYPE_BROWSER_FULLSCREEN_EXIT_INSTRUCTION ||
bubble_type ==
EXCLUSIVE_ACCESS_BUBBLE_TYPE_EXTENSION_FULLSCREEN_EXIT_INSTRUCTION) {
accelerator = browser_fullscreen_exit_accelerator_;
} else {
accelerator = l10n_util::GetStringUTF16(IDS_APP_ESC_KEY);
if (bubble_type !=
EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION) {
link_visible = false;
}
}
i.e., it's only used for browser/extension fullscreen, but not OS/immersive fullscreen modes, which just clobber |browser_fullscreen_exit_accelerator_| with IDS_APP_ESC_KEY
This came up in https://chromium-review.googlesource.com/c/chromium/src/+/648985/13/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc#462
(there's probably a lot of cleanups around these bubbles once we can ditch the non-simplified codepaths and the chrome://flag).
,
Jan 9 2018
,
Sep 13
Archiving old bugs that haven't been actively assigned in over 180 days. If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!
,
Sep 13
Archiving old bugs that haven't been actively assigned in over 180 days. If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks! |
|||
►
Sign in to add a comment |
|||
Comment 1 by mgiuca@chromium.org
, Sep 22 2017