Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Before CL Shift+F10 won't open Context Menu when NumLock or CapsLock is
on, which is confusing and does not match ContextMenu key nor other browsers.

e.g.
1. On Chrome NumLock+ContextMenu opens CM
2. On Firefox NumLock+Shift+F10 opens CM
But on Chrome NumLock+Shift+F10 won't open CM.

After CL Chrome will match Firefox, Windows Notepad, IE, plus ContextMenu key's
behavior.

BUG=6941
Committed: https://crrev.com/9ca5ec119dad81a9653712ced1efd2ab9a9539e2
Cr-Commit-Position: refs/heads/master@{#404194}

[chongz, 2016-07-06 16:57:45]

Patchset #1 (id:1) has been deleted

[chongz, 2016-07-06 17:08:47]

Description was changed from

==========
Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Before CL Shift+F10 won't open Context Menu when NumLock or CapsLock is
on, which is confusing and does not match ContextMenu key nor other browsers.

e.g.
1. On Chrome NumLock+ContextMenu opens CM
2. On Firefox NumLock+Shift+F10 opens CM

After CL Chrome will match Firefox, Windows Notepad, IE, plus ContextMenu key's
behavior.

BUG=6941
==========

to

==========
Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Before CL Shift+F10 won't open Context Menu when NumLock or CapsLock is
on, which is confusing and does not match ContextMenu key nor other browsers.

e.g.
1. On Chrome NumLock+ContextMenu opens CM
2. On Firefox NumLock+Shift+F10 opens CM
But on Chrome NumLock+Shift+F10 won't open CM.

After CL Chrome will match Firefox, Windows Notepad, IE, plus ContextMenu key's
behavior.

BUG=6941
==========

[chongz, 2016-07-06 17:11:41]

chongz@chromium.org changed reviewers:
+ bokan@chromium.org

[chongz, 2016-07-06 17:11:42]

bokan@ PTAL at this Context Menu issue, thanks!

[bokan, 2016-07-06 18:01:49]

lgtm! (Great test!) :)

[chongz, 2016-07-06 18:06:38]

On 2016/07/06 18:01:49, bokan wrote:
> lgtm! (Great test!) :)

Thx! :)

[chongz, 2016-07-06 18:06:43]

The CQ bit was checked by chongz@chromium.org

[commit-bot: I haz the power, 2016-07-06 18:07:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bokan, 2016-07-06 18:21:11]

https://codereview.chromium.org/2122053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/2122053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:841: bool isShiftF10 =
(event.modifiers & WebInputEvent::InputModifiers) == WebInputEvent::ShiftKey &&
event.windowsKeyCode == VKEY_F10;
Actually, before you submit, how does this work? If Shift + NumLock are down:
event.modifiers & InputModifiers == ShiftKey & NumLock != ShiftKey so isShiftF10
will be false.

Should this instead be: (event.modifiers & ShiftKey) && !(event.modifiers &
~InputModifiers)? i.e. The shift key is set and no modifiers outside
InputModifiers are set. Or am I misunderstanding something?

[bokan, 2016-07-06 18:23:30]

https://codereview.chromium.org/2122053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/2122053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:841: bool isShiftF10 =
(event.modifiers & WebInputEvent::InputModifiers) == WebInputEvent::ShiftKey &&
event.windowsKeyCode == VKEY_F10;
On 2016/07/06 18:21:11, bokan wrote:
> Actually, before you submit, how does this work? If Shift + NumLock are down:
> event.modifiers & InputModifiers == ShiftKey & NumLock != ShiftKey so
isShiftF10
> will be false.
> 
> Should this instead be: (event.modifiers & ShiftKey) && !(event.modifiers &
> ~InputModifiers)? i.e. The shift key is set and no modifiers outside
> InputModifiers are set. Or am I misunderstanding something?

nvm...just realized Num/Caps lock aren't in InputModifiers. carry on...

[chongz, 2016-07-06 18:30:34]

https://codereview.chromium.org/2122053003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp (right):

https://codereview.chromium.org/2122053003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebFrameWidgetImpl.cpp:840: bool
isUnmodifiedMenuKey = !(event.modifiers & WebInputEvent::InputModifiers) &&
event.windowsKeyCode == VKEY_APPS;
Ya actually it's doing the same thing as this line, the original author seems
forgot ShiftF10...

See:
https://chromium.googlesource.com/chromium/src/+/0bbd1cc7fdf21066699f1c5fd870...

[commit-bot: I haz the power, 2016-07-06 22:04:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-06 22:04:57]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[chongz, 2016-07-07 17:43:58]

The CQ bit was checked by chongz@chromium.org

[chongz, 2016-07-07 17:43:58]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org
Link to the patchset: https://codereview.chromium.org/2122053003/#ps40001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-07 17:44:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 18:55:24]

Description was changed from

==========
Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Before CL Shift+F10 won't open Context Menu when NumLock or CapsLock is
on, which is confusing and does not match ContextMenu key nor other browsers.

e.g.
1. On Chrome NumLock+ContextMenu opens CM
2. On Firefox NumLock+Shift+F10 opens CM
But on Chrome NumLock+Shift+F10 won't open CM.

After CL Chrome will match Firefox, Windows Notepad, IE, plus ContextMenu key's
behavior.

BUG=6941
==========

to

==========
Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Before CL Shift+F10 won't open Context Menu when NumLock or CapsLock is
on, which is confusing and does not match ContextMenu key nor other browsers.

e.g.
1. On Chrome NumLock+ContextMenu opens CM
2. On Firefox NumLock+Shift+F10 opens CM
But on Chrome NumLock+Shift+F10 won't open CM.

After CL Chrome will match Firefox, Windows Notepad, IE, plus ContextMenu key's
behavior.

BUG=6941
==========

[commit-bot: I haz the power, 2016-07-07 18:55:25]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-07-07 18:55:35]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 18:56:50]

Description was changed from

==========
Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Before CL Shift+F10 won't open Context Menu when NumLock or CapsLock is
on, which is confusing and does not match ContextMenu key nor other browsers.

e.g.
1. On Chrome NumLock+ContextMenu opens CM
2. On Firefox NumLock+Shift+F10 opens CM
But on Chrome NumLock+Shift+F10 won't open CM.

After CL Chrome will match Firefox, Windows Notepad, IE, plus ContextMenu key's
behavior.

BUG=6941
==========

to

==========
Shift+F10 should open Context Menu regardless the state of NumLock/CapsLock

Before CL Shift+F10 won't open Context Menu when NumLock or CapsLock is
on, which is confusing and does not match ContextMenu key nor other browsers.

e.g.
1. On Chrome NumLock+ContextMenu opens CM
2. On Firefox NumLock+Shift+F10 opens CM
But on Chrome NumLock+Shift+F10 won't open CM.

After CL Chrome will match Firefox, Windows Notepad, IE, plus ContextMenu key's
behavior.

BUG=6941

Committed: https://crrev.com/9ca5ec119dad81a9653712ced1efd2ab9a9539e2
Cr-Commit-Position: refs/heads/master@{#404194}
==========

[commit-bot: I haz the power, 2016-07-07 18:56:51]

Patchset 2 (id:??) landed as
https://crrev.com/9ca5ec119dad81a9653712ced1efd2ab9a9539e2
Cr-Commit-Position: refs/heads/master@{#404194}