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

Form emptied on submit, resulting in invalid form #106

Closed
rvanlaak opened this issue May 9, 2017 · 13 comments
Closed

Form emptied on submit, resulting in invalid form #106

rvanlaak opened this issue May 9, 2017 · 13 comments

Comments

@rvanlaak
Copy link
Contributor

rvanlaak commented May 9, 2017

@guillaumepotier while testing #105 on staging, another form we use Garlic on gets emptied / destroyed before submit. As a result of that no data is sent, and the form validation does not pass as the CQRF token also is cleared.

Narrowed this down to comparing 1.2.4 to 1.3.0, on which the bug occurs (so is not specific to PR #105 ): 1.2.4...master

The form initialization used to be like this:

$(function() {
    $('#orderInfoForm').garlic({
        excluded: '[data-persist="false"]'
    });
});

and restoring the form to a working situation was possible by:

$(function() {
    $('#orderInfoForm').garlic();
});

We now see that the form is emptied, but that the form submit works. I think the form should not be emptied, but only the storage should be emptied right?

@rvanlaak
Copy link
Contributor Author

rvanlaak commented May 10, 2017

@guillaumepotier I now found out that data is not even submitted at all. Any idea why 1.3.0 is not working for us on latest Chrome with jQuery 2.2?

@rvanlaak
Copy link
Contributor Author

FYI: I can now confirm that cherry-picking #105 on top of tag 1.2.4 resolves all issues.

We will start using this one in production: https://github.com/rvanlaak/Garlic.js/tree/pre-events

So there is some regression in the standard behavior since 1.3.0.

@guillaumepotier
Copy link
Owner

Hi @rvanlaak

I do not understand the problem here. I do not recall Garlic emptying values ever on a form. It just fills values with what it has in storage. Clearing/Emptying fields is really weird.

Could you provide a jsbin/jsfidle etc.. to show that behavior with latest release please?

Thanks

@cmer
Copy link

cmer commented May 11, 2017

I'm having the same issue. It also happens with capybara-webkit so it's not specific to Chrome.

@AlexeyKosov
Copy link

Ran into the same bug, had to revert to 1.2.4

@chamnap
Copy link

chamnap commented Nov 2, 2017

Facing the same issue. It's wierd. The textboxes contains the value, but the empty data got sent.

@chamnap
Copy link

chamnap commented Nov 2, 2017

@guillaumepotier, it's on this line, https://github.com/guillaumepotier/Garlic.js/blob/master/garlic.js#L118. Is it a typo or what? With this line of code, it will empty all inputs on submit....

@rvanlaak
Copy link
Contributor Author

rvanlaak commented Nov 3, 2017

@guillaumepotier what about merging #105 to resolve this? 👍

@turgs
Copy link

turgs commented Jan 26, 2018

So, it looks to me like #105 has been merged in, which from the comments above, suggest it may resolve this issue.

It doesn't. The issue still exists for me. Even if I remove all my garlicjs configuration options, and just call $('form').garlic() on the form, I still experience this problem. I'm using Firefox 58.

I've just tested going back to the prior 3 commits (I haven't gone back further) and this problem still exists. So add this point I don't think it's an introduced error, maybe it's a change in browser behavior?

@turgs
Copy link

turgs commented Jan 26, 2018

I've got it working. I agree with @chamnap that the error is on this line https://github.com/guillaumepotier/Garlic.js/blob/master/garlic.js#L118

I changed it from
$( this.parentForm ).on( 'submit reset' , false, $.proxy( this.remove, this ) );
to
$( this.parentForm ).on( 'submit reset' , false, $.proxy( this.destroy, this ) );
...and that fixed it for me.

@msanatan
Copy link

Confirming with @turgs and @chamnap, changing that line fixes the error

@guillaumepotier
Copy link
Owner

Ok guys, seems to be a nasty typo here.

Would you please submit a fix PR then?

Best

@soullivaneuh
Copy link
Contributor

Please see #116

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

No branches or pull requests

8 participants