Electron v9.1.2

[rafeca]

⚠️ This PR is based on #9766 (I can't use it as the base branch of this PR since it's in a fork).

Related issue: #10085

Description

Updates the Electron from v8.4.0 to v9.1.0

This will primarly fix a read: ENOTCONN error we're seeing more frequently on Electron 7 due to the node version.

Breaking changes

Split shell.openItem(path) into synchronous and asynchronous methods (more info).

This has been addressed in this PR (commit 7d4a65e).

Note that this needs to get fixed at the same time as upgrading to v9 since that breaking change had no transition/deprecation plan.

Major changes

Chromium got upgraded from v80 to v83.

• Release notes for v81
• Release notes for v83

(Chromium v82 release got cancelled due to covid-19)

Node.js got upgraded from v12.13.0 to v12.4.1.

• Release notes for v12.13.1
• Release notes for v12.14.0
• Release notes for v12.14.1

V8 got upgraded from v8.0 to v8.3.

• Release notes for v8.1
• Release notes for v8.3

(V8 v8.2 release got cancelled due to covid-19)

Chromium has changed the styling of form controls (more info).

I noticed a few subtle differences in the app caused by this (not big changes though since we have custom styling for most of the form controls):

Before	After

I think the only want that we may want to address is the styling of the "clear" icon on search inputs.

Full changelog

Changelog of all major/minor Electron releases (these links may break when new Electron versions get published since the pagination in their changelog is not stable):

• v9.0.0
• v9.1.0

Known issues in Desktop

Development extensions not working

Currently the development extensions (React devtools, Chromelens, etc) are broken with Electron v9.

This is due to a big internal change in how extensions work in electron v9 (see this issue and this PR) which is not documented in the release notes.

There are a few underlying issues with extensions:

1. Extensions cannot be activated when using file:// URLs (issue).

   In Desktop we load the app using a file:// URL so this is completely blocking us.

2. electron-devtools-installer needs to add CRX3 support to be able to install React DevTools.

   There's a PR open for this, so we need to wait until it's merged and shipped. Edit: The PR has been shipped so this is no longer an issue!

3. There are reports of extensions failing to load when activated just after launching Electron.

   This is a Chromium issue (link to the issue). I don't yet know if this is going to affect us, but if it does we'll need to wait for a new Chromium release and a new Electron release that includes it.

Layout changes related to flexbox (again)

With the new version of Chromium we're experiencing more layout issues. All the issues we've lately experienced when upgrading Electron seem to be related to the gradual transition that Chromium is doing towards the new layout engine LayoutNG.

In this specific case, when using a SVG element as a flexbox item, Chrome will not be able to calculate correctly the width of the SVG element and will set it to its default size (300px):

<div style="display: flex; width: 500px; height: 16px;">
  <div style="flex: 1">Flexbox item</div>
  <svg style="height: 16px; flex-grow: 0; flex-shrink: 0; background-color: red" version="1.1" viewBox="0 0 16 16">
    <path d="M7.48 8l3.75 3.75-1.48 1.48L6 9.48l-3.75 3.75-1.48-1.48L4.52 8 .77 4.25l1.48-1.48L6 6.52l3.75-3.75 1.48 1.48L7.48 8z"></path>
  </svg>
</div>

The code above gets rendered as:

Under normal situations, by defining the height of the SVG the render engine should be able to infer its width using the viewBox information but in this case it doesn't and it default to the default 300px size.

⚠️ Only in experimental mode

The most interesting part is that this only happens when Electron is configured to tell Chromium to enable its experimental features (which we do in Desktop 😅).

This flag enables the Chromium experimental Web Platform Features. I tried enabling this same flag on my Chrome Web browser (via the about://flags interface) but couldn't reproduce the issue in Chrome 84 (I'm using this codepen), so it's possible that the issue got fixed and it never reached stable Chrome 🙃

For now I've worked around this problem by wrapping the SVGs that caused problems under a <div/> in c092ccf. This is a harmless change and even if it didn't fix this specific bug it wouldn't be a change to worry about imho.

As a next step initiative, I think we should disable the experimental Chrome features in Desktop. They were enabled a long time ago in this PR to overcome a limitation at that time (the ResizeObserver API was only enabled as an experimental feature).

This was done when Desktop used v1.6.3, which used Chromium v56.0.2924.87 (more info), and the ResizeObserver API was enabled in Chrome v64 so we shouldn't need the experimental flag at least for that.

I didn't want to disable the flag on this PR since it's possible that we're relying on experimental features somewhere else in the app (or in other rendering quirks that happen only when this flag is enabled) and I prefer this PR to focus only on Electron v9.

(As an additional data point, note that the Electron docs don't recommend enabling experimental features - source).

Release notes

Notes: No notes

[rafeca]

The current build failure is caused by the fs-admin package, which latest published release doesn't include prebuilt binaries for Electron v9.

Good thing is that there's an already merged PR that adds support for it, so we only need a new version of the package to get published (I've talked to @shiftkey , the maintainer of the package, and he's planning to publish a new version in the upcoming days).

Once this gets fixed, builds will hopefully start passing 😃

[shiftkey]

@rafeca please update fs-admin to v0.15.0 as that includes prebuild support for Electron 9

[rafeca]

Seems like CI is passing now! The only blocking thing that we need to solve now is the installation of devtools extensions for development.

[say25]

Electron 9.1.1 + 9.1.2 have dropped recently, with bug fixes.

[rafeca]

As discussed with @niik, I've ended up disabling the Web Experimental features in this PR (see
3d84bc8) since we already need to do full regression tests here. This allows us to revert the commit that wrapped SVGs around divs (c092ccf).

[niik]

A few humble requests for the emoji workaround

[niik]

Fantastic work with the PR description here @rafeca, really helpful.

I did an audit of all the open issues labeled for electron 9 in the electron repository to see if I could find anything scary and I've outlined the results below. TL;DR is I don't think any of these are blockers for merging.

electron/electron#23726 - Unspecific crash, good idea to keep a link around should it pop up after release
electron/electron#23852 - Reports of custom protocols not working on macOS when the app is closed (cc @tierninho to test this once we've got an actual build out)
electron/electron#23859 - Crash when calling preventDefault in the new-window event. We do this but we're not running the app in sandboxed mode so we should be safe
electron/electron#24135 - Crash during wake/sleep on macOS with multiple displays (worth keeping in mind if we see issues with this but I haven't been able to reproduce it)
electron/electron#24173 - Windows hangs due to new default value of allowRendererProcessReuse. We've temporarily reverted that property to avoid it.
electron/electron#24426 - We've address the emojis on both Windows and macOS in #10330 and https://github.com/desktop/desktop/blob/78e048fbdd6127c3e6c07772b442e75d6195665b/app/src/lib/fix-emoji-spacing.ts.

[tierninho]

Performed a sweep of tests on both OSX/Win and nothing was spotted outside of the two bugs filed (#10340 and #10341). Additionally, I did not directly experience any of the open Electron issue above. All appears to be functioning as intended, so I believe we are ready for the next step.