Upgrade to Electron 4 #19373

nathansobo · 2019-05-20T21:47:34Z

This PR will track our progress upgrading to Electron 4.

We just upgraded to Electron 3, but we're experiencing a crash deep in the bowels of V8 (#19372). I'm interested in whether Electron 4 might fix this crash.

Issues to resolve

Crash in mksnapshot when building with Electron 4 #19374 Crash in mksnapshot when building with Electron 4
Update any native modules Atom is using to make sure win_delay_load_hook isn't false: https://electronjs.org/docs/tutorial/using-native-node-modules#a-note-about-win_delay_load_hook
- @atom/nsfw: Merge upstream changes from Axosoft/nsfw up to v1.2.2 nsfw#6
- git-utils: Don't set win_delay_load_hook to false git-utils#94
- scandal (git-utils version bump): Update git-utils to allow loading on Windows scandal#47
Failing integration test
Native tabs super slow (Native tabs super slow on Electron v4 #19611).

mfonville · 2019-05-21T09:46:28Z

package.json

@@ -12,7 +12,7 @@
    "url": "https://github.com/atom/atom/issues"
  },
  "license": "MIT",
-  "electronVersion": "3.1.9",
+  "electronVersion": "4.2.0",


Isn't it more practical to use 4.2.2 to also have the latest bugfixes?

Why not go straight to v5.0.1?

It's just about being incremental. We tried to go to 3 first, but we're hitting #19372, so we wanted to see how we'd do in 4. Unfortunately we still don't really know if the crash is fixed on Electron 4 because we can't get it started yet.

But yeah, we should be on the latest patch release.

4.2.3 has been released yesterday

And 4.2.4 has been tagged

And 4.2.5 has been tagged now too

50Wliu · 2019-05-21T14:07:25Z

I was able to successfully build Atom by skipping the snapshot generation and using a patched version of fuzzy-native (to bypass node-pre-gyp), but starting Atom fails with "The specified procedure could not be found" when attempting to load nsfw 😕. That usually points to a rebuild problem but apm seems to be doing the right thing...

Edit: Making progress. I can get Atom to start now. Mostly by patching our native modules to get rid of win_delay_load_hook: https://electronjs.org/docs/tutorial/using-native-node-modules#a-note-about-win_delay_load_hook

More info here: https://developers.google.com/web/updates/2018/03/cssom

In Chrome 66+ it seems that getComputedStyle().fontWeight returns the computed numeric value of the style instead of the original descriptive name. We now look for value '700' which corresponds to the value of 'bold'.

And a bump for fuzzy-finder to remove unnecessary dependencies that were causing installation issues

Let's see if it fixes the config watching warnings...

Asday · 2019-05-23T10:10:53Z

Just to make sure, this finally fixes #15323, right?

This reverts commit 6e90fbe.

Since ChromeDriver v2.41, ChromeDriver will only connect if, either we precise a port for remote debugging, either the embedder (ie electron) made sure to pass `USER_DATA_DIR` to the remote debugging server. So, for now, we'll just use a random port (we don't care about its value since we're not connecting through it). (inspired by electron-userland/spectron@737db13).

rafeca · 2019-06-25T14:17:23Z

General testing

Review issues tagged with electron to see if any of them are fixed
Review any comment in the Atom code that contains the keyword TodoElectronIssue to see if any of them can be resolved (search link).
Verify that you can collaborate with Teletype
- Host using Atom's current stable release can collaborate with guest using Atom with the new version of Electron
- Guest using Atom's current stable release can collaborate with host using Atom with the new version of Electron
- Host and guest can collaborate when both are using Atom with the new version of Electron
Steps
1. Open a file 1. Share a portal 1. Have a guest join the portal 1. When guest edits the text in the file, verify that the host sees those changes

Check for regressions experienced in previous upgrades

Text Input/Keybindings

See how to setup keyboard layouts.

IME not working (1.19.0-beta2: Cursor stucks at the first letter when using macOS Chinese IME #14911)
- macOS
- Windows 10
- Ubuntu
Repro steps
1. Install an IME layout, for example Pinyin simplified
2. Open Atom and use the layout you installed as the keyboard layout
3. Example, if you picked Pinyin simplified type in zhongwenshuru
Expected: in Pinyin simplified 中文输入 is expected

Issues we've seen: Only shows latin first character and not every character typed.
macOS: Key binding with composition characters are ignored (key-mappings with alt don't suppress Mac OS composition characters anymore (atom-1.19) #15189)
Repro steps
1. Open your keymap.cson file and add the following:
```
'atom-text-editor':
  # use hjkl; vim-like when pressing alt (word by word, paragraph by paragraph)
  # or alt (letter by letter)
  'alt-h': 'core:move-left'
  'alt-j': 'core:move-down'
  'alt-k': 'core:move-up'
  'alt-l': 'core:move-right'
```
2. Save keymap.cson and reload atom.
3. Try the new keymappings under ABC - Extended keyboard layout
Expected: the keybindings work

Issues we've seen: it types out the modified keys and ignores the key mapping
macOS: Composition characters mess up insertion point (Composition characters mess up insertion point #15344)
Repro steps
1. Open keymap.cson and add the following:
```
'atom-text-editor':
  'alt-n': 'core:move-down'
  'alt-o': 'core:move-down'
```
2. Have the following in your buffer:
```
Foo
Bar
```
3. Have the cursor at the beginning of file(before F character) Type alt-n, then arrow right, then insert a character. Observe that the character is inserted at the beginning of the line (where the alt-n left you).
4. Repeat steps using alt-o instead of alt-n.
Expected: Both alt-o and alt-n behave the same

Issues we've seen:
Ubuntu: Keystrokes involving ctrl resolve to the default layout instead of the active layout (Keystrokes involving ctrl on Linux resolve to the default layout instead of the active layout #13170)
Repro steps

This should be tested on Linux with gnome
1. Install a Linux ditro with gnome, for example Ubuntu
2. Install French AZERTY and English US QWERTY layouts
3. Change the default layout to be French AZERTY
4. Open Atom and switch to English US QWERTY layout
5. Press ctrl-w
Expected: core:close to dispatch (Or the keybinding-resolver to resolve to ctrl-w). Should be resolving to the keyboard layout that is chosen and not the OS default layout.

Issues we've seen: core:undo dispatched because it was resolved as ctrl-z because AZERTY has Z where W is on QWERTY.
Other keyboard layouts on new Electron version
- macOS: ABC - Extended
- macOS: Slovak
- macOS: U.S. International
- macOS: Squirrel (https://github.com/rime/squirrel)
- macOS: Full-Width Alphanumeric (Google) - Japanese layout (https://www.google.co.jp/ime/)
- Windows: U.S.
- Linux: Pinyin Simplified

UI

Other

jasonrudolph · 2019-06-25T16:36:32Z

@rafeca: Since Electron 4 drops support for macOS 10.9, is there something we can do to ensure that Atom users on macOS 10.9 don't get auto-upgraded to this incompatible version of Electron?

rafeca · 2019-06-25T17:31:05Z

@rafeca: Since Electron 4 drops support for macOS 10.9, is there something we can do to ensure that Atom users on macOS 10.9 don't get auto-upgraded to this incompatible version of Electron?

I have no idea, maybe @ckerr , @jkleinsc or @BinaryMuse have some suggestions here, since other Electron apps that upgraded to v4 have probably experienced this same problem.

What we can do now is to start showing now banners on Atom running on OSX 10.9 informing users that we're going to drop support for this operating system very soon (maybe we can add this banner to v1.39 or v1.40).

rafeca · 2019-06-26T17:26:15Z

Regarding blocking 10.9 users from upgrading, this seems to be possible to do only by our atom.io backend system, which is the one that returns the potential list of versions to upgrade (This is based on a Slack conversation with @jkleinsc).

client side changes

In order to do so, Atom needs to pass the OS version to atom.io when it asks if there are new versions. This is a very easy change which can be added in this line.

In order to make everything work, though, we need to do this change at least one the version before we ship Electron v4, so older clients that do not need to get updated are already sending the OS version.

My suggestion is to make these changes as soon as possible and cherry-pick them to the current beta to get them to stable soon.

atom.io changes

Then, we need to change atom.io logic to take into account the OS version. This is going to be trickier to be able to also deal correctly with clients having much older versions of Atom.

In order to explain the logic a little bit clearer, I'm going to assume that the new parameter is added on Atom v1.40 and Electron v4 is added on Atom v1.42:

If the client does not pass the os_version param (old client), atom.io will return the latest v1.40.x version.
If the client passes os_version=10.9 (and is a mac), atom.io will return the latest v1.41.x version.
If the client passes any other os_version param (or it's not a mac), atom.io will return the latest version (as it does now).

The same logic should be applied to each release channel (stable, beta and nightly).

Good thing is that if we build this logic in a configurable way, later it's going to be trivial to block e.g older Windows versions from upgrading if Electron drops support for them.

The atom.io changes can wait until we merge this branch (then we'll want to have them to prevent nightly users from upgrading).

@jasonrudolph thoughts?

jasonrudolph · 2019-06-26T19:17:27Z

@rafeca: That approach sounds good to me. 👍

nathansobo · 2019-06-26T19:20:18Z

This plan makes sense to me @rafeca.

One thing worth noting... Unless there's an API I don't know about, I think the best we'll be able to do is for determining the OS version is require("os").release(). That reports different version numbers than the 10.x used in Apple's marketing. Based on this table of macOS releases, it looks like 10.9 corresponds to a release number of 13.0.0 and 10.10 corresponds to 14.0.0.

rafeca · 2019-06-28T08:11:09Z

While doing the regression testing I found an issue related to native tabs: when I enable them, doing operation with tabs becomes super slow and makes the editor unresponsive for ~5-10s. Same happens when doing operations with the native tabs (like merging different windows into a single window or splitting tabs into multiple windows).

I've created an issue to track it: #19611

rafeca · 2019-07-05T12:44:22Z

Ok, it looks like all the identified blockers are solved. In order to reduce the risk we're going to wait to merge this PR until just after the Atom Railcar Release for 1.39 is done (which is expected on July 19th).

Until then, I'll be using a version of Atom built from this PR to see if I can find any other regression.

mfonville · 2019-07-06T16:09:27Z

package.json

@@ -12,7 +12,7 @@
    "url": "https://github.com/atom/atom/issues"
  },
  "license": "MIT",
-  "electronVersion": "3.1.10",
+  "electronVersion": "4.2.5",


4.2.6 is also available

@rafeca I guess the 4.2.7 assets were not live yet when you tried to push the update

Yup, I tried 4.2.7 and it didn't work well (the release notes were not even published). I'm gonna upgrade to 4.2.7 now

mfonville · 2019-07-18T19:16:39Z

Will Electron 4 be merged for the 1.41-dev branch?

rafeca · 2019-07-19T15:42:15Z

Will Electron 4 be merged for the 1.41-dev branch?

Yup, I'm going to merge this PR once the CI passes. Since we've just rolled the railcars for v1.40.0, Electron v4 going to be available on v1.41 (probably from v1.41.0-nightly1).

mfonville · 2019-08-30T11:23:58Z

For 1.41 you can maybe upgrade Electron to the latest update 4.2.10?

rafeca added 3 commits May 20, 2019 15:31

⬆️ electron@4.2.0

5571a7c

⬆️ electron-chromedriver@4.0.0-beta.1

eb2f091

⬆️ electron-mksnapshot@4.2.0

abaaf75

nathansobo mentioned this pull request May 20, 2019

Crash in mksnapshot when building with Electron 4 #19374

Closed

Merge branch 'master' into electron-4

a4c6f31

mfonville reviewed May 21, 2019

View reviewed changes

daviwil and others added 12 commits May 21, 2019 14:56

Use new attributeStyleMap for elements

218657f

More info here: https://developers.google.com/web/updates/2018/03/cssom

Use correct fontWeight value in ThemeManager spec

4c47249

In Chrome 66+ it seems that getComputedStyle().fontWeight returns the computed numeric value of the style instead of the original descriptive name. We now look for value '700' which corresponds to the value of 'bold'.

Forward-port chromedriver changes

d988983

⬆️ apm prerelease for updated Node

a1aa1bb

⬆️ @atom/nsfw, git-utils to allow loading on Windows

1483f4b

And a bump for fuzzy-finder to remove unnecessary dependencies that were causing installation issues

⬆️ scandal to update nested git-utils dependency

906a106

Temporarily disable snapshotting

cfaca0a

Try updating @atom/nsfw to 1.0.23-1

7f5e639

Let's see if it fixes the config watching warnings...

Oops, package lock it

c2f1d65

Don't add menu items without labels

a186961

Update Electron builtins

10c1c97

Investigate chromedriver failures

00362cc

as-cii mentioned this pull request May 29, 2019

Don't create a promise when preloading package settings in snapshot #19418

Merged

50Wliu added 4 commits May 30, 2019 09:22

Merge branch 'master' into electron-4

1ebfcba

Re-enable snapshotting

3e6d7cc

Merge branch 'master' into electron-4

5f0aad2

Back to manual debugging it is

6e90fbe

Arcanemagus added the electron-4 label Jun 6, 2019

rafeca added 4 commits June 15, 2019 10:26

Merge branch 'master' into electron-4

5ddb665

⬆️ electron-chromedriver@4.2.0

8ad7aa0

Revert "Back to manual debugging it is"

60093e4

This reverts commit 6e90fbe.

Arcanemagus added the electron label Jun 24, 2019

rafeca mentioned this pull request Jun 25, 2019

Iteration Plan: June 24 - July 5, 2019 #19593

Closed

8 tasks

Arcanemagus mentioned this pull request Jun 25, 2019

Upgrade to Electron 3.1.10 #19419

Merged

nathansobo assigned rafeca Jun 26, 2019

This was referenced Jun 27, 2019

Send OS version to the autoupdate endpoint #19607

Merged

Native tabs super slow on Electron v4 #19611

Closed

rafeca marked this pull request as ready for review July 5, 2019 12:44

mfonville reviewed Jul 6, 2019

View reviewed changes

⬆️ electron@4.2.6

79a952b

rafeca force-pushed the electron-4 branch from c9a290a to 79a952b Compare July 16, 2019 10:32

IAMCRISTAL approved these changes Jul 17, 2019

View reviewed changes

rafeca added 2 commits July 19, 2019 17:35

Merge branch 'master' into electron-4

f7f002f

⬆️ electron@4.2.7

a8e2c83

rafeca merged commit e0a377e into master Jul 19, 2019

rafeca deleted the electron-4 branch July 19, 2019 16:38

vinkla mentioned this pull request Jul 22, 2019

Text Rendering on macOS changed with Electron 3.0 #19356

Closed

ashthespy mentioned this pull request Aug 5, 2019

Check if running on Electron (#99) paulmillr/readdirp#102

Closed

Arcanemagus mentioned this pull request Aug 22, 2019

Mojave dark mode #18137

Closed

connorlr mentioned this pull request Oct 23, 2019

Electron 4 upgrade causes thin font rendering on MacOS 10.14+ #20052

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Electron 4 #19373

Upgrade to Electron 4 #19373

nathansobo commented May 20, 2019 •

edited by rafeca

mfonville May 21, 2019

UziTech May 21, 2019

nathansobo May 21, 2019 •

edited

nathansobo May 21, 2019

mfonville Jun 1, 2019

mfonville Jun 5, 2019

mfonville Jun 19, 2019

50Wliu commented May 21, 2019 •

edited

Asday commented May 23, 2019

rafeca commented Jun 25, 2019 •

edited by as-cii

jasonrudolph commented Jun 25, 2019

rafeca commented Jun 25, 2019

rafeca commented Jun 26, 2019

jasonrudolph commented Jun 26, 2019

nathansobo commented Jun 26, 2019

rafeca commented Jun 28, 2019

rafeca commented Jul 5, 2019

mfonville Jul 6, 2019

mfonville Jul 16, 2019

mfonville Jul 18, 2019

rafeca Jul 19, 2019

mfonville commented Jul 18, 2019

rafeca commented Jul 19, 2019

mfonville commented Aug 30, 2019

Upgrade to Electron 4 #19373

Upgrade to Electron 4 #19373

Conversation

nathansobo commented May 20, 2019 • edited by rafeca

Issues to resolve

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathansobo May 21, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

50Wliu commented May 21, 2019 • edited

Asday commented May 23, 2019

rafeca commented Jun 25, 2019 • edited by as-cii

General testing

Check for regressions experienced in previous upgrades

Text Input/Keybindings

UI

Other

jasonrudolph commented Jun 25, 2019

rafeca commented Jun 25, 2019

rafeca commented Jun 26, 2019

client side changes

atom.io changes

jasonrudolph commented Jun 26, 2019

nathansobo commented Jun 26, 2019

rafeca commented Jun 28, 2019

rafeca commented Jul 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mfonville commented Jul 18, 2019

rafeca commented Jul 19, 2019

mfonville commented Aug 30, 2019

nathansobo commented May 20, 2019 •

edited by rafeca

nathansobo May 21, 2019 •

edited

50Wliu commented May 21, 2019 •

edited

rafeca commented Jun 25, 2019 •

edited by as-cii