Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Don't toggle menu unless ALT was the last key pressed #5450

Merged
merged 1 commit into from
Nov 7, 2016
Merged

Don't toggle menu unless ALT was the last key pressed #5450

merged 1 commit into from
Nov 7, 2016

Conversation

bsclifton
Copy link
Member

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Don't toggle menu unless ALT was the last key pressed
This prevents ALT codes from toggling menu

Fixes #4295

Auditors: @jonathansampson, @BrendanEich

Test Plan:

  1. Launch Brave on Windows and go to http://jsbin.com
  2. Click inside a text area and type a few letters
  3. Push ALT (left) and release to confirm menu shows and do same to hide it
  4. Push and hold ALT (left), key in 0133, then let go of ALT
  5. The ellipsis should be typed out and the menu should NOT have been toggled

This prevents ALT codes from toggling menu

Fixes #4295

Auditors: @jonathansampson, @BrendanEich

Test Plan:
1. Launch Brave on Windows and go to http://jsbin.com
2. Click inside a text area and type a few letters
3. Push ALT (left) and release to confirm menu shows and do same to hide it
4. Push and hold ALT (left), key in 0133, then let go of ALT
5. The ellipsis should be typed out and the menu should NOT have been toggled
Copy link
Collaborator

@jonathansampson jonathansampson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bbondy
Copy link
Member

bbondy commented Nov 7, 2016

💯

@luixxiul
Copy link
Contributor

luixxiul commented Nov 15, 2016

I cannot confirm this.

3 Push ALT (left) and release to confirm menu shows and do same to hide it

Pushing left alt key does not oepn the menu. Is this because of difference between keyboard layouts?

@srirambv
Copy link
Collaborator

srirambv commented Nov 15, 2016

Push and hold ALT (left), key in 0133, then let go of ALT

This doesn't type ellipse for me but the menu doesn't toggle. However pressing Alt+PrintScreen still shows the menu for me after the keys are released

@luixxiul
Copy link
Contributor

I confirmed it right now. @bsclifton suggested me it has been due to the focus issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants