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

expired-callback not working as expected #11

Closed
rdrenker opened this issue Jan 31, 2017 · 7 comments
Closed

expired-callback not working as expected #11

rdrenker opened this issue Jan 31, 2017 · 7 comments
Labels

Comments

@rdrenker
Copy link

The 'expired-callback' located within the "recaptcha.component.ts" is not working as expected - specifically, its not emitting a null via the "resolved" EventEmitter;

I verified that the call back is fired, ie: the following code does result in "reset()" being executed:

 'expired-callback': () => {
        this.zone.run(() => this.reset());
      },

However, within the reset method, the code never makes it past the "this.grecaptcha.getResponse" check; ie:

if (this.grecaptcha.getResponse(this.widget)) {
        // Only reset the captcha and emit an event in case if something would actually change.
        // That way we do not trigger "touching" of the control if someone does a "reset"
        // on a non-resolved captcha.
        this.grecaptcha.reset(this.widget);
        this.resolved.emit(null);
      }

It seems in every test I've performed, this method always returns an empty string after the recaptcha expires.

I'm not sure the best way to fix this; personally, I would think a good design would be to expose a new responseExpired EventEmitter, and let the developer subscribe to that to explicitly handle expiration; for example, perhaps we don't want to do an automatic reset of the recaptcha upon expiration.

@DethAriel
Copy link
Owner

I will post various scenarios below:

  • User solved recaptcha, then recaptcha expires. Expected result: there is a (resolved) event
  • User did not solve recaptcha, then recaptcha expires. Expected result: there is no (resolved) event

Can you clarify which of the cases does not work, please? Just so you know, it's not the lib who controls whether we reset the captcha after expiration - it's the recaptcha itself that gets reset. So you cannot avoid automatic captcha reset after it has expired.

@rdrenker
Copy link
Author

rdrenker commented Feb 1, 2017

In my specific scenario, I solved the recaptcha, then sat around and waited for it to expire - so the first scenario. I have not tested the second scenario (and actually, I didn't even realize an unsolved recaptcha could expire).

Regarding your statement "it's the recaptcha itself that gets reset" - you are absolutely correct, however when this happens it shows the following text within the widget: "Verification expired. Check the checkbox again."; this is kind of nice as it provides a bit of feedback to the user as to why the recaptcha was reset. Conversely, calling the reset() method exposed by the component completely resets the recaptcha, and this info message is lost.

@DethAriel
Copy link
Owner

So the real issue here is that "Verification expired. Check the checkbox again." message is lost, am I getting this right?

@DethAriel
Copy link
Owner

With regards to my second scenario - yeah, that's probably not what's happening, something must have messed up my mind

@rdrenker
Copy link
Author

rdrenker commented Feb 1, 2017

No - that's just a nicety - the real issue is that the response event is never fired, which means i don't know that the recaptcha is no longer valid.

So take this sample code for instance:

resolved(recaptchaResponse: string) {
    this.criteria.recaptchaResponse = recaptchaResponse;
}

So when I first solve the recaptcha, my criteria object gets the big long recaptcha data string. When I sit around for a bit, the recaptcha expires, but the resolved() event is not fired, so I'm not being notified that I need to clear out that value from my criteria object.

From my investigation, its not being fired because the following code

this.grecaptcha.getResponse(this.widget)

within the reset() method is returning an empty string

@DethAriel
Copy link
Owner

please update to v1.5.3

@rdrenker
Copy link
Author

rdrenker commented Feb 1, 2017

Works like a charm!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants