Commit 915a3854 authored by mlcui's avatar mlcui Committed by Commit Bot

Shortcut viewer: Minor fixes to |shortcut_message_id| logic

This fixes two issues:

- If |has_invalid_dom_key| is set and |item.shortcut_message_id| was
  missing, the old logic would incorrectly DCHECK instead of displaying
  the "no mapping" string.
- Dynamically changing the length of replacement strings does not change
  the shortcut message used. This isn't a problem with the current
  implementation, as the length of replacement strings is static, but
  may be a problem if replacement strings are dynamically chosen in the
  future.

Change-Id: Idfa355b68e29508125e2278dac83a60f7a215807
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513307
Commit-Queue: Michael Cui <mlcui@google.com>
Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823833}
parent 7abb9382
...@@ -102,14 +102,18 @@ KeyboardShortcutItemView::KeyboardShortcutItemView( ...@@ -102,14 +102,18 @@ KeyboardShortcutItemView::KeyboardShortcutItemView(
} }
int shortcut_message_id; int shortcut_message_id;
if (item.shortcut_message_id) { if (has_invalid_dom_key) {
// |shortcut_message_id| should never be used if the shortcut is not
// supported on the current keyboard layout.
shortcut_message_id = -1;
} else if (item.shortcut_message_id) {
shortcut_message_id = *item.shortcut_message_id; shortcut_message_id = *item.shortcut_message_id;
} else { } else {
// Automatically determine the shortcut message based on the number of // Automatically determine the shortcut message based on the number of
// shortcut_key_codes. // replacement strings.
// As there are separators inserted between the modifiers, a shortcut with // As there are separators inserted between the modifiers, a shortcut with
// N modifiers has 2*N + 1 shortcut_key_codes. // N modifiers has 2*N + 1 replacement strings.
switch (item.shortcut_key_codes.size()) { switch (replacement_strings.size()) {
case 1: case 1:
shortcut_message_id = IDS_KSV_SHORTCUT_ONE_KEY; shortcut_message_id = IDS_KSV_SHORTCUT_ONE_KEY;
break; break;
...@@ -124,7 +128,7 @@ KeyboardShortcutItemView::KeyboardShortcutItemView( ...@@ -124,7 +128,7 @@ KeyboardShortcutItemView::KeyboardShortcutItemView(
break; break;
default: default:
NOTREACHED() << "Automatically determined shortcut has " NOTREACHED() << "Automatically determined shortcut has "
<< item.shortcut_key_codes.size() << " key codes."; << replacement_strings.size() << " replacement strings.";
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment