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

Upgrade to Electron 2.0.0 #17273

Merged
merged 14 commits into from May 11, 2018

Conversation

Projects
None yet
@jasonrudolph
Member

jasonrudolph commented May 4, 2018

This pull request is a work-in-progress. Electron 2.0.0 includes a few breaking API changes that we'll likely need to work through in order to get Atom running smoothly on Electron 2.0.0.

TODO

  • Get CI passing
  • Run through Atom's Electron upgrade test manifest (below) to identify any regressions

Regressions to fix before merging

Known bugs resolved by this upgrade

Fixes #12338

Electron Upgrade Test Manifest

  • Review issues tagged with electron to see if any of them are fixed

  • 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
    2. Share a portal
    3. Have a guest join the portal
    4. When guest edits the text in the file, verify that the host sees those changes

Text Input/Keybindings

See how to setup keyboard layouts.

  • IME not working (#14911)

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps
    1. Install an IME layout, for example Pinyin simplified 1. Open Atom and use the layout you installed as the keyboard layout
    2. Example, if you picked Pinyin simplified type in zhongwenshuru

    Expected: in Pinyin simplified 中文输入 is expected

    gif

    Issues we've seen: Only shows latin first character and not every character typed.

    gif

  • macOS: Key binding with composition characters are ignored (#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 (#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:
    bug

  • Ubuntu: Keystrokes involving ctrl 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

UI

  • tree-view drag image (atom/tree-view#1054)

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps
    1. Open a project in Atom with multiple files and a few folders
    2. Drag a file into a folder

    Expected:
    image

    Issues we've seen:

    image

  • drag-and-drop indicator (atom/tree-view#1055, atom/tabs#426, atom/tabs#437)

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps
    1. Open Atom with a few folder projects on tree-view (you can do this on terminal in a directory that has existing folder and having a space between each folder. Example: atom folderA folderB folderC)
    2. Drag and drop all of the folders to reorder them on Atom

    Expected:
    The placeholder indicates where the folder/project is being dropped to.

    screen shot 2017-03-29 at 16 31 16

    Issues we've seen:
    No placeholder shows up after drag and dropping

    screen shot 2017-03-29 at 16 30 51

  • Large file rendering (#16591)

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps
    1. Open a file that has over one million lines
    2. Scroll down to the bottom

    Expected: To not regress the number of lines that can be rendered. Rendering to be correct for lines past a certain point. Atom 1.25 can render around 800k to 1 million lines correctly.

    Issues we've seen: Increased number of lines rendered but bad rendering past a certain point

    bad renderer

  • Loss of subpixel AA when the cursor is at the end of long lines (#16889, #16595)

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps
    1. Open text-editor.js in Atom
    2. Copy the first 21 lines to a new untitled window
    3. Turn on soft wrapping and change language mode to javascript
    4. Resize the Window so it's soft wrapped
    5. On line 15 type a until the you reach the end of the window

    Expected: To not lose subpixel AA

    Issues we've seen: Loss of subpixel AA. Both when soft wrapping was enabled and disabled.

    subpixel aa loss softwrap

  • Scrolling horizontally shift + scroll wheel (#12696 (comment))

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps
    1. Open a file that has long lines
    2. Turn off soft wrapping and resize the window so you have a horizontal scrollbar
    3. Press shift and scroll in both directions using the mouse wheel

    Expected: File to scroll horizontally in both directions

    Issues we've seen: File does not scroll horizontally at all in any direction

  • Scrollbars misbehaving on the first file that is opened (#12696 (comment))

    Repro steps
    1. Open Atom
    2. Open some file that has content enough to have a vertical scrollbar
    3. Close Atom
    4. Open Atom and look at the vertical scrollbar

    Expected: Scrollbar to be visible

    Issues we've seen: Scrollbar is not visible and is flickering when you are editing

  • Middle clicking on unsaved tab (#15197)

    • macOS
    • Ubuntu
    Repro steps
    1. Open Atom and open a new unsaved file.
    2. Make edits to the unsaved file and middle click on the tab.

    Expected:
    Clicking save/cancel or the options on the dialog works.

    Issues we've seen:
    The UI and the dialog is unable to receive mouse clicks. You can still choose options via Keyboard, but not mouse.

  • Linux: Atom scrolls even when not focused (#15482)

    Repro steps
    1. Open Atom with a large enough file that has a scrollbar
    2. Open a different application and place it over the Atom window
    3. Scroll with the mouse inside the other window
    4. Focus the Atom window

    Expected: Atom window should keep the original scroll position

    Issues we've seen: The Atom window scrolls after it is focused

Other

  • Deprecation warnings (#12696 (comment))

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps

    This should be tested on macOS, Linux and Windows with community packages installed

    1. Open Atom
    2. Open the Developer tools console on the master branch and check for deprecation warnings
    3. Open the Developer tools console on the electron upgrade branch and check for deprecation warnings
    4. Compare the two outputs

    Expected: No new deprecation warnings

    Issues we've seen: New deprecation warnings both from core and community packages

  • Supported Versions of OS (#15297)

    • macOS
    • Windows 10
    • Ubuntu
    Repro steps
    1. Check the list of supported platforms Electron Master
    2. Check what version is listed as minimum for each platform on https://atom.io and https://atom.io/faq

    Expected: https://atom.io and https://atom.io/faq to display the same versions as the Electron documentation

    Issues we've seen: macOS version support changed and https://atom.io and https://atom.io/faq was not updated

  • macOS : Native tabs (#14711)

    Repro steps
    1. Open two atom windows on mac
    2. Go to View in menus and check Show Tab Bar
      screen shot 2017-06-03 at 5 10 32 pm
    3. Do this for both Atom windows.
    4. Drag one atom window to the other via tab bar

    Expected:
    Native Tab bar shows up and able to drag different windows to each other. Also make sure UI isn't messed up.

    Issues we've seen:
    Native Tab Bar doesn't work and messes up UI when enabled

  • Ubuntu with KDE: Menu uses 100% CPU (electron/electron#8455, #13885)

    Repro steps
    1. Install Atom on Ubuntu KDE.
    2. Enable kde plasma global menu in System Settings -> Application Style -> Widget Style -> Fine Tuning tab -> Menubar style -> Application Menu widget -> Add a menubar Panel to desktop
    3. Start atom
    4. Try to use the desktop menu

    Expected:
    Desktop menu items work

    Issues we've seen:
    Desktop menu items do nothing when you click on things.

  • macOS: Slovak QWERTZ (atom/atom-keymap#223)

    Repro steps
    1. Install Slovak keyboard layout on mac
    2. Open keyboard viewer to see the keys are are suppose to show up and open keyboard resolver on atom.
    3. hit cmd+'.

    Expected: Resolves to cmd+' like it shows on keyboard viewer on mac.

    Issues we've seen: It resolves to ctrl+§

  • macOS: IME jump (#15696)

    Repro steps
    1. Install an IME layout, for example Chinese pinyin
    2. Type one character

    Expected:
    Expected the IME to be positioned at under the character you entered
    Issues we've seen: The IME window is in the top left corner. When you enter the second character it jumps to be positioned under the text


/cc @codebytere

@codebytere codebytere self-requested a review May 4, 2018

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented on db88a07 May 4, 2018

Doh. The second URL in the commit message should be nodejs/node@403b89e.

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 4, 2018

The core render tests are passing now. 😅

Only two packages have failing tests: the archive-view package and the github package.

I'm hopeful that we can resolve the archive-view failures by:

  1. Publishing a new release of ls-archive that incorporates with the changes in atom/node-ls-archive#10
  2. Publishing a new version of archive-view that depends on the new version of ls-archive

I'll look into the github package failures next.

@smashwilson

This comment has been minimized.

Member

smashwilson commented May 5, 2018

I glanced at the failing GitHub package tests and it looks like they're all related to the prompt server. We use a Unix domain socket or a Windows named pipe to communicate with a subprocess spawned by git to be able to prompt for credentials through Atom dialogs... but those tests have always been a bit of a headache, because they're prone to timing problems and stalls when the OS decides to buffer data.

If there were changes to node's net module, even fairly subtle ones, that could cause our tests to fail and/or actually break the prompt.

I'd be happy to help figure out how to get them passing on here; I have fairly recent context on them 😄

@jasonrudolph jasonrudolph referenced this pull request May 6, 2018

Merged

⬆️ ls-archive@1.2.5 #62

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 7, 2018

Electron 2.0 uses Node 8.9.3. Atom's build currently uses Node 6.9.4 [A]. Should we update Atom's build to match the Node version used in Electron?

@as-cii: Do you happen to know whether the Node version in the Atom build needs to match the Node version that's in the version of Electron that we're using?


[A] There are a handful of places where we specify a dependency on Node 6.9.4. It's in .travis.yml, appveyor.yml, and circle.yml. And in the flight manual, we say that you need Node.js 6.x or later to build Atom.

@as-cii

This comment has been minimized.

Member

as-cii commented May 7, 2018

I believe it's not strictly required but I guess it might be nice to get them to match from a "language capabilities" standpoint, so that we can use the same set of language constructs and APIs for Atom as well as for build scripts.

@Arcanemagus

This comment has been minimized.

Contributor

Arcanemagus commented May 7, 2018

On the other hand, we should probably still have something checking that Node.js v6 still works to build Atom, at least until it's maintenance period ends or we drop support for it due to this:

And in the flight manual, we say that you need Node.js 6.x or later to build Atom.

@smashwilson

This comment has been minimized.

Member

smashwilson commented May 7, 2018

Lol, what

AssertionError: More than one application to run tests against was found. /home/travis/build/atom/atom/out/app/node_modules/github/lib/atom,/home/travis/build/atom/atom/out/app/node_modules/github/test/atom,/home/travis/build/atom/atom/out/atom-1.28.0-dev-358d6dd-amd64/atom

The lib/atom folder within atom/github is relatively new. I'm guessing that's just tripping up some path location logic?

@smashwilson

This comment has been minimized.

Member

smashwilson commented May 7, 2018

Okay, that's fixed. Those two dialog cancellation tests are still timing out on CircleCI, though.

jasonrudolph and others added some commits May 7, 2018

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 7, 2018

  • Get CI passing

😅

Big thanks to @smashwilson. ⚡️

@daviwil

This comment has been minimized.

Member

daviwil commented May 7, 2018

Nice work! I'm building this branch now to dogfood it until we're ready to merge this PR.

@daviwil

This comment has been minimized.

Member

daviwil commented May 7, 2018

Built and ran Atom successfully from this branch! One weird thing I noticed, our SVG-rendered Atom logo in the About screen is broken:

image

Did something change in the Chromium renderer related to SVG?

@daviwil

This comment has been minimized.

Member

daviwil commented May 7, 2018

@lee-dohm let me know that this problem shows up in 1.28-dev without the Electron 2.0 change, so that's not the culprit here!

@ckerr

This comment has been minimized.

Member

ckerr commented May 7, 2018

I'm super happy to see this getting unstuck. Thank you @jasonrudolph and @codebytere for doing this! 💚

@simurai

This comment has been minimized.

Member

simurai commented May 8, 2018

@daviwil One weird thing I noticed, our SVG-rendered Atom logo in the About screen is broken

Seems unrelated to this branch, happens on master too. Made a new issue: atom/about#78

@codebytere

This comment has been minimized.

Member

codebytere commented May 9, 2018

@jasonrudolph possibly! flying back to durham today, sorry for late follow-up!

@Ben3eeE

This comment has been minimized.

Member

Ben3eeE commented May 9, 2018

@jasonrudolph Running this branch as part of daily work to see if I notice anything. I also tested some of the items in the test plan. Most of the Windows things are checked off already though ⚡️

Big Files:
It seems the rendering limit is now ~2 million lines which is a huge improvement and the bad rendering is no longer happening and we are back to everything just being black. We still can't click on the content past ~1 million lines because we never get a click event. The vertical scrollbar limit also seems to have increased and is now the same as the rendering limit. I need to get another "really big file" soon because mine is only 2.8 million lines long 😅

image

Loss of subpixel AA:
I verified this and checked it off.

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 9, 2018

@codebytere: In case it helps, regarding the problem committing from the GitHub package (i.e., the suspected asar issue), it seems to be due a change that occurs somewhere between Electron 1.8.6 and Electron 2.0.0.

Committing from the GitHub package works when using the Electron 1.8.6 build of Atom from #16755. But it fails when using the Electron 2.0.0 build of Atom from this pull request.

@jasonrudolph jasonrudolph referenced this pull request May 9, 2018

Closed

Edit menu interferes with View menu on Linux #16766

1 of 1 task complete
@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 9, 2018

UPDATE: This doesn't appear to be a problem after all. See #17273 (comment) and #17273 (comment).


Steps to reproduce:

1. Open a new buffer
2. Type any text (e.g., abc)
3. Take a screenshot
4. Zoom in real close
5. Observe absence of subpixel rendering

Atom 1.27-beta (using Electron 1.7)

screen shot 2018-05-09 at 10 51 46 am

Atom 1.28-dev (using Electron 1.8.6)

screen shot 2018-05-09 at 10 59 02 am

Atom 1.28-dev (using Electron 2.0.0)

screen shot 2018-05-09 at 10 52 18 am


@codebytere @ckerr: Is this something you can help us resolve?

@lee-dohm

This comment has been minimized.

Member

lee-dohm commented May 9, 2018

@jasonrudolph Depending on the DPI of your screen, this may be expected. I'll try to find the Chromium bug tracker references I mention there for more details.

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 9, 2018

@jasonrudolph Depending on the DPI of your screen, this may be expected. I'll try to find the Chromium bug tracker references I mention there for more details.

@lee-dohm: Thanks for pointing that out! That inspired me to check on my Retina display, and I am indeed seeing subpixel rendering there. So, I'm going to remove that item from the list of regressions related to this upgrade.

@50Wliu

This comment has been minimized.

Member

50Wliu commented May 9, 2018

Here's a regression I'm seeing: branch name and push/pull status bar icons now show an underline when hovering whereas they didn't before.

github-underline

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 9, 2018

I'm seeing: branch name and push/pull status bar icons now show an underline when hovering whereas they didn't before.

@50Wliu: Thanks for pointing that out. I'm pretty sure that's a change in the GitHub package, and not a result of the Electron upgrade. I installed Atom from the current master branch (using Electron 1.7.14) and then installed github@0.14.0-3, and I'm seeing those same underlines.

I'll ping the folks working on the GitHub package to see if they're aware of that change.

@jasonrudolph

This comment has been minimized.

Member

jasonrudolph commented May 9, 2018

I'm trying to test the final item in the checklist, but I think the repro steps might be out of date:

  1. Install Atom on Ubuntu KDE.
  2. Enable kde plasma global menu in System Settings -> Application Style -> Widget Style -> Fine Tuning tab -> Menubar style -> Application Menu widget -> Add a menubar Panel to desktop

In the "Fine Tuning" tab, I don't see anything about "Menubar style."

image

@Ben3eeE or @daviwil: Would either of you happen to know what I need to do to replicate these setup steps? I'm using Ubuntu 18.04 and this version of KDE:

image

@jasonrudolph jasonrudolph changed the title from [WIP] Upgrade to Electron 2.0.0 to Upgrade to Electron 2.0.0 May 9, 2018

:arrow-up: github
as per @smashwilson's suggestion, fold github package `0.15.0` into the pr in case we find other blockers.  Also this way we don't ship a prerelease version and keep our minor-version-per-Atom-release pattern alive.

@jasonrudolph jasonrudolph merged commit c5dea46 into master May 11, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jasonrudolph jasonrudolph deleted the electron-2.0 branch May 11, 2018

jasonrudolph added a commit that referenced this pull request Jul 9, 2018

@atom atom deleted a comment Jul 9, 2018

@atom atom deleted a comment Jul 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment