Skip to content
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

Fix incorrect Google alert caused by changing user agent coresponding to current platform #384

Merged
merged 2 commits into from
Sep 15, 2021

Conversation

konhi
Copy link
Contributor

@konhi konhi commented Aug 19, 2021

Solves #327

Copy link
Collaborator

@Araxeus Araxeus left a comment

Choose a reason for hiding this comment

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

Could be slightly simplified using electron-is (it is already used throughout this program and imported in index.js line 6)

const userAgent = 
	is.macOS() ? userAgents.mac :
	is.windows() ? userAgents.windows :
	userAgents.linux;

(also because you were trying to change the value of a const after initialization)

but anyways did you check that google OAuth works? there is a comment saying firefox must be forced for it to work

@konhi
Copy link
Contributor Author

konhi commented Aug 20, 2021

Could be slightly simplified using electron-is (it is already used throughout this program and imported in index.js line

Thanks! This code looks much prettier! 💕

(also because you were trying to change the value of a const after initialization)

Is it a bad practice? If so, could you please give some reference? I'm not really a javascript expert! 😅

but anyways did you check that google OAuth works? there is a comment saying firefox must be forced for it to work

Good question! If you mean logging in, yes, it worked for me and Google didn't even alert me about anything suspicious! Anyway, I think it may be better option to still use Firefox user-agent.

@Araxeus
Copy link
Collaborator

Araxeus commented Aug 20, 2021

Is it a bad practice? If so, could you please give some reference? I'm not really a javascript expert! 😅

Well const = constant which means immutable value, shouldn't be changed after giving it its initial value.
https://developer.mozilla.org/en-US/docs/Learn/JavaScript/First_steps/Variables#constants_in_javascript

If the value is supposed to change later, you would probably use let instead of const
https://developer.mozilla.org/en-US/docs/Learn/JavaScript/First_steps/Variables#declaring_a_variable

Copy link
Owner

@th-ch th-ch left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the improvement! ✅

@th-ch th-ch merged commit 6dc0ba7 into th-ch:master Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants