Skip to content

[STUD-256] Seems like on chrome when cancel is hit in the file chooser, the 'change... - #129

Merged
chrisndodge merged 3 commits into
masterfrom
fix/cdodge/cancel-asset-upload-crash
Jun 11, 2013
Merged

[STUD-256] Seems like on chrome when cancel is hit in the file chooser, the 'change...#129
chrisndodge merged 3 commits into
masterfrom
fix/cdodge/cancel-asset-upload-crash

Conversation

@chrisndodge

Copy link
Copy Markdown
Contributor

...d' event is fired and startUpload() was being called. We need to check to see if the upload file is blank and - if so - just return immediately out of the function

…nged' event is fired and startUpload() was being called. We need to check to see if the upload file is blank and - if so - just return immediately out of the function
@chrisndodge

Copy link
Copy Markdown
Contributor Author

Re: testing. Per Jay's comment today that the interns are building out test harnessing around asset uploads, I'll see if I can build on that when it's done. Although the File Upload file picker is not a DOM element (it's a browser modal), so I'm not sure if it can simulate mouse clicking in those tests.

Comment thread cms/static/js/base.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You're better off using the actual files API: do something like $('.file-input').get(0).files and introspect what you get. It's a much nicer API than what you've hacked together here. :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@chrisndodge

Copy link
Copy Markdown
Contributor Author

@singingwolfboy updated API

Comment thread cms/static/js/base.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry to be pedantic, but files should be a local variable, not a global variable; which means it needs to be prefixed with var.

@cahrens

cahrens commented Jun 11, 2013

Copy link
Copy Markdown

I thought I put in a comment, but now I don't see it... Do we know WHY this behavior is happening on Chrome? I like your changes, but I wonder if our selector that is binding the event is incorrect.

@singingwolfboy

Copy link
Copy Markdown
Contributor

@cahrens: it's happening because the startUpload function is bound to the change event on the <input type="file"> object, which means it will be fired anytime the user does something to change it -- whether or not a file is selected. This is the correct fix -- we just need to be sure that the startUpload function is smart enough to handle the case where a file is not selected.

@chrisndodge Looks good to me!

@cahrens

cahrens commented Jun 11, 2013

Copy link
Copy Markdown

👍 But please do follow up on the test.

chrisndodge pushed a commit that referenced this pull request Jun 11, 2013
[STUD-256] Seems like on chrome when cancel is hit in the file chooser, the 'change...
@chrisndodge
chrisndodge merged commit e685ff7 into master Jun 11, 2013
@chrisndodge
chrisndodge deleted the fix/cdodge/cancel-asset-upload-crash branch August 8, 2013 18:37
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
bugfix: lcp score can apparently be 0/0
wowkalucky referenced this pull request in raccoongang/edx-platform Dec 1, 2017
* Update views.py

* Adding import statements

* Update test_views.py

* updating test cases

* Update test_views.py

* Update views.py

* removing white spaces
CrewS pushed a commit to CrewS/edx-platform-1 that referenced this pull request Dec 21, 2018
rediris pushed a commit to gymnasium/edx-platform that referenced this pull request Feb 25, 2021
…re/fix-matplotlib-numpy

push matplotlib to post requirements
ktyagiapphelix2u pushed a commit to ktyagiapphelix2u/edx-platform that referenced this pull request Jun 2, 2025
Danyal-Faheem pushed a commit to Danyal-Faheem/edx-platform that referenced this pull request Jul 15, 2025
Co-authored-by: AliSalman <alisalman@192.168.10.8>
kiram15 pushed a commit that referenced this pull request Feb 25, 2026
fix: added Redirection logic for enterprise users with the correct workflows
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

Successfully merging this pull request may close these issues.

3 participants