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

Login: Social login button remains disabled if Google SDK fails to init #81152

Merged

Conversation

p-jackson
Copy link
Member

Fixes error raised by p1693237148930679-slack-C04U5A26MJB

Proposed Changes

  • loadGoogleIdentityServicesAPI() will returns a falsey value when the Google SDK fails to set up the globals correctly.

It's unclear why this might happen, but we have seen it happen in production. The code is already designed to handle this case. When loadGoogleIdentityServicesAPI() returns falsey the social login button stays disabled and the user must use a different login/signup method.

Testing Instructions

The usual way of testing—blocking Google scripts—won't work here. In this situation there was no exception thrown by loadScript() so Google's SDK had no issue loading. In this case I believe we just need to visually confirm that this code change is safe.

And also run through the happy login path.

@p-jackson p-jackson requested a review from zaguiini August 28, 2023 21:44
@p-jackson p-jackson self-assigned this Aug 28, 2023
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 28, 2023
@github-actions
Copy link

github-actions bot commented Aug 28, 2023

Copy link
Contributor

@zaguiini zaguiini left a comment

Choose a reason for hiding this comment

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

That makes sense, let's see if it silences the error for good.

Should we also check for the existence of window? That feels paranoid, but 🤷

client/components/social-buttons/google.js Outdated Show resolved Hide resolved
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@p-jackson p-jackson force-pushed the fix/social-buttons-disabled-if-google-fails-to-setup branch from dbbfbbe to e1f6d24 Compare August 28, 2023 21:54
@p-jackson p-jackson force-pushed the fix/social-buttons-disabled-if-google-fails-to-setup branch from e1f6d24 to aeb13d7 Compare August 28, 2023 22:03
@p-jackson p-jackson merged commit 2769f2a into trunk Aug 28, 2023
@p-jackson p-jackson deleted the fix/social-buttons-disabled-if-google-fails-to-setup branch August 28, 2023 22:26
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants