Upgrade to Electron 12 #2352

aryan9600 · 2021-10-26T18:59:20Z

Objective

It'd be very helpful if Mailspring was updated to use Electron 12 (instead of Electron 8). Upgrading to Electron 12 would make Mailspring arm64 compatible (much needed, since all future macs are going to be arm64 for a long period of time) and also unlock support for Wayland which is the standard for Linux GUIs.

Testing Notes

Linux
macOS

Tips

References

https://community.getmailspring.com/t/electron-12-migration/504/2

marcelovbcfilho · 2021-11-08T21:34:47Z

This would be very hand, because I updated to Fedora 35 which enables wayland and crashes mailspring and many other electron apps that doesn't update to the latest electrom version.

thiblahute · 2021-11-18T02:06:34Z

This would be very hand, because I updated to Fedora 35 which enables wayland and crashes mailspring and many other electron apps that doesn't update to the latest electrom version.

fiwiw The flatpak version woorks just fine here on F35 (silverblue) running wayland.

aryan9600 · 2021-11-18T11:58:56Z

cc: @CodeMouse92 @bengotow

Phylu · 2021-11-22T19:20:59Z

I am trying to take this over. However, my electron knowledge is quite limited. So if there is anybody willing to help with this, feel free to contribute :)

Current Target Version: 16.0.0 (latest as of 28.11.2021)

TODOs:

Fix Spellchecker
Ensure that Adding words to the dictionary works (Works on macOS, but not on Ubuntu right now)
Fix Notifications on Mac
Fix Crash Reporter
Replace vm module (Optional)
Replace remote module with either userspace module (Optional)
~~Remove key tar~~

Issues List:

Electron 9

electron-spellchecker fails due to Non-context-aware native module warning electron-userland/electron-spellchecker#167. Need to switch to the electron native spellchecker instead: https://www.electronjs.org/docs/latest/tutorial/spellchecker
In electron 12 the native spellchecker can be used with a similar design as before: https://www.electronjs.org/docs/latest/api/web-frame#webframegetwordsuggestionsword
Problematic is the automated language detection. The method used is not available in the native electron spellchecker. I will need to see how this was implemented in the used plugin to see if it is feasible to implement this within Mailspring instead.

Electron 10

Set enableRemoteModule = true due to: Deprecate the 'remote' module and move it to userland electron/electron#21408

Electron 11

node-mac-notifier does not work anymore due to disabling ffi-napi in chromium. --> Switch to electron's own api: https://www.electronjs.org/docs/latest/api/system-preferences#systempreferencespostnotificationevent-userinfo-deliverimmediately-macos

Electron 12

crashReporter.start() does not exist in the renderer process anymore: https://github.com/electron/electron/blob/12-x-y/lib/renderer/api/crash-reporter.ts. Therefore, we need to adjust the error tracking to only setup the crashReporter once in the main process.

Electron 13++

Development Branch:

https://github.com/Phylu/Mailspring/tree/electron-upgrade

foundry376-bot · 2021-11-22T19:55:07Z

This issue has been mentioned on Mailspring Community. There might be relevant details there:

https://community.getmailspring.com/t/electron-12-migration/504/6

mamantoha · 2021-11-24T14:05:02Z

Hi @Phylu . Hope you are doing well.
I was able to run Mailspring with Electron 12.2.3 natively on Wayland based on your branch https://github.com/Phylu/Mailspring/tree/electron-upgrade

.

The only issue I faced is:

ReferenceError: crashReporter is not defined
    at ErrorLogger.module.exports.ErrorLogger._startCrashReporter (/media/disk/ruby/github/Mailspring/app/src/error-logger.js:109:5)

So I just comment on these lines.

Phylu · 2021-11-24T15:06:49Z

@mamantoha Very nice. I stopped at Electron 10 due to time constraints and will pick up from there. If you proceeded with the upgrade, feel free to open an MR against my development branch. Would be really nice, if it can safe me some time.

mamantoha · 2021-11-24T16:02:59Z

@Phylu done Phylu#1

I will take a look later if I can help with other stuff.

Phylu · 2021-11-24T16:15:35Z

@mamantoha Thanks a lot! :)

Phylu · 2021-11-24T21:59:42Z

I am tracking my progress here: #2352 (comment)

So far, I think that I can get it running, but it needs some work e.g. to get the spellchecker or the notifications on Mac up again.

bengotow · 2021-11-25T21:14:06Z

Hey gang! Ahh this would be wonderful - I think Electron 13 would actually be a better target because they built a spellchecker into Electron itself so we don't need to get an external one working.

@Phylu if you've got some cycles, I actually have a branch I started in September but hadn't published with a bit of Electron 13 work! https://github.com/Foundry376/Mailspring/tree/bengotow/electron-13. Maybe you can cherry pick things from that into your branch.

I think once we move to 12+, it'll be easier to continue keeping pace with the Electron releases. It's just that 8->12 is particularly painful...

Phylu · 2021-11-25T21:26:16Z

@bengotow Thanks for sharing the branch. I will definitely pick done things from there. Actually it currently looks quite similar to my development branch. ;-)

As far as I understand all the spellchecker methods needed are already implemented in electron 12.

What will be painful is the change from the remote module to using native IPC calls oder the remote method from the userland. If you have a preference here and probably some support (especially if we go for the IPC way) that would be great. But I think this would be a target for electron 13 or 14.

Phylu · 2021-11-26T16:15:47Z

@bengotow When trying to build Mailspring with Electron 13, it fails with the error below. I have tested this on macOS 12 and Ubuntu 21.10. Did you have the build from your branch running when you worked on it? I had no luck with any of the node versions I tried or different keytar versions. If you have any idea of why this is happening, let me know.

As the native spellchecker is available in Electron 12, I will target this for now.

npm ERR! gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
npm ERR! In file included from ../node_modules/node-addon-api/napi.h:2725,
npm ERR!                  from ../src/async.cc:4:
npm ERR! ../node_modules/node-addon-api/napi-inl.h: In member function ‘bool Napi::Object::Freeze()’:
npm ERR! ../node_modules/node-addon-api/napi-inl.h:1393:24: error: ‘napi_object_freeze’ was not declared in this scope; did you mean ‘napi_object_expected’?
npm ERR!  1393 |   napi_status status = napi_object_freeze(_env, _value);
npm ERR!       |                        ^~~~~~~~~~~~~~~~~~
npm ERR!       |                        napi_object_expected
npm ERR! ../node_modules/node-addon-api/napi-inl.h: In member function ‘bool Napi::Object::Seal()’:
npm ERR! ../node_modules/node-addon-api/napi-inl.h:1399:24: error: ‘napi_object_seal’ was not declared in this scope; did you mean ‘napi_object’?
npm ERR!  1399 |   napi_status status = napi_object_seal(_env, _value);
npm ERR!       |                        ^~~~~~~~~~~~~~~~
npm ERR!       |                        napi_object
npm ERR! make: *** [keytar.target.mk:125: Release/obj.target/keytar/src/async.o] Fehler 1
npm ERR! gyp ERR! build error 
npm ERR! gyp ERR! stack Error: `make` failed with exit code: 2
npm ERR! gyp ERR! stack     at ChildProcess.onExit (/home/janosch/git/Mailspring/node_modules/node-gyp/lib/build.js:194:23)
npm ERR! gyp ERR! stack     at ChildProcess.emit (node:events:390:28)
npm ERR! gyp ERR! stack     at Process.ChildProcess._handle.onexit (node:internal/child_process:290:12)
npm ERR! gyp ERR! System Linux 5.13.0-21-generic
npm ERR! gyp ERR! command "/home/janosch/.nvm/versions/node/v16.13.0/bin/node" "/home/janosch/git/Mailspring/node_modules/.bin/node-gyp" "rebuild"
npm ERR! gyp ERR! cwd /home/janosch/git/Mailspring/app/node_modules/keytar
npm ERR! gyp ERR! node -v v16.13.0
npm ERR! gyp ERR! node-gyp -v v7.1.2
npm ERR! gyp ERR! not ok

Phylu · 2021-11-28T17:12:24Z

When trying to build Mailspring with Electron 13, it fails with the error below.

I figured out the problem... If the node version is too new, it breaks :(
The build works with v14.11.0. I will add this to the Readme for now. We should try to remove keytar to avoid this issue.

foundry376-bot · 2021-11-28T22:13:00Z

This issue has been mentioned on Mailspring Community. There might be relevant details there:

https://community.getmailspring.com/t/unable-to-launch-the-app-on-fedora-35/3424/2

bengotow · 2021-11-29T01:03:00Z

Oh wow good find! I bet we could fix that keytar issue, that library wraps a ton of native stuff to provide secure password storage but somehow it's always the first thing to break :-/

Phylu · 2021-11-29T06:52:59Z

@bengotow I really hate npm and its versions. After updating electron further, I managed to build it with even newer node versions. So the build now works on my Ubuntu and my MacOS machines with the latest node and electron version. I would say that this is a win :)

I am now back to fixing the issues that broke, but at least do not need to replace keytar for now.

Phylu · 2021-11-29T22:11:19Z

@bengotow I need some input from you again. Sorry for this. With the removal of the node-mac-notifier (which fails to build with newer electron versions and seems abandoned), we need to switch to the native notifications. This basically works, but to keep the full functionality on macOS (having a reply button directly in the notification), is more complicated. There are basically three options:

Remove the possibility to reply directly from the notification. I think, this would be ok, as I don't assume that many people will directly answer their mails from the notifications. I did not even notice that this feature existed before I started working on this part of the code. There is also no possibility to add a subtitle on macOS here. So the notification would probably loose the content part of the message and only show the sender and the subject.
Adding a service worker to the notifications to make them persistent, which opens the possibility to add actions to them. (subtitle issue on macOS applies similar to 1)
Moving the notifications from the renderer process to the main process, where we can use the electron native notifications instead of HTML5 notifications which support actions etc.

Option 1 is straight forward, but will loose some functionality. I am still suggesting to go this way (see reasoning above). If that seems bad to you, I could opt for 2 or 3, but would need some pointers on where to add the service worker or the notifications in the main process not sure where to add this stuff to the codebase.

Phylu · 2021-11-29T22:36:39Z

Apart from the Mac notifications, which are currently more basic than they used to be, this is nearly finished.

@mamantoha Would you be willing to do some test runs using a build from my development branch? That would be really great! The same – of course – applies to anybody else reading in here :)

Especially tests on Windows are highly appreciated, as I currently don't have a test system available there.

mamantoha · 2021-11-29T23:45:18Z

Hi @Phylu . Thanks for your work.

I am mostly interested in Wayland support.
I tried branch with Electron 16.0.0. But there is an issue with Wayland electron/electron#31885.

So I downgrade to 15.3.2 for testing. It starts under both Xorg and Wayland.

Issues:

Click "Compose new message" - nothing happens.
Open "Preferences" fails - SyntaxError: Identifier 'BrowserWindow' has already been declared
Click on email on "View activity" fails with ReferenceError: newrequire is not defined . I guess this is a typo in the code.

Phylu · 2021-11-30T06:14:56Z

Thanks a lot! I did not expect the election version being too new after I am went the effort if all the upgrades... I will follow the issue you mentioned and see that we use a working electron version in the end.

Do you see any errors/logs when trying to open the "New message" window?

mamantoha · 2021-11-30T08:27:03Z

Do you see any errors/logs when trying to open the "New message" window?

Nevermind. A window is actually created but it is invisible. This is Wayland's only issue. With Xorg(XWayland) everything works fine.

Phylu · 2021-11-30T16:08:59Z

@mamantoha

Click "Compose new message" - nothing happens.

This is very strange on Wayland. Any ideas whether we can do anything about this on the Mailspring side?

Open "Preferences" fails - SyntaxError: Identifier 'BrowserWindow' has already been declared

Fixed this in my latest updates.

Click on email on "View activity" fails with ReferenceError: newrequire is not defined . I guess this is a typo in the code.

Fixed this in my latest updates.

mamantoha · 2021-11-30T17:20:52Z

@Phylu

This is very strange on Wayland. Any ideas whether we can do anything about this on the Mailspring side?

This is stranger than I thought 🤯

I accidentally noticed that It only not showing on my laptop with fractional scaling.
When I connect the second monitor without scaling everything works fine.

I'm investigating where is an issue.

bengotow · 2021-12-01T03:21:04Z

Hey gang! Ahh I think #1 sounds fine for notifications - I'm not sure people use that "Inline Reply" feature in the notifications, and if they do we can come back to that and find another solution. Overall I think it's fine to cut things back a bit if it means easier Electron upgrades in the future. All the custom modules have really made the process difficult over the years...

The Wayland issues are definitely interesting - I know in Electron 12 Wayland support was one of the big improvements... it looks like Electron 16.0 just came out a few weeks ago and it might be something they broke recently in the upgrade. Maybe try 13 or 14 and see if those work better?

Thanks for all the hard work @Phylu that's super exciting this is pretty much working! If it's at a good point we can try making Travis builds and publish an "edge" Snapcraft release (currently the only real beta channel) for more folks to try!

mamantoha · 2021-12-01T17:47:16Z

Electron 14.2.1 is the latest stable which works fine for me. So I think we can stay with it for now.

If it's at a good point we can try making Travis builds and publish an "edge" Snapcraft release

This would be great!

Phylu · 2021-12-01T17:51:25Z

I opened the PR in Draft mode now: #2357

I have installed it on my Mac Monterey and Ubuntu 20.14 and will use it for a few days to see if I encounter some more errors during daily use, that I missed so far. If I feel, it is safe, I will let you know. :)

@bengotow Is there anything, I can do to help with the Travis?

@mamantoha So 15.3.2 does not work for you, as you mentioned this earlier? I targeted this for now, as I assumed that it was safe, based on your tests.

Phylu · 2021-12-05T16:31:50Z

I have tested it for a few days now on Ubuntu and macOS, and I am satisfied with the result and ran into no more bugs so far. I also downgraded to the latest version that works for @mamantoha (15.2.1).

So from my side this would be good to go. @bengotow Please let me know if I can do anything regarding the packaging/build pipeline/edge version.

frigaut · 2021-12-19T00:18:45Z

Thanks @Phylu for this. I am running it and will report on possible issues.
Btw: forgot to mention, after checking out your electron-upgrade branch, and running "npm install", I am running with
npm start -- --enable-features=UseOzonePlatform --ozone-platform=wayland

foundry376-bot · 2021-12-20T11:44:52Z

This issue has been mentioned on Mailspring Community. There might be relevant details there:

https://community.getmailspring.com/t/mailspring-for-apple-sillicon-macs/451/10

frigaut · 2021-12-27T06:15:27Z

So far so good for me too. @bengotow, I could also possibly help for an AUR mailspring wayland version then.

frigaut · 2022-01-02T03:45:50Z

Just one small GUI mishap: right clicking to get a context menu pops up the menu way too far to the right (about 300/400 pixels). Not a big problem, but just mentioning. Using @Phylu's version as mentioned above.

Phylu · 2022-01-02T07:32:34Z

Just one small GUI mishap: right clicking to get a context menu pops up the menu way too far to the right (about 300/400 pixels). Not a big problem, but just mentioning. Using @Phylu's version as mentioned above.

@frigaut Thanks for the notice. Can you provide a screenshot of the issue?

frigaut · 2022-01-02T08:10:45Z

sure. The star is where my cursor was (added afterwards as not captured by grim), and I have added a red rectangle to show where the context menu is.

Phylu · 2022-01-02T17:45:01Z

@frigaut I tried to reproduce this both on MacOS and Ubuntu 21.10. This does not happen for me on any of those two systems. Could this be an issue with the (dark) theme that you are using? Can you try with the default light theme?

mamantoha · 2022-01-02T23:11:03Z

@frigaut I also can't reproduce on my Linux.
Can you provide more info? Like OS/Distributive, DE, and display server(Xorg/Wayland).

frigaut · 2022-01-04T08:58:58Z

@Phylu + @mamantoha Indeed this seems related to my config. I ran Mailspring in a separate account and the issue is not there. I'll continue investigating but right now can drop the issue.
Just for completeness, the info @mamantoha requested:

Running arch on a thinkpad carbon X1
Intel UHD graphics 620
Sway 1.6.1 (on wayland obviously, V1.20)
Running the mailspring version out of Phylu's repo as above, build locally (version number listed as "1.9.3-COMMIT-INSERTED-DURING-PACKAGING")
Starting mailspring with npm start -- --enable-features=UseOzonePlatform --ozone-platform=wayland in root directory of the mailspring I checked out from Phylu's repo (after I built it of course).

I tried changing theme back to the default, no change. I looked up my environment but there is nothing in there that seems related to scaling, etc. My output/display wlroot scaling is 1.0 (using different value doesn't change the issue). Interestingly, using --force-device-scale-factor=something as additional parameter solves the issue of placement of the contextual menus, but introduces others (like incomplete menu size, i.e. the fonts are scaling but not the physical size of the menu thus one has to scan to access bottom/right of the menus - note that this doesn't happen when --force-device-scale-factor=1.0, but of course this results in very small fonts so not really usable.
Anyway, still investigating, will update when I find the issue, could be useful to some other poor soul out there. Thanks,

frigaut · 2022-01-04T12:17:55Z

Found it!
This was caused by one of my setting in gnome-tweaks - a remnant from when I was running gnome. The font scaling was set at 1.4. Putting it back to 1.0 solves the issue of the misplaced contextual menu in wayland mailspring.
So somehow, some apps, even when running wayland, seem to obey the gnome (GTK?) font scaling directive. Mailspring for one, but firefox too.
Of course setting it back to 1.0 messes up my font settings in these app, but this is easily fixed with a combination of setting font sizes (also in gnome-tweaks), and zoom factors in the apps (mailspring and firefox in my case).

Now, why the gtk font scaling factor is affecting where the contextual menu pops up is beyond me. At first sight, it might be a bug, but unsure about the logic that's used for this.

frigaut · 2022-01-05T06:13:23Z

Hello again - A question: when running mailspring with the above npm start -- --enable-features=UseOzonePlatform --ozone-platform=wayland, it is running in dev mode. Is there a way to run it NOT in dev mode by passing it some flag? If I uncheck "Developer > run with debug flags", the window disapear (even though the mailspring processes are still alive).

mamantoha · 2022-01-05T10:14:04Z

Hi @frigaut . You can try without --dev flag

./node_modules/.bin/electron --enable-features=UseOzonePlatform --ozone-platform=wayland ./app --dev

frigaut · 2022-01-05T10:33:16Z

Thanks @mamantoha .
Tried without the dev flag. I can get it to start. At first pass it gets through "get started" all right, with these windows being indeed wayland. But it won't display the main mailspring window after account set up. Restarted did the same thing. App is running as I can see the processes and the tray icon is there and responds to a "quit mailspring", but no main window.
It works normally with the --dev flag.
I can provide logs if you think it is interesting to follow up on this. Otherwise will continue to run the dev version here.

mamantoha · 2022-01-05T11:01:13Z

@frigaut I have exactly the same behavior on Wayland with KDE Plasma.
I forgot to mention this.

It works with this command:

QT_QPA_PLATFORM=wayland ./node_modules/.bin/electron --disable-gpu --enable-features=UseOzonePlatform --ozone-platform=wayland ./app

I'm not sure is an Electron or/and Wayland/Plasma issue. I saw similar problems with other Electron apps line GitHub Desktop, Slack, Postman.

frigaut · 2022-01-05T11:08:07Z

@mamantoha Thanks. Unfortunately, it doesn't work in my case (sway). I have tried your suggested command, plus a combination of with or without the "--disable-gpu", and adding other env variable like GDK_BACKEND=wayland XDG_SESSION_TYPE=wayland on top of the QT_QPA_PLATFORM=wayland, but still no main mailspring window...

frigaut · 2022-01-06T10:00:00Z

Because I guess we're beta-tester, I have this error (just occurred after sending new email, not sure it's linked). It might be benign (app doesn't crash nor anything, no visible adverse effects):

/home/frigaut/sandbox/Mailspring/app/src/error-logger.js:99 Error: ResizeObserver loop limit exceeded
    at window.onerror (app-env.ts:158) {url: 'file:///home/frigaut/sandbox/Mailspring/app/static…3A%221.9.3-COMMIT_INSERTED_DURING_PACKAGING%22%7D', line: 0, column: 0, pluginIds: Array(0)}

Phylu · 2022-01-06T18:41:47Z

Is there a way to run it NOT in dev mode by passing it some flag?

@frigaut Have you tried creating build using npm build (IIRC)? Than you have the package that you can install which runs (of course) without the build flags. You can then try to run the installed Mailspring with your flags.

If you have problems with the build, I can also provide Linux and Mac packages from my branch if needed.

Phylu self-assigned this Nov 22, 2021

Phylu mentioned this issue Dec 1, 2021

Electron upgrade to 14.2.1 #2357

Merged

bengotow closed this as completed in #2357 Feb 28, 2022

terrorizer1980 mentioned this issue Nov 29, 2023

[Snyk] Fix for 11 vulnerabilities terrorizer1980/Mailspring#58

Open

terrorizer1980 mentioned this issue Dec 20, 2023

[Snyk] Fix for 8 vulnerabilities terrorizer1980/Mailspring#61

Open

Upgrade to Electron 12 #2352

Upgrade to Electron 12 #2352

Comments

aryan9600 commented Oct 26, 2021

Objective

Testing Notes

Tips

References

marcelovbcfilho commented Nov 8, 2021

thiblahute commented Nov 18, 2021

aryan9600 commented Nov 18, 2021

Phylu commented Nov 22, 2021 • edited Loading

TODOs:

Issues List:

Development Branch:

foundry376-bot commented Nov 22, 2021

mamantoha commented Nov 24, 2021

Phylu commented Nov 24, 2021

mamantoha commented Nov 24, 2021

Phylu commented Nov 24, 2021 • edited Loading

Phylu commented Nov 24, 2021

bengotow commented Nov 25, 2021

Phylu commented Nov 25, 2021

Phylu commented Nov 26, 2021

Phylu commented Nov 28, 2021

foundry376-bot commented Nov 28, 2021

bengotow commented Nov 29, 2021

Phylu commented Nov 29, 2021

Phylu commented Nov 29, 2021 • edited Loading

Phylu commented Nov 29, 2021

mamantoha commented Nov 29, 2021 • edited Loading

Phylu commented Nov 30, 2021

mamantoha commented Nov 30, 2021

Phylu commented Nov 30, 2021 • edited Loading

mamantoha commented Nov 30, 2021

bengotow commented Dec 1, 2021

mamantoha commented Dec 1, 2021

Phylu commented Dec 1, 2021

Phylu commented Dec 5, 2021

frigaut commented Dec 19, 2021 • edited Loading

foundry376-bot commented Dec 20, 2021

frigaut commented Dec 27, 2021

frigaut commented Jan 2, 2022

Phylu commented Jan 2, 2022 • edited Loading

frigaut commented Jan 2, 2022

Phylu commented Jan 2, 2022

mamantoha commented Jan 2, 2022

frigaut commented Jan 4, 2022

frigaut commented Jan 4, 2022 • edited Loading

frigaut commented Jan 5, 2022

mamantoha commented Jan 5, 2022

frigaut commented Jan 5, 2022

mamantoha commented Jan 5, 2022

frigaut commented Jan 5, 2022

frigaut commented Jan 6, 2022

Phylu commented Jan 6, 2022

Phylu commented Nov 22, 2021 •

edited

Loading

Phylu commented Nov 24, 2021 •

edited

Loading

Phylu commented Nov 29, 2021 •

edited

Loading

mamantoha commented Nov 29, 2021 •

edited

Loading

Phylu commented Nov 30, 2021 •

edited

Loading

frigaut commented Dec 19, 2021 •

edited

Loading

Phylu commented Jan 2, 2022 •

edited

Loading

frigaut commented Jan 4, 2022 •

edited

Loading