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: disable more private macOS APIs in MAS build (4-2-x) #21003

Closed

Conversation

@cmgustavo
Copy link

cmgustavo commented Nov 6, 2019

Backport of #20965 .

Notes: Fix Electron apps getting rejected to Mac App Store.

@cmgustavo cmgustavo requested a review from electron/wg-upgrades as a code owner Nov 6, 2019
@welcome

This comment has been minimized.

Copy link

welcome bot commented Nov 6, 2019

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

@trop trop bot mentioned this pull request Nov 6, 2019
4 of 6 tasks complete
@trop trop bot added 4-2-x backport labels Nov 6, 2019
@electron-cation electron-cation bot removed the new-pr 🌱 label Nov 6, 2019
@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Nov 6, 2019

@cmgustavo Electron 4 is not supported anymore

@codebytere

This comment has been minimized.

Copy link
Member

codebytere commented Nov 6, 2019

@cmgustavo 4.x is no longer supported and as such we are no longer running releases from that branch

@codebytere codebytere closed this Nov 6, 2019
@ffflorian

This comment has been minimized.

Copy link

ffflorian commented Nov 6, 2019

@codebytere I am really unhappy about the fact that this backport was closed. I understand that you have a strict release cycle but right now it's impossible to publish Electron 4 apps to the Mac App Store.

Would you maybe make an exception for this one so that people running Electron 4 don't have to upgrade to Electron 5 just to be able to publish to the Mac App Store?

@jarek-foksa

This comment has been minimized.

Copy link

jarek-foksa commented Nov 7, 2019

I second that, could you please make an exception here? This is a special case since perfectly stable Electron 4 builds are no longer usable due to sudden and unannounced Apple policy changes.

Electron 5.x (or later) has too many serious bugs to be used in production. For example my app is completely broken on some devices because of bug #19626.

@MarshallOfSound

This comment has been minimized.

Copy link
Member

MarshallOfSound commented Nov 7, 2019

@ffflorian @jarek-foksa It's not even necessarily a case of making an exception. We have complex release automation that changes quite rapidly and once a version is EOL we no longer ensure compatibility with the current system.

This PR also did not appear to contain valid patches, based on the output of the CI runs the patch files are invalid the version of Chromium that 4-2-x depends on.

FWIW there are seval large apps using Electron 6 and 7 in production. Updating to our latest versions of Electron is still our strong reccomendation here.

@jarek-foksa

This comment has been minimized.

Copy link

jarek-foksa commented Nov 7, 2019

@MarshallOfSound Are you sure those large apps are actually published on the Mac App Store?

I did try updating to newer versions of Electron, all of them were crashing on my machine due to bug #19626. Therefore, now I can't publish any updates on the Mac App Store.

@circleapps

This comment has been minimized.

Copy link

circleapps commented Nov 13, 2019

@MarshallOfSound

My app is depending on Native Client to play local video files.

After Electron 5.0.x, this feature has been dropped, so I have to depend on Electron 4.x forever.

If you don't provide patch for Electron 4.x, I can't publish any updates on the Mac App Store, which is a disaster for myself and all my customers.

PLEASE reconsider this patch, thanks.

@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Nov 14, 2019

@circleapps you can always build Electron yourself with any custom backports / patches.

@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Nov 14, 2019

@circleapps are you using NaCl or just a regular Pepper plugin? Native pepper plugins may not work anymore due to the host process being sandboxed, which can be fixed.

@circleapps

This comment has been minimized.

Copy link

circleapps commented Nov 14, 2019

Thanks miniak,

I will learn how to build Electron 4.x with this patch by myself.

I remember the "sandbox" was introduced in Electron 5.x. So I assume if I stay with 4.x, my plugin will still work?

@circleapps

This comment has been minimized.

Copy link

circleapps commented Nov 14, 2019

I'm using mpv.js to play video files.
https://github.com/Kagami/mpv.js

@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Nov 15, 2019

So it’s a pepper plugin, I can help you get it working with latest Electron

@miniak

This comment has been minimized.

Copy link
Contributor

miniak commented Nov 15, 2019

it's tracked by #18516

@circleapps

This comment has been minimized.

Copy link

circleapps commented Nov 16, 2019

Thanks! I will try this method to see if it works for me.

@circleapps

This comment has been minimized.

Copy link

circleapps commented Nov 16, 2019

@miniak
It works! Really appreciate your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.