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

Directly require clipboard from Electron #16526

Merged
merged 4 commits into from Aug 24, 2018

Conversation

Projects
None yet
2 participants
@50Wliu
Member

50Wliu commented Jan 10, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

As far as I can tell the "safe-clipboard" workaround was for an Electron bug that was fixed somewhere around atom-shell 0.24.0. Searching for "clipboard linux" on the electron issues list doesn't yield any crash results and the docs don't have anything Linux-specific, so I think that we should be good now.

Alternate Designs

None.

Why Should This Be In Core?

This cleans up some old code present in core.

Benefits

Requiring the clipboard is more straightforward.

Possible Drawbacks

It is possible that the crash still exists and I'm searching for the wrong thing.

Verification Process

I currently don't have a Linux distribution set up, so I'm unable to verify that using the renderer clipboard does not crash Atom.

Applicable Issues

Refs electron/electron#1372

50Wliu added some commits Jan 5, 2018

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Aug 24, 2018

Contributor

I just tested this on my Ubuntu VM. Requiring clipboard directly from electron and calling writeText and readText on it worked just fine from the renderer process.

Contributor

maxbrunsfeld commented Aug 24, 2018

I just tested this on my Ubuntu VM. Requiring clipboard directly from electron and calling writeText and readText on it worked just fine from the renderer process.

@maxbrunsfeld maxbrunsfeld merged commit 1668617 into master Aug 24, 2018

0 of 3 checks passed

VSTS: Atom Pull Requests queued
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Aug 24, 2018

Contributor

Thanks @50Wliu!

Contributor

maxbrunsfeld commented Aug 24, 2018

Thanks @50Wliu!

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