-
Notifications
You must be signed in to change notification settings - Fork 829
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
[GH-2857] Allow only macOS to call setSecureKeyboardEntryEnabled #2860
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick fix @devinbinnie!
if (process.platform !== 'darwin') { | ||
return; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any tests we can add for this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None that would add any value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have the username/password inputs covered in E2E tests? That would be a good one to have to catch any of these type of issues in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes many E2E tests require a login so those should be catching this.
@yasserfaraazkhan heads up that we are including this fix for #2857 in the upcoming dot release so we can test post-merge in the RC build. |
Cherry pick is scheduled. |
…abled (mattermost#2860) (cherry picked from commit 2a88175)
…) (#2861) (cherry picked from commit 2a88175) Co-authored-by: Devin Binnie <[email protected]>
Summary
It would appear that the
app.setSecureKeyboardEntryEnabled
function is undefined for Windows and Linux, instead of escaped for those OSes since it doesn't affect them.This PR ensures that we only call this function for macOS.
Ticket Link
Closes #2857