fix: don't register some shortcuts without accessibility #16125

codebytere · 2018-12-18T18:10:58Z

Description of Change

Resolves #14837.

/cc @ckerr

Checklist

PR description included and stakeholders cc'd
npm test passes
relevant documentation is changed or added
PR title follows semantic commit guidelines
PR release notes describe the change in a way relevant to app-developers

Release Notes

Notes: Fixed crash on macOS when using globalShortcut for media keys when accessibility access is not granted.

4ndv · 2018-12-19T06:26:20Z

AFAIK it required only since Mojave on macOS
Crashes occur on unregister, maybe checks also should be there?

codebytere · 2018-12-19T06:41:42Z

@4ndv the crash occurs on unregister because the shortcuts are falsely registered from what i could tell, meaning that the simplest way to prevent that would be to ensure that no globalShortcuts are registered with incorrect accessibility permissions

atom/browser/api/atom_api_global_shortcut.cc

nornagon

Add a note about macOS-only to release notes.

BinaryMuse

Minor typos below, but otherwise approving on behalf of docs.

It could be nice to link to a document when we mentioned being "authorized as a trusted accessibility client" in case folks don't know what that means or how to do it.

docs/api/global-shortcut.md


        fix: don't register some shortcuts without accessibility


        cleanup


        add media stop key


        conditionally include mac_util


        MacOS -> macOS


        update docs

release-clerk · 2019-01-04T04:40:20Z

Release Notes Persisted

Fixed crash on macOS when using globalShortcut for media keys when accessibility access is not granted.

codebytere · 2019-01-04T04:40:54Z

/trop run backport-to 4-0-x,3-1-x,3-0-x

trop · 2019-01-04T04:40:59Z

The backport process for this PR has been manually initiated,
sending your 1's and 0's to "4-0-x" here we go! :D

trop · 2019-01-04T04:41:03Z

The backport process for this PR has been manually initiated,
sending your 1's and 0's to "3-1-x" here we go! :D

trop · 2019-01-04T04:41:07Z

The backport process for this PR has been manually initiated,
sending your 1's and 0's to "3-0-x" here we go! :D

trop · 2019-01-04T04:41:42Z

I was unable to backport this PR to "4-0-x" cleanly;
you will need to perform this backport manually.

trop · 2019-01-04T04:41:46Z

I was unable to backport this PR to "3-1-x" cleanly;
you will need to perform this backport manually.

trop · 2019-01-04T04:41:47Z

I was unable to backport this PR to "3-0-x" cleanly;
you will need to perform this backport manually.


        fix: don't register some shortcuts without accessibility (electron#16125

) Fixed crash on macOS when using globalShortcut for media keys when accessibility access is not granted.


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms

This was a regression in electron#16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms (#…

…20963) This was a regression in #16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms

This was a regression in #16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms

This was a regression in #16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms

This was a regression in #16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms (#…

…20983) This was a regression in #16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms (#…

…20982) This was a regression in #16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux


        fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms (#…

…20984) This was a regression in #16125, which unintentionally put `GlobalShortcutListener::RegisterAccelerator` into a `#if defined(OS_MACOSX)` block. Notes: Fix broken `globalShortcut.registerAll()` on Windows and Linux

codebytere requested a review from Dec 18, 2018

codebytere changed the title ~~[WIP] fix: don't register some shortcuts without accessibility~~ [wip] fix: don't register some shortcuts without accessibility Dec 22, 2018

codebytere force-pushed the fix-globalshortcut-crash branch from 3f01165 to 3940e37 Jan 2, 2019

codebytere changed the title ~~[wip] fix: don't register some shortcuts without accessibility~~ fix: don't register some shortcuts without accessibility Jan 2, 2019

codebytere changed the title ~~fix: don't register some shortcuts without accessibility~~ fix: don't register some shortcuts without accessibility Jan 2, 2019

miniak requested changes Jan 2, 2019

View changes

atom/browser/api/atom_api_global_shortcut.cc Outdated Show resolved Hide resolved

atom/browser/api/atom_api_global_shortcut.cc Outdated Show resolved Hide resolved

atom/browser/api/atom_api_global_shortcut.cc Outdated Show resolved Hide resolved

nornagon requested changes Jan 2, 2019

View changes

codebytere requested a review from Jan 2, 2019

codebytere force-pushed the fix-globalshortcut-crash branch from 0398d20 to 9705298 Jan 2, 2019

miniak approved these changes Jan 3, 2019

View changes

nornagon approved these changes Jan 3, 2019

View changes

BinaryMuse approved these changes Jan 3, 2019

View changes

docs/api/global-shortcut.md Outdated Show resolved Hide resolved

docs/api/global-shortcut.md Outdated Show resolved Hide resolved

codebytere added 6 commits Jan 2, 2019

fix: don't register some shortcuts without accessibility

Verified

This commit was signed with a verified signature.

codebytere Shelley Vohr

GPG key ID: F13993A75599653C Learn about signing commits

22e133a

cleanup

Verified

This commit was signed with a verified signature.

codebytere Shelley Vohr

GPG key ID: F13993A75599653C Learn about signing commits

cced4e1

add media stop key

Verified

This commit was signed with a verified signature.

codebytere Shelley Vohr

GPG key ID: F13993A75599653C Learn about signing commits

99156ec

conditionally include mac_util

Verified

This commit was signed with a verified signature.

codebytere Shelley Vohr

GPG key ID: F13993A75599653C Learn about signing commits

89e1b28

MacOS -> macOS

Verified

This commit was signed with a verified signature.

codebytere Shelley Vohr

GPG key ID: F13993A75599653C Learn about signing commits

7b66d3a

update docs

Verified

This commit was signed with a verified signature.

codebytere Shelley Vohr

GPG key ID: F13993A75599653C Learn about signing commits

Loading status checks…

35c76aa

codebytere force-pushed the fix-globalshortcut-crash branch from aaceb1f to 35c76aa Jan 4, 2019

codebytere deleted the fix-globalshortcut-crash branch Jan 4, 2019

codebytere mentioned this pull request Jan 4, 2019

fix: don't register some shortcuts without accessibility #16262

Merged

5 of 5 tasks complete

codebytere added needs-manual-bp/3-1-x needs-manual-bp/3-0-x in-flight/4-0-x labels Jan 4, 2019

codebytere mentioned this pull request Jan 4, 2019

fix: don't register some shortcuts without accessibility #16276

Merged

4 of 4 tasks complete

codebytere added in-flight/3-1-x and removed needs-manual-bp/3-1-x in-flight/4-0-x labels Jan 4, 2019

akisctx added a commit to akisctx/electron that referenced this pull request Jan 22, 2019

fix: don't register some shortcuts without accessibility (electron#16125

b885f35

) Fixed crash on macOS when using globalShortcut for media keys when accessibility access is not granted.

poiru mentioned this pull request Nov 5, 2019

fix: Fix broken globalShortcuts.registerAll() on non-macOS platforms #20963

Merged

electron/electron

fix: don't register some shortcuts without accessibility #16125

fix: don't register some shortcuts without accessibility #16125

codebytere commented Dec 18, 2018 •

edited by BinaryMuse

This comment has been minimized.

4ndv commented Dec 19, 2018 •

edited

This comment has been minimized.

codebytere commented Dec 19, 2018

nornagon left a comment

BinaryMuse left a comment

This comment has been minimized.

release-clerk bot commented Jan 4, 2019

This comment has been minimized.

codebytere commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

electron/electron

Join GitHub today

fix: don't register some shortcuts without accessibility #16125

Conversation

codebytere commented Dec 18, 2018 • edited by BinaryMuse

Description of Change

Checklist

Release Notes

This comment has been minimized.

4ndv commented Dec 19, 2018 • edited

This comment has been minimized.

codebytere commented Dec 19, 2018

nornagon left a comment

BinaryMuse left a comment

This comment has been minimized.

release-clerk bot commented Jan 4, 2019

This comment has been minimized.

codebytere commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

This comment has been minimized.

trop bot commented Jan 4, 2019

codebytere commented Dec 18, 2018 •

edited by BinaryMuse

4ndv commented Dec 19, 2018 •

edited