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

Setup protocol after ready has already fired #6095

Merged
merged 1 commit into from Jun 17, 2016

Conversation

Projects
None yet
3 participants
@kevinsawicki
Contributor

kevinsawicki commented Jun 16, 2016

Looks like in 1.2.3 browser/api/protocol.js is no longer eagerly required so the properties/methods are missing on it since the ready event on app has already been fired when required later on and so the properties from session.defaultSession.protocol were never copied over to the exports object.

This pull request adds a check app.isReady() to handle this case.

I think it might have been introduced by this line, https://github.com/electron/electron/pull/5904/files#diff-8d85c651cb0fb8feef2c4694d52da2e3L1

Closes #6094

@paulcbetts

This comment has been minimized.

Contributor

paulcbetts commented Jun 16, 2016

I feel like in general, the ready event should immediately fire if someone subscribes to it after app is ready, but this fixes the short-term problem at least

@zcbenz

This comment has been minimized.

Contributor

zcbenz commented Jun 17, 2016

👍

@zcbenz zcbenz merged commit 66fe1e4 into master Jun 17, 2016

8 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
electron-linux-arm Build #3463074 succeeded in 41s
Details
electron-linux-ia32 Build #3463075 succeeded in 35s
Details
electron-linux-x64 Build #3463076 succeeded in 123s
Details
electron-mas-x64 Build #1567 succeeded in 6 min 12 sec
Details
electron-osx-x64 Build #1573 succeeded in 5 min 48 sec
Details
electron-win-ia32 Build #571 succeeded in 6 min 17 sec
Details
electron-win-x64 Build #561 succeeded in 6 min 20 sec
Details

@zcbenz zcbenz deleted the protocol-ready-bug branch Jun 17, 2016

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