Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: window menu should handle keys correctly #21434

Merged
merged 2 commits into from Dec 10, 2019
Merged

Conversation

zcbenz
Copy link
Member

@zcbenz zcbenz commented Dec 9, 2019

Description of Change

Fix a few weird behaviors and crashes when controlling window menu by keys, for example:

  • Press Alt => Press Esc => Crash
  • Press Alt => Press Alt again => Focus does not move back

This was caused by Chromium removing a few virtual methods. I have added the virtuals back, and I'll try to upstream the patch.

Checklist

Release Notes

Notes: Fix a weird behaviors and crashes when controlling window menu by keys.

@zcbenz zcbenz requested a review from a team as a code owner December 9, 2019 01:51
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Dec 9, 2019
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

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

LGTM for the patch given regression caused.

Date: Thu, 4 Oct 2018 14:57:02 -0700
Subject: fix: add back virtual methods in AccessiblePaneView

Mark SetPaneFocus and RemovePaneFocus as virtual in AccessiblePaneView, as we
Copy link
Member

Choose a reason for hiding this comment

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

Can you open the CL to chromium and add that CL to this patch so that we can more easily follow/track this patch to ensure it's deleted in future?

Copy link
Member Author

Choose a reason for hiding this comment

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

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Dec 10, 2019
@zcbenz zcbenz merged commit a6d142a into master Dec 10, 2019
@release-clerk
Copy link

release-clerk bot commented Dec 10, 2019

Release Notes Persisted

Fix a weird behaviors and crashes when controlling window menu by keys.

@trop
Copy link
Contributor

trop bot commented Dec 10, 2019

I have automatically backported this PR to "8-x-y", please check out #21452

@trop trop bot removed the target/8-x-y label Dec 10, 2019
@trop
Copy link
Contributor

trop bot commented Dec 10, 2019

I have automatically backported this PR to "7-1-x", please check out #21453

@sofianguy sofianguy added this to Fixed in 8.0.0-beta.5 in 8.2.x Jan 14, 2020
@sofianguy sofianguy added this to Fixed in 7.1.5 in 7.2.x Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
7.2.x
Fixed in 7.1.5
8.2.x
Fixed in 8.0.0-beta.5
Development

Successfully merging this pull request may close these issues.

None yet

3 participants