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

Major Advanced File Uploader Bug - doesn't work on Mac OS & Safari #2514

Closed
black-eye opened this issue May 17, 2018 · 10 comments
Closed

Major Advanced File Uploader Bug - doesn't work on Mac OS & Safari #2514

black-eye opened this issue May 17, 2018 · 10 comments
Assignees
Labels
bug:major Has PR Bug/Feature with a PR created for it.
Milestone

Comments

@black-eye
Copy link

black-eye commented May 17, 2018

What Version Of Caldera Forms, WordPress and PHP Are You Using?

WordPress Version: 4.9.5PHP Version: 7.1.17MySQL Version: 10.1.26Caldera Forms Version: 1.6.1.1WP_DEBUG:

What Is The Unexpected Behaviour?

When my client (on Mac OS & Safari) tried to send the form here: https://www.humangarden.com/test-web/obsazovane-pozice/, form get stucked after submitting. It happened just in Safari, in Chrome it was OK. When I switched the field to simple (non advanced) uploader, it was OK.

On the other hand, while using simple uploader I'm unable to send multiple files, even though this option is checked.

What PHP Errors Have You Logged While Reproducing This Bug?

Unfortunately I'm on Windows and client is unable to provide this info.

What JavaScript Errors Have You Seen While Reproducing This Bug?

Unfortunately I'm on Windows and client is unable to provide this info.

@black-eye black-eye changed the title Major Advanced File Uploader Bug - doesn't work on iOS & Safari Major Advanced File Uploader Bug - doesn't work on Mac OS & Safari May 17, 2018
@black-eye
Copy link
Author

black-eye commented May 18, 2018

UPDATE:
To isolate the problem to its core, I created a clean WP install just to test the upload form, here it is: https://www.humangarden.com/prac/caldera-upload/

Just WP 4.9.5, Twenty Seventeen Theme and Caldera 1.6.1.1. nothing else. Problem stays.

Please make a repair ASAP. It makes the forms with advanced upload fields unusable on Mac.
Important note: the form isn't submitted even though, you haven't upload any file - the problem is the pure presence of the field in the form.

And it's not just UI problem, the data aren't submitted either.
Big thanks.

@Shelob9 Shelob9 added this to the 1.6.3 milestone May 21, 2018
@Shelob9 Shelob9 self-assigned this May 21, 2018
@Shelob9
Copy link
Collaborator

Shelob9 commented May 21, 2018

Thanks for the details. Looks like the file upload request works fine, but then Safari is getting a 400 error from nginx (on your test site and in my local test environment) for the form submit request. It's never gets to WordPress at all when making the request to upload.

This is probably going to require rewriting form submission from scratch. Fuck.

@black-eye
Copy link
Author

black-eye commented May 21, 2018

Ouch, sounds like bad news. :(
I suppose this will take some time to figure it out.

As a workaround, I would be fine using the simple uploader, but it has it's own bug too. It won't allow selecting multiple files. Even though I check corresponding option, the attribute "multiple" is not added to the input tag code.

And one more question concerning simple uploader - what is the real purpose of the extra button "Add Files", which is added, when selecting Multiple?

Thanks

@chemann
Copy link

chemann commented May 22, 2018

Same major problem with WP 4.9.6. Caldera Forms 1.6.3. Not possible to use advanced file upload from Chrome nor FireFox. Some of help? Only simple file upload works.

Worried about new customers arriving to the web, not proper image of our enterprise and possible leads missing...

@Shelob9
Copy link
Collaborator

Shelob9 commented May 26, 2018

I am done with GDPR stuff and back to this.

Here is what I know

Theories

If a file upload field is in the form we create a new XMLHttpRequest object, and then modify it and that should not_ effect how other HTTP requests works, but clearly something is.

This least invasive fix would be to change how that works. Also:

We have var xhr = new window.XMLHttpRequest();

But in the docs for Safari they say to do var myRequest = new XMLHttpRequest(); and note that's not IE compatible. we've come full circle.

@Shelob9 Shelob9 modified the milestones: 1.6.3, 1.7.1 Jun 6, 2018
@Shelob9
Copy link
Collaborator

Shelob9 commented Jun 6, 2018

Notes on my call with Bluma:

  • In Bluma's VM with Safari, we can not reproduce this. Does that tell us something about the event bindings? It must because a remote desktop controlling a VM is simulating click events.
  • In Chrome and Safari, the form submit request is sending an octet stream for the file field. That's wrong, and is likely why the server is rejecting the form submit request.
  • We wrote a hack solution to jQuery.remove() the file field (its wrapper and everything inside) after it is uploaded. that made the submission complete, but the the file field was not associated with the submission, even though it was saved. This is because removing the DOM node lost the control key that links the two submissions.

Proposed solution:
Just removing the DOM node isn't acceptable. If there is a validation error, the file field has to still be there for re-submit. Also, we loose the link for submissions.

I need to either modify how formData is created or how the file field changes after submission. Goal is to prevent uploading octet stream again, but to keep control linked.


Content-Disposition: form-data; name="fld_2747627"; filename=""
Content-Type: application/octet-stream


------WebKitFormBoundaryoPc5IcFYLyQ4Q98l
Content-Disposition: form-data; name="fld_2747627"

trupl5b17fdd4b8aee
------WebKitFormBoundaryoPc5IcFYLyQ4Q98l


@Shelob9
Copy link
Collaborator

Shelob9 commented Jun 6, 2018

Both the hidden field with control (bottom) and the file field with the file have the same name. 87% sure that's the issue

Possible places I broke this:

@Shelob9
Copy link
Collaborator

Shelob9 commented Jun 6, 2018

So, the actual problem is that in the request to submit the form sends in POST request for that field the file (not needed) and the control ID we need for this to actually work. That's bad, Chrome handles it in a way that nGinx can make sense of the way we want it to, and the way Safari and Edge send it, shit goes wrong.

I hate how simple the solution I have is. But, once the file is uploaded we don't need it anyway, even if the validation fails, because the data is already on the server. We need to clear the file field out before form serialization happens. So, once the file is pushed into the upload, we can turn the hidden file field that advanced file upload UI is wrapped around into a hidden field, and empty it's value.

NOTE TO FUTURE SELF: Always use jQuery.val('') to empty a field fields value before setting it to anything else.

@Shelob9
Copy link
Collaborator

Shelob9 commented Jun 6, 2018

🌋 This should be fixed via #2588 🌋

Shelob9 added a commit that referenced this issue Jun 7, 2018
Do not transmit file field data again, preventing submission on Safari and Edge Fixes #2514
@Shelob9
Copy link
Collaborator

Shelob9 commented Jun 7, 2018

🔥 🌋 Close via #2588

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:major Has PR Bug/Feature with a PR created for it.
Projects
None yet
Development

No branches or pull requests

3 participants