Skip to content

Add a .once() guard on File module's AJAX uploads to prevent double-uploading #3212

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

Closed
quicksketch opened this issue Jul 23, 2018 · 8 comments

Comments

@quicksketch
Copy link
Member

quicksketch commented Jul 23, 2018

Describe your issue or idea

This is a related issue to the double-binding problem we have with AJAX events: #3210

That PR will solve the double-throbber problem in the most common case, but other AJAX events can result in the same double-binding problem for File module.

Steps to reproduce (if reporting a bug)

  • Add a post at node/add/post
  • For testing, open the developer tools in your browser and enable connection throttling down to "3G" in the network tab.
  • Upload an image (at least 2MB).
  • Remove the image.
  • Upload an image again.

Actual behavior (if reporting a bug)

  • During the second upload you'll see the throbber displayed twice.

Expected behavior (if reporting a bug)

  • During the second upload only a single throbber should be shown.

This is actually worse than it looks because the second image is actually uploaded twice, doubling the amount of time it takes to upload on a slow connection.


PR by @quicksketch: backdrop/backdrop#2252

@quicksketch quicksketch added this to the 1.10.2 milestone Jul 23, 2018
@quicksketch quicksketch changed the title Add a .once() guard on File module's AJAX uploads Add a .once() guard on File module's AJAX uploads to prevent double-uploading Jul 23, 2018
@alexfinnarn
Copy link

I can confirm that the attached PR fixes the double throbber issue; however, the tests are failing. I can't see which tests are failing.

@klonos
Copy link
Member

klonos commented Jul 28, 2018

I also confirm that the issue with the double throbber is fixed in the PR sandbox. Thanks for filing this ticket and for providing a fix @quicksketch 👍...this has annoyed me oh so many times (my OCD again 😜).

The failing tests are pointing to a missing /core/misc/zen-ci/init_test.sh file ...which if not mistaken has something to do with the recent work @Gormartsen and @serundeputy have been doing in order to allow zenCI to run phpcs (#3213). ...I think.

@serundeputy
Copy link
Member

@klonos nah; nothing from #3213 is merged yet. It was just a hiccup with the test suite/runners ... I restarted and it is happier now: https://zen.ci/backdrop/backdrop/test/test-php53-backdrop_backdrop_2252-c9c7d06

@klonos
Copy link
Member

klonos commented Jul 28, 2018

Thanks @serundeputy 👍 ...the php70 test has gone AWOL though.

@serundeputy
Copy link
Member

@klonos i turned it on and off again and everyone is happy :) backdrop/backdrop#2252

@klonos
Copy link
Member

klonos commented Jul 28, 2018

@serundeputy don't be fooled ...did you actually expand the "All checks have passed" fieldset and then check the "details" links for php53 and php70? ...as I said, AWOL:

screen shot 2018-07-29 at 9 08 33 am

...this bug with zen CI has been happening for a while when you close and reopen a PR:

screen shot 2018-07-29 at 9 09 22 am

@serundeputy
Copy link
Member

this is an expanded one: https://zen.ci/backdrop/backdrop/deploy/deploy-backdrop_backdrop_2252-c9c7d06-2 but i think we are getting off track on this issue; if there are issues with testing you wish to sort out file them and scope them.

@quicksketch
Copy link
Member Author

Super, I've merged backdrop/backdrop#2252 into 1.x and 1.10.x. Thank @alexfinnarn and @klonos for reviewing!

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

No branches or pull requests

4 participants