From 16125bdbbd1840dfe1b1668fd2003af44b0b24a4 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 23 Feb 2017 15:24:45 -0500 Subject: [PATCH] views: Support accelerators in MenuButton label (issue #2102) --- include/internal/cef_string_types.h | 11 +++ libcef/browser/views/menu_button_impl.cc | 9 +++ libcef/browser/views/menu_button_impl.h | 3 + libcef/browser/views/menu_button_view.cc | 13 +++ libcef/browser/views/menu_button_view.h | 5 ++ libcef/common/string_types_impl.cc | 13 +++ patch/patch.cfg | 1 + patch/patches/views_menu_2102.patch | 97 +++++++++++++++++++++++ tests/cefclient/browser/views_menu_bar.cc | 61 +++++++++++++- tests/cefclient/browser/views_menu_bar.h | 11 ++- tests/cefclient/browser/views_window.cc | 7 +- 11 files changed, 226 insertions(+), 5 deletions(-) diff --git a/include/internal/cef_string_types.h b/include/internal/cef_string_types.h index ab21fe84c..062065709 100644 --- a/include/internal/cef_string_types.h +++ b/include/internal/cef_string_types.h @@ -185,6 +185,17 @@ CEF_EXPORT void cef_string_userfree_utf8_free(cef_string_userfree_utf8_t str); CEF_EXPORT void cef_string_userfree_utf16_free(cef_string_userfree_utf16_t str); +/// +// These functions convert utf16 string case using the current ICU locale. This +// may change the length of the string in some cases. +/// + +CEF_EXPORT int cef_string_utf16_to_lower(const char16* src, size_t src_len, + cef_string_utf16_t* output); +CEF_EXPORT int cef_string_utf16_to_upper(const char16* src, size_t src_len, + cef_string_utf16_t* output); + + #ifdef __cplusplus } #endif diff --git a/libcef/browser/views/menu_button_impl.cc b/libcef/browser/views/menu_button_impl.cc index d5b2d8663..4e7ef9478 100644 --- a/libcef/browser/views/menu_button_impl.cc +++ b/libcef/browser/views/menu_button_impl.cc @@ -7,6 +7,8 @@ #include "libcef/browser/views/menu_button_view.h" #include "libcef/browser/views/window_impl.h" +#include "ui/gfx/canvas.h" + // static CefRefPtr CefMenuButton::CreateMenuButton( CefRefPtr delegate, @@ -55,6 +57,13 @@ void CefMenuButtonImpl::TriggerMenu() { root_view()->Activate(nullptr); } +void CefMenuButtonImpl::SetFocusable(bool focusable) { + CEF_REQUIRE_VALID_RETURN_VOID(); + static_cast(root_view())->SetDrawStringsFlags( + focusable ? gfx::Canvas::SHOW_PREFIX : gfx::Canvas::HIDE_PREFIX); + ParentClass::SetFocusable(focusable); +} + CefMenuButtonImpl::CefMenuButtonImpl(CefRefPtr delegate) : ParentClass(delegate) { DCHECK(delegate); diff --git a/libcef/browser/views/menu_button_impl.h b/libcef/browser/views/menu_button_impl.h index 2ef1e88eb..3f4a7d858 100644 --- a/libcef/browser/views/menu_button_impl.h +++ b/libcef/browser/views/menu_button_impl.h @@ -40,6 +40,9 @@ class CefMenuButtonImpl : // CefViewAdapter methods: std::string GetDebugType() override { return "MenuButton"; } + // CefView methods: + void SetFocusable(bool focusable) override; + private: // Create a new implementation object. // Always call Initialize() after creation. diff --git a/libcef/browser/views/menu_button_view.cc b/libcef/browser/views/menu_button_view.cc index 5b94632cc..0de13fa94 100644 --- a/libcef/browser/views/menu_button_view.cc +++ b/libcef/browser/views/menu_button_view.cc @@ -4,18 +4,31 @@ #include "libcef/browser/views/menu_button_view.h" +#include "ui/gfx/canvas.h" + CefMenuButtonView::CefMenuButtonView( CefMenuButtonDelegate* cef_delegate) : ParentClass(cef_delegate) { DCHECK(cef_delegate); } +void CefMenuButtonView::Initialize() { + ParentClass::Initialize(); + + SetDrawStringsFlags(IsFocusable() ? gfx::Canvas::SHOW_PREFIX : + gfx::Canvas::HIDE_PREFIX); +} + CefRefPtr CefMenuButtonView::GetCefMenuButton() const { CefRefPtr menu_button = GetCefLabelButton()->AsMenuButton(); DCHECK(menu_button); return menu_button; } +void CefMenuButtonView::SetDrawStringsFlags(int flags) { + label()->SetDrawStringsFlags(flags); +} + void CefMenuButtonView::OnMenuButtonClicked(views::MenuButton* source, const gfx::Point& point, const ui::Event* event) { diff --git a/libcef/browser/views/menu_button_view.h b/libcef/browser/views/menu_button_view.h index 714bda50f..569928c4e 100644 --- a/libcef/browser/views/menu_button_view.h +++ b/libcef/browser/views/menu_button_view.h @@ -36,10 +36,15 @@ class CefMenuButtonView : // |cef_delegate| must not be nullptr. explicit CefMenuButtonView(CefMenuButtonDelegate* cef_delegate); + void Initialize() override; + // Returns the CefMenuButton associated with this view. See comments on // CefViewView::GetCefView. CefRefPtr GetCefMenuButton() const; + // Set the flags that control display of accelerator characters. + void SetDrawStringsFlags(int flags); + // views::MenuButtonListener methods: void OnMenuButtonClicked(views::MenuButton* source, const gfx::Point& point, diff --git a/libcef/common/string_types_impl.cc b/libcef/common/string_types_impl.cc index a893c4e2b..ebc0051cc 100644 --- a/libcef/common/string_types_impl.cc +++ b/libcef/common/string_types_impl.cc @@ -4,6 +4,7 @@ #include "include/internal/cef_string_types.h" #include +#include "base/i18n/case_conversion.h" #include "base/logging.h" #include "base/strings/string16.h" #include "base/strings/string_util.h" @@ -275,3 +276,15 @@ CEF_EXPORT void cef_string_userfree_utf16_free( cef_string_utf16_clear(str); delete str; } + +CEF_EXPORT int cef_string_utf16_to_lower(const char16* src, size_t src_len, + cef_string_utf16_t* output) { + const base::string16& str = base::i18n::ToLower(base::string16(src, src_len)); + return cef_string_utf16_set(str.c_str(), str.length(), output, true); +} + +CEF_EXPORT int cef_string_utf16_to_upper(const char16* src, size_t src_len, + cef_string_utf16_t* output) { + const base::string16& str = base::i18n::ToUpper(base::string16(src, src_len)); + return cef_string_utf16_set(str.c_str(), str.length(), output, true); +} diff --git a/patch/patch.cfg b/patch/patch.cfg index d897a2541..04f55dbb1 100644 --- a/patch/patch.cfg +++ b/patch/patch.cfg @@ -329,6 +329,7 @@ patches = [ }, { # Expose callbacks for mouse/keyboard events that trigger menu switching. + # Add accelerator display support to Label. # https://bitbucket.org/chromiumembedded/cef/issues/2102 'name': 'views_menu_2102', 'path': '../', diff --git a/patch/patches/views_menu_2102.patch b/patch/patches/views_menu_2102.patch index 10630916d..3d93a5f26 100644 --- a/patch/patches/views_menu_2102.patch +++ b/patch/patches/views_menu_2102.patch @@ -26,6 +26,103 @@ index 0755f27..72db677 100644 // Called when the menu is about to be shown. virtual void MenuWillShow() {} +diff --git ui/views/controls/label.cc ui/views/controls/label.cc +index 9b04c09..5d99c81 100644 +--- ui/views/controls/label.cc ++++ ui/views/controls/label.cc +@@ -28,6 +28,7 @@ + #include "ui/gfx/color_utils.h" + #include "ui/gfx/geometry/insets.h" + #include "ui/gfx/text_elider.h" ++#include "ui/gfx/text_utils.h" + #include "ui/native_theme/native_theme.h" + #include "ui/strings/grit/ui_strings.h" + #include "ui/views/controls/menu/menu_runner.h" +@@ -36,6 +37,25 @@ + #include "ui/views/selection_controller.h" + + namespace views { ++ ++namespace { ++ ++// Strips accelerator character prefixes in |text| if needed, based on |flags|. ++// Returns a range in |text| to underline or Range::InvalidRange() if ++// underlining is not needed. ++gfx::Range StripAcceleratorChars(int flags, base::string16* text) { ++ if (flags & (gfx::Canvas::SHOW_PREFIX | gfx::Canvas::HIDE_PREFIX)) { ++ int char_pos = -1; ++ int char_span = 0; ++ *text = gfx::RemoveAcceleratorChar(*text, '&', &char_pos, &char_span); ++ if ((flags & gfx::Canvas::SHOW_PREFIX) && char_pos != -1) ++ return gfx::Range(char_pos, char_pos + char_span); ++ } ++ return gfx::Range::InvalidRange(); ++} ++ ++} // namespace ++ + // static + const char Label::kViewClassName[] = "Label"; + const int Label::kFocusBorderPadding = 1; +@@ -210,6 +230,14 @@ void Label::SetElideBehavior(gfx::ElideBehavior elide_behavior) { + ResetLayout(); + } + ++void Label::SetDrawStringsFlags(int flags) { ++ if (draw_strings_flags_ == flags) ++ return; ++ is_first_paint_text_ = true; ++ draw_strings_flags_ = flags; ++ ResetLayout(); ++} ++ + void Label::SetTooltipText(const base::string16& tooltip_text) { + DCHECK(handles_tooltips_); + tooltip_text_ = tooltip_text; +@@ -440,7 +468,19 @@ std::unique_ptr Label::CreateRenderText( + render_text->SetFontList(font_list()); + render_text->set_shadows(shadows()); + render_text->SetCursorEnabled(false); +- render_text->SetText(text); ++ ++ if (draw_strings_flags_ != 0) { ++ base::string16 text_str = text; ++ gfx::Range range = StripAcceleratorChars(draw_strings_flags_, &text_str); ++ render_text->SetText(text_str); ++ if (range.IsValid()) { ++ render_text->SetDisplayRect(bounds()); ++ render_text->ApplyStyle(gfx::UNDERLINE, true, range); ++ } ++ } else { ++ render_text->SetText(text); ++ } ++ + return render_text; + } + +diff --git ui/views/controls/label.h ui/views/controls/label.h +index 6293cff..d0a5a8f 100644 +--- ui/views/controls/label.h ++++ ui/views/controls/label.h +@@ -117,6 +117,10 @@ class VIEWS_EXPORT Label : public View, + void SetElideBehavior(gfx::ElideBehavior elide_behavior); + gfx::ElideBehavior elide_behavior() const { return elide_behavior_; } + ++ // Get or set the flags that control display of accelerator characters. ++ void SetDrawStringsFlags(int flags); ++ int draw_strings_flags() const { return draw_strings_flags_; } ++ + // Sets the tooltip text. Default behavior for a label (single-line) is to + // show the full text if it is wider than its bounds. Calling this overrides + // the default behavior and lets you set a custom tooltip. To revert to +@@ -333,6 +337,7 @@ class VIEWS_EXPORT Label : public View, + bool collapse_when_hidden_; + int fixed_width_; + int max_width_; ++ int draw_strings_flags_ = 0; + + // TODO(ckocagil): Remove is_first_paint_text_ before crbug.com/441028 is + // closed. diff --git ui/views/controls/menu/menu_controller.cc ui/views/controls/menu/menu_controller.cc index 79ff77c..a0582c0 100644 --- ui/views/controls/menu/menu_controller.cc diff --git a/tests/cefclient/browser/views_menu_bar.cc b/tests/cefclient/browser/views_menu_bar.cc index 06b291cef..23fadc1a6 100644 --- a/tests/cefclient/browser/views_menu_bar.cc +++ b/tests/cefclient/browser/views_menu_bar.cc @@ -13,6 +13,32 @@ namespace { const int kMenuBarGroupId = 100; +// Convert |c| to lowercase using the current ICU locale. +// TODO(jshin): What about Turkish locale? See http://crbug.com/81719. +// If the mnemonic is capital I and the UI language is Turkish, lowercasing it +// results in 'small dotless i', which is different from a 'dotted i'. Similar +// issues may exist for az and lt locales. +base::char16 ToLower(base::char16 c) { + CefStringUTF16 str16; + cef_string_utf16_to_lower(&c, 1, str16.GetWritableStruct()); + return str16.length() > 0 ? str16.c_str()[0] : 0; +} + +// Extract the mnemonic character from |title|. For example, if |title| is +// "&Test" then the mnemonic character is 'T'. +base::char16 GetMnemonic(const base::string16& title) { + size_t index = 0; + do { + index = title.find('&', index); + if (index != base::string16::npos) { + if (index + 1 != title.size() && title[index + 1] != '&') + return ToLower(title[index + 1]); + index++; + } + } while (index != base::string16::npos); + return 0; +} + } // namespace ViewsMenuBar::ViewsMenuBar(Delegate* delegate, @@ -34,7 +60,7 @@ CefRefPtr ViewsMenuBar::GetMenuPanel() { return panel_; } -CefRefPtr ViewsMenuBar::CreateMenuModel(const std::string& label, +CefRefPtr ViewsMenuBar::CreateMenuModel(const CefString& label, int* menu_id) { EnsureMenuPanel(); @@ -58,7 +84,12 @@ CefRefPtr ViewsMenuBar::CreateMenuModel(const std::string& label, // Add the new MenuButton to the Planel. panel_->AddChildView(button); - + + // Extract the mnemonic that triggers the menu, if any. + base::char16 mnemonic = GetMnemonic(label); + if (mnemonic != 0) + mnemonics_.insert(std::make_pair(mnemonic, new_menu_id)); + return model; } @@ -81,9 +112,35 @@ void ViewsMenuBar::SetMenuFocusable(bool focusable) { } } +bool ViewsMenuBar::OnKeyEvent(const CefKeyEvent& event) { + if (!panel_) + return false; + + if (event.type != KEYEVENT_RAWKEYDOWN) + return false; + + // Do not check mnemonics if the Alt or Ctrl modifiers are pressed. For + // example Ctrl+ is an accelerator, but only is a mnemonic. + if (event.modifiers & (EVENTFLAG_ALT_DOWN | EVENTFLAG_CONTROL_DOWN)) + return false; + + MnemonicMap::const_iterator it = mnemonics_.find(ToLower(event.character)); + if (it == mnemonics_.end()) + return false; + + // Set status indicating that we navigated using the keyboard. + last_nav_with_keyboard_ = true; + + // Show the selected menu. + TriggerMenuButton(panel_->GetViewForID(it->second)); + + return true; +} + void ViewsMenuBar::Reset() { panel_ = NULL; models_.clear(); + mnemonics_.clear(); id_next_ = id_start_; } diff --git a/tests/cefclient/browser/views_menu_bar.h b/tests/cefclient/browser/views_menu_bar.h index c24ef54c5..0d146261d 100644 --- a/tests/cefclient/browser/views_menu_bar.h +++ b/tests/cefclient/browser/views_menu_bar.h @@ -6,6 +6,7 @@ #define CEF_TESTS_CEFCLIENT_BROWSER_VIEWS_MENU_BAR_H_ #pragma once +#include #include #include @@ -50,7 +51,7 @@ class ViewsMenuBar : public CefMenuButtonDelegate, // Create a new menu with the specified |label|. If |menu_id| is non-NULL it // will be populated with the new menu ID. - CefRefPtr CreateMenuModel(const std::string& label, + CefRefPtr CreateMenuModel(const CefString& label, int* menu_id); // Returns the menu with the specified |menu_id|, or NULL if no such menu @@ -63,6 +64,10 @@ class ViewsMenuBar : public CefMenuButtonDelegate, // when a control not in the menu bar gains focus. void SetMenuFocusable(bool focusable); + // Key events forwarded from ViewsWindow::OnKeyEvent when the menu bar has + // focus. + bool OnKeyEvent(const CefKeyEvent& event); + // Reset menu bar state. void Reset(); @@ -107,6 +112,10 @@ class ViewsMenuBar : public CefMenuButtonDelegate, std::vector > models_; bool last_nav_with_keyboard_; + // Map of mnemonic to MenuButton ID. + typedef std::map MnemonicMap; + MnemonicMap mnemonics_; + IMPLEMENT_REFCOUNTING(ViewsMenuBar); DISALLOW_COPY_AND_ASSIGN(ViewsMenuBar); }; diff --git a/tests/cefclient/browser/views_window.cc b/tests/cefclient/browser/views_window.cc index 639761de2..6ef7e482c 100644 --- a/tests/cefclient/browser/views_window.cc +++ b/tests/cefclient/browser/views_window.cc @@ -457,6 +457,9 @@ bool ViewsWindow::OnKeyEvent(CefRefPtr window, return true; } + if (menu_has_focus_ && top_menu_bar_) + return top_menu_bar_->OnKeyEvent(event); + return false; } @@ -525,8 +528,8 @@ void ViewsWindow::CreateMenuModel() { if (top_menu_bar_) { // Add the menus to the top menu bar. - AddFileMenuItems(top_menu_bar_->CreateMenuModel("File", NULL), 0); - AddTestMenuItems(top_menu_bar_->CreateMenuModel("Tests", NULL)); + AddFileMenuItems(top_menu_bar_->CreateMenuModel("&File", NULL), 0); + AddTestMenuItems(top_menu_bar_->CreateMenuModel("&Tests", NULL)); } }