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

Ability to upload non-image files #17

Closed
wants to merge 10 commits into from
Closed

Conversation

syko
Copy link

@syko syko commented Oct 26, 2011

Hi,
The ability to upload non-image files was missing, which rendered the "embed flash" button completely broken.
I've fixed views.py so that it wouldn't assume that the file is an image and also changed the galleriffic filebrowser to display nice file icons for non-image files.

Other minor issues I fixed along the way:

  • Added i18n support
  • Fixed filename localisation issues
  • Fixed the gallery js to consider the image and viewport height when resizing images. (displaying very tall images was broken)

@mjtorn
Copy link

mjtorn commented Nov 23, 2011

Just tested a PDF upload - looks good! Would be really nice to have this pulled in!

@eamonnfaherty
Copy link

Would be really nice to have this pulled in!!

@eamonnfaherty
Copy link

IMHO: might be nice to add a regex checking the file to see if it really is an image - otherwise this could allow users to upload corrupted images :(

@mjtorn
Copy link

mjtorn commented Feb 4, 2012

On Sat, Feb 04, 2012 at 07:13:36AM -0800, eamonnfaherty wrote:

IMHO: might be nice to add a regex checking the file to see if it really is an image - otherwise this could allow users to upload corrupted images :(

Not sure a regex would cut it. But of course the file magic
could be checked. In any case, anything allowing arbitrary
uploads is IMO a good deal :)

mjt

@elidickinson
Copy link

PIL's Image class has a verify() function that checks if a given file is indeed a valid image.

@webjunkie
Copy link

What's up with this ticket? Is this change merged somewhere?

@ghost
Copy link

ghost commented May 27, 2013

we really need this one! merge, please!

@danbruegge
Copy link

2 years? Please merge! We need it too.

@riklaunim
Copy link
Contributor

The file handling changed a lot. I don't know if it will pass non-images, will have to check.

@riklaunim
Copy link
Contributor

This would have to be rebased and rather rewritten not to hack image uploads. There are some Django file upload applications that could be a better solution as they are ready and tested. Integrating those somehow with the editor could be a better option.

@pnovotnak
Copy link
Contributor

FYI, the maintainer was MIA for a long time but there are a number of maintainers now, so a new pull request won't languish like this one did.

What about dumping the built-in file upload capability and adopting something boxed? Was thinking django-filebrowser, but it appears it requires Grappelli, and that rubs me the wrong way. I don't want to turn this project into a dependency hell. Does this sound silly @riklaunim?

@riklaunim
Copy link
Contributor

Image upload seems quite specific to the ckeditor widget (and how it later can insert them). File upload seems less bundled with the WYSIWYG features so uploading in one widget and copy-pasting the URL shouldn't be a hassle. I think that we could check the most popular uploaders options for django and maybe provide some examples or code snippets etc. showing how to use them quickly with django-ckeditor.

@pnovotnak
Copy link
Contributor

@riklaunim - I think you just might be right for now. Either way, I'm going to close this PR since it's old and crusty. If someone wants to open another one that rejiggers things in a way we can all agree is a happy way, we can go from there.

I want to reiterate, thank you @syko for your pull request, if you'd like to submit another, here's my wishlist. If you meet these criteria, and your code isn't a mess, you'll likely get my vote.

  • I'd like to see if uploads can (in a sane manner) optionally be tied into a 3rd party app that provides nice access control and shit like that. We'll retain basic image upload functionality in the app for the foreseeable future, I think.
  • Nice integration with WYSIWYG is required.

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.

None yet

8 participants