Skip to content
This repository was archived by the owner on Sep 29, 2023. It is now read-only.

Fix app not starting since widevine PR / #1164#1170

Merged
TheCleric merged 1 commit into
masterfrom
fix-app-not-starting-since-widevine
Apr 30, 2021
Merged

Fix app not starting since widevine PR / #1164#1170
TheCleric merged 1 commit into
masterfrom
fix-app-not-starting-since-widevine

Conversation

@ronjouch

@ronjouch ronjouch commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

Fixes regression in f6e1ebd .

Also, fixing a typo I missed in the PR, and adding types for callback params.

Comment thread app/src/main.ts
}
});

if (appArgs.widevine !== undefined) {

@ronjouch ronjouch Apr 30, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition was excessive in a normal (non-widevine) build. In a normal build, appArgs.widevine is false (which is !== undefined). So, this widevine branch was taken even in a non-widevine build, and since widevine-ready isn't called here, onReady was never called and the main window was never created.

@ronjouch ronjouch Apr 30, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheCleric asking for a PR to be sure this is the fix you'd expect, or if instead your preferred fix is to avoid appArgs.widevine being false at this point. Feel free to push to my PR branch if you have more/different changes to do

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope looks good to me. Sorry for the bug!

@ronjouch ronjouch requested a review from TheCleric April 30, 2021 02:58
@TheCleric TheCleric merged commit c4356e4 into master Apr 30, 2021
@ronjouch ronjouch deleted the fix-app-not-starting-since-widevine branch April 30, 2021 05:27
Adam777Z pushed a commit to Adam777Z/nativefier that referenced this pull request Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants