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

[Bug]: Playing any form of audio causes electron to ask for Accessibility permissions on macOS #31558

Closed
3 tasks done
josh-signal opened this issue Oct 22, 2021 · 11 comments · Fixed by #31805
Closed
3 tasks done
Labels

Comments

@josh-signal
Copy link

Preflight Checklist

Electron Version

13.5.1

What operating system are you using?

macOS

Operating System Version

Any version

What arch are you using?

x64

Last Known Working Electron version

13.5.0

Expected Behavior

I would like an option to stop asking for Accessibility entitlement if our app doesn't need it.

Actual Behavior

User is prompted for permission for application to control their computer and it's listed under Accessibility in macOS Security & Privacy section.

Testcase Gist URL

No response

Additional Information

This is the commit that introduced this behavior: 75349f5#diff-4390247d2e627f63ca771b47b71bfe35268814a3472039f1749ea424bec3359dR32

We'd love to be on 13.5.1 or later since that fixes the Let's Encrypt CA but the entitlement makes it so we can't.

@codebytere
Copy link
Member

@josh-signal good news 😆 this just got fixed in #31492 and should be released soon!

@EvanHahn-Signal
Copy link
Contributor

Thanks for the quick response! Unfortunately, I don't think this fixes our issue, as I'm still able to see the macOS Accessibility permissions popup.

I made a simple app to reproduce the problem: https://github.com/EvanHahn-Signal/electron-issue-31558

To reproduce it, I do the following:

  1. Set up the app (npm install as normal)
  2. Make sure that my terminal app is completely missing from my macOS Accessibility settings. I go to System Preferences → Security & Privacy → Privacy → Accessibility and fully delete my terminal app—it's not enough if it's unchecked.
  3. Start the app (npm start).
  4. Play the audio in the app. Note that it's just an <audio> tag; there's no JavaScript or the use of any Node APIs. When I do this, I see the accesssibility popup.

@codebytere
Copy link
Member

codebytere commented Oct 25, 2021

@EvanHahn-Signal it looks like your sample uses 15.3.0? This fix hasn't been released in any version yet, as noted above - it will be in upcoming versions across release lines.

@EvanHahn-Signal
Copy link
Contributor

My bad!! Ignore me—apologies!

@indutny-signal
Copy link
Contributor

@codebytere was it fixed in 15.3.1? Judging by commit history the change should have made its way there, but we are seeing reports from internal testing that the Accessibility permissions popup is back after electron update.

@indutny-signal
Copy link
Contributor

indutny-signal commented Nov 11, 2021

@codebytere could you reopen this, please? Looks like the issue wasn't fixed by that PR and is in fact caused by:

  // Add an event tap to intercept the system defined media key events.
  event_tap_ = CGEventTapCreate(
      kCGSessionEventTap, kCGHeadInsertEventTap, kCGEventTapOptionDefault,
      CGEventMaskBit(NX_SYSDEFINED), EventTapCallback, this);

in ui/base/accelerators/media_keys_listener_mac.mm. I'm investigating why this function (StartEventTapIfNecessary) is called at all. Commenting the line fixes the issue completely, but I'm not sure if this is what we want to do.

@indutny-signal
Copy link
Contributor

Alright, I see what's going on now. kGlobalRequiresAccessibility has no effect whatsoever anymore so essentially whenever we use kGlobal it ends up requiring accessibility permissions.

@indutny-signal
Copy link
Contributor

Reading full path now I realize that this enum was added in the same patch, and themedia_keys_listener_mac.mm probably never supported it?

@ArmelChesnais
Copy link

@codebytere seems this is now happening on electron 14.2.1 If we could either reopen this, or I can open a separate issue for it if needed.

@indutny-signal
Copy link
Contributor

@ArmelChesnais I think #31805 just needs to be backported there.

@ArmelChesnais
Copy link

@ArmelChesnais I think #31805 just needs to be backported there.

@indutny-signal how do we initiate it? I'm not seeing any indication of backporting for 14.2.1 on that PR

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

Successfully merging a pull request may close this issue.

6 participants