This was the bounty: http://drupal.org/node/138108.
The new version of imagefield has been completed per the clients wishes, and we now wish to contribute it back to the community.
Coding was done by me (Bojan Zivanovic) and development sponsored by Brakkar from Kaeria SARL.
As a base, the last 5.2 HEAD version was used. This provided us with basic ajax upload, although without support for uploading more than one file at a time.

What was done:

1) Parallel ajax upload
You can now upload via ajax more than one file at a time. The key thing was to change upload.js so it creates a iframe for every uplaod, instead of trying to use one iframe for each upload.
As you will see from the module, it requires patching upload.js, which I realise is not the most optimal solution.
What are your thoughts on this?
One of the solutions would be that my changes be included in the next drupal version. The other is to use our own upload.js file instead of the drupal one.

2) Ajax delete
3) Thumbnail support
As a basis we used a patch posted to this list. However, it didn't fit our needs, so code was moved, deleted, added, and only a small part of that patch remains. The thumbnail is created in the temp dir as soon as a file is uploaded.
This allows us to show the thumb in the edit page instead of the full image, which decreases load times (an uploaded 3mb picture takes quite a while to load in the edit screen). When submiting the node, the thumb is moved to the proper location.

I am looking forward to all suggestions and criticism.
There are possibly some smaller bugs left, but right now we have the module working fine.

I am providing the complete module now, since I think it would be easier for testing. I can provide patches as needed. the .module, .install and .css files have been changed, and the js files added. Please consult the short README.
Since Drupal doesn't let me upload zip or rar files, and I'm on my Windows box right now, I am providing a direct download link from my server:
http://www.party-studio.com/bojan/imagefield.zip

CommentFileSizeAuthor
#5 imagefield_install.patch573 bytesbojanz

Comments

Robardi56’s picture

Robardi56 commented

Nice work Bojan, I enjoyed working with you on this as a sponsor.
I hope they will use your work as the 5.x-2.0 of imagefield, with a -dev version so anyone can test it, and report issues.

Maybe they could even let you commit on this ?

jpetso’s picture

jpetso commented

If you aim for your work being included in the upstream version, you should probably post those patches proactively. I for one am _very_ interested in the AJAX deletion stuff, but there's obviously more interesting code to be found in your improved version. Please provide proper patches, I would hate to see the good work getting ignored because of problems with a large code drop.

bojanz’s picture

bojanz commented

Will do.
Wanted to provide the whole thing first so that people can easily install it and see what it's about, without applying patches.
I will make the patches and send them first thing tomorrow.

Robardi56’s picture

Robardi56 commented

I'm not sure it can be patched or can it ?

This work was against the version 2 cvs version of this module.... I think a new entry must be created.

But i'm unfamiliar with the procedure so maybe someone with the knowledge can explain.

bojanz’s picture

bojanz commented
StatusFileSize
new imagefield_install.patch573 bytes

Thanks to brakkar's carefull testing the new code has gone through several changes, with many smaller bugs squashed. I think we have a stable module on our hands now.
Changes:
1) Integrated jpetso's patch from the bug list (http://drupal.org/node/88136)
2) Fixed bugs regarding uploading images with the same name (and showing their thubs).

I have updated the module archive at http://www.party-studio.com/bojan/imagefield.zip
I am also providing an archive with patches to imagefield.module, imagefield.css, imagefield.install and upload.js (from misc/), as well as the new file, imagefield_delete.js at http://www.party-studio.com/bojan/patches.zip

Tell me what you think,
Bojan

Robardi56’s picture

Robardi56 commented
Status: Active » Needs review

Great work Bojan, people will love this patch.
(i'm changing the status of the thread)

bojanz’s picture

bojanz commented

Refreshed both archives, adding some defensive code to prevent issues when thumbnail_resolution is not set (after module upgrade to new version, if the field settings haven't been resaved).

dopry’s picture

dopry commented

Can you please post a proper diff? I will not be reviewing this code without it. I can't easily ascertain what has changed and what hasn't.

bojanz’s picture

bojanz commented

As I said in one of my previous posts, http://www.party-studio.com/bojan/patches.zip is the archive with the diffs for all changed files.

bojanz’s picture

bojanz commented

dopry, did you manage to find it?
Just updated the module zip and the patches zip with the latest version.
As of now, we can find no more bugs, we have tested even the least possible variations, files of the same name, same name different case, etc...

jpetso’s picture

jpetso commented
Status: Needs review » Needs work

What dopry means is that you should split each feature or bugfix into a seperate, independent patch, so that it's not one big change but one task at a time. It's pretty hard to test and review a patch that does a multitude of things at the same time.

As for me, the multi-delete-button didn't work here (or I just didn't get how it's supposed to work), and on first installation you get two errors about the thumbname and thumbpath columns not existing in {files}. Which is because you just added it in the update routine, but not in imagefield_install() itself. Also, I find it pretty intrusive to add such columns to the {files} table - after all, this table supposed to be used by all files, even (and mostly) for those which don't have associated thumbnails. I would say {files} is not the proper place for this, better add it as additional CCK fields if you really need those entries.

bojanz’s picture

bojanz commented

jpetso,
Thank you for taking the time to review this.
I now understand what dopry wanted to say and will provide sepparate patches.

What do you think should be the most optimal solution for thumbnails?
I added the two columns to files because of what dopry said in http://drupal.org/node/127729:
"1) thumb_path needs to be added to the database columns, eliminating it from hook_field op=load. We need to save the path in the database at the time of thumbnail creation/image upload. "

Maybe create a thumbnail table? Would slow the module down a bit, cause it would need a INNER JOIN with the files table.
As far as I am concerned, the two columns are unnedded, thumbs are saved as filename_thumb.fileExtension in the filepath (although I could create a thumbpath in settings), so I could determine it's location in code without the columns, but I was following what dopry said..
Give me your thoughts on this.

Also, can you provide a more detailed report on what is wrong with the delete button? It is working correctly for me and brakkar, so let's see what the problem is.

The support for parallel uploading files is all in one patch, in the changes to upload.js, see upload_js.patch in patches.zip

That is the area where your comments are needed most, since I am not sure this is the best solution (having to replace the upload.js in misc/). Better would be either to commit it to drupal head (don't no if it's possible due to my calling deleteAutoAttach in code which is imagefield specific) or use our own upload.js.

Robardi56’s picture

Robardi56 commented

Hi Jpesto,
about the delete feature: once you have uploaded some images and you want them deleted, you check the boxes of the images you want to delete, and press the "Delete selected images" large button. The images will be removed from the edit screen (you still have to submit the node tough).

Last update: Hard work was put in avoiding the duplicate image problem when a user submits an image with a name of an already existing image. Now both image and thumbs should get properly incremented in all kind of situations.

To all the people following this thread: we WILL get the work of Bojan committed. Bojan is hard working and quick to commit bug fixes.

be9’s picture

be9 commented

Hi, Bojan (привет из России!), thank you for your work.

I had a problem with delete button not deleting, just as jpetso had. The problem was in line 594:

drupal_add_js('modules/imagefield/imagefield_delete.js');

I had put imagefield module to the sites/all/modules/, so javascript file was not found. Solution was simple:

drupal_add_js(drupal_get_path('module', 'imagefield') . '/imagefield_delete.js');

be9’s picture

be9 commented

Small issue.

theme_imagefield_image() function generates <a ... target="_blank" ...>, which is invalid XHTML.

bojanz’s picture

bojanz commented

Hi Oleg (поздрав из Србије!).
Thank you for taking the time to review my code. I appreciate it

I have just updated http://www.party-studio.com/bojan/imagefield.zip with the new version of my code.
Changes:
1) Implemented code change that Oleg suggested
2) Removed the non-standards compilant _target="blank", it was useless anyway
3) Changed the code so that thumbpath is stored in the content tables, instead of the files table.
Much more elegant. Thanks to jpetso for suggesting.

I have removed the patches.zip archive, the patches are too big to be considered for inclusion right now.
In the next couple of days I will start posting patches to the list, starting with bug fixes, up to the functionality.

Please note that if you used my code before you will have to manually drop the thumbpath and thumbname columns from the files table, and run update 3 from update.php.

Bojan

gordon’s picture

gordon commented

Just to comment on the size of the patches, this is because the end of lines on the editored files have been changed from UNIX format to DOS format (\n verse \r\n)

When doing the diff use the --strip-trailing-cr option to make sure it is the same and the patch will be much smaller.

bobdalob’s picture

bobdalob commented

I'd just like to add an observation on installing this version (1.30.2.6.2.16). I tried it out for obvious reasons (much needed feature!) but came across a 'knock-on' effect in thickbox, which did not previously exist in otherwise the same configuration.

Thickbox display fails* for images with periods or underscores in the filename (perhaps other characters too) but will work for other filenames:
imagecache 5.x-1.3
thickbox 5.x-1.0
imagefield 5.x-1.1 (also tried dev version 1.30.2.17)

* When I say fails, I mean thickbox just sticks in 'loading' state when selecting or switching to an offending image. It loads 'good' images in the same node without any trouble.

dopry’s picture

dopry commented

@bojanz: Can you please post the just the diff's to the issue? Not a link to an offsite zip. It is the defacto way of submitting patches for review. Thanks.

bojanz’s picture

bojanz commented

All the changes have been broken down into individual patches.
JavaScript Delete
Thumbnail Support
Image resized message shown more than once

And one patch to Drupal Core JS:
JavaScript Delete

Aside from those, three more patches to imagefield have been contributed and commited (or the issue fixed):
Storing files in session
Ajax upload not keeping alt and title
Undocumented dependence on upload module

bojanz’s picture

bojanz commented

The title of the patch for Drupal Core JS is Parallel uploads and Opera support, not Javascript delete.
Also, there was that patch for images getting duplicate filenames, I can't locate it now, must run.
It's here on the list.
That's all.
Regards,
Bojan

jpetso’s picture

jpetso commented
Status: Needs work » Closed (fixed)

So if all patches have been split out, I guess we can close this issue in favor of the new ones. Good work, bojanz :D

Robardi56’s picture

Robardi56 commented
Status: Closed (fixed) » Fixed

To make the thread visible again i'm just changing the status so people coming to the list of issues can better understand how the original patch got split.

bobdalob’s picture

bobdalob commented

Excuse me if I am posting this issue in the wrong place - but since I found this version here and nowhere else, I am reporting here.

I reviewed the error I was getting (see #18) and thickbox has nothing to do with it - the offending image file is stored without file extension and thickbox merely causes an error when trying to display as would any view of the image file.

So it must be in the way the uploaded image file is renamed - as far as I can see - the renaming function searches for the period in a filename as the separator and strips there. Of course people name image files in all sorts of ways and so sometimes someone will upload using an image called, for example, "summerholiday.2007.jpg" and right now that translates to "summerholiday_1182790065.2007" NOT "Summerholiday_1182790065.2007.jpg". I don't know which is the best way to remedy this - perhaps strip out periods?

On a separate note, the thumbnail previews fine on upload however in node preview the full image does not show along the body text (it tries to but the image is "missing").

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
bojanz’s picture

bojanz commented

bobdalob, thank you for spoting this.
I am updating my patch at http://drupal.org/node/143686
A manual solution:
Add this function:

function _imagefield_rebuild_filename($filename_parts, $cnt) {
  $filename = ucfirst(strtolower($filename_parts[0])) . '_' . $cnt;
  
  foreach($filename_parts as $id => $part) {
    if($id == 0) continue; // Ignore the first part, we already included it
    $filename .= '.' . strtolower($part);  
  }
  
  return $filename;
}

And replace this:

$filename = ucfirst(strtolower($filename_parts[0])) . '_' . $cnt . '.' . strtolower($filename_parts[1]);

With this

$filename = _imagefield_rebuild_filename($filename_parts, $cnt);

in _imagefield_widget_prepare_form_values.

Thanks,
Bojan

sotzing’s picture

sotzing commented
Category: task » bug

I'm running CCK version 5.x-1.6-1, the latest imagefield, my /tmp and files directory are chmod'ed to 777, my download method is public, and i've copied the .js files to my Drupal 5.2 install's misc directory.

When I add an imagefield field to a piece of content however, and attempt to upload an image, the status bar hangs at the 'uploading file' animation. If I submit the content while the animation is still loading, the image files are correctly uploaded to the server (though they are never acknowledged as completing within the new node page - and i am unable to upload more than 1 image due to this hang [and imagefield is set for multiple uploads])

If anyone can offer me any advice I'd really appreciate.
I love this module and really wanna make it work!

Thanks,
Andrew

sotzing’s picture

sotzing commented

for some reason when i have the 'devel' module installed it breaks the uploader. I will make do without it (since yours module is so much more essential to me) :)

Please disregard my previous post - unless someone wants to figure out why devel makes imagefield break

Thanks again,
Andrew

jpetso’s picture

jpetso commented
Category: bug » task

Sure, that's because Devel appends some piece of HTML to any output that Drupal prints, including the JavaScript callbacks that are responsible for uploading the file. Setting this issue back to "task", as this was the original category.

sherk_wing20’s picture

sherk_wing20 commented

hello bojans!!! thanks for your work... how i wish i could have a copy of your module but it seems your site is offline.. hope i could have one..
thanks!

bobdalob’s picture

bobdalob commented

You'll find that this has evolved into ImageField 2.0. It doesn't have ajax delete, yet, but it has options for maximum upload size, dimensions, number of images.

RC6, at the time of writing, is available for download from here (5.x-2.0-rc6), and is working very well in my opinion.

sherk_wing20’s picture

sherk_wing20 commented

Thanks for your reply bobdalob!!! I'll be checking that module.. thanks.

timefor’s picture

timefor commented
Status: Closed (fixed) » Active

The links are down for this mod. Could you post an updated link?

Thank You

- Jayson

jpetso’s picture

jpetso commented
Status: Active » Closed (fixed)

This mod is a fork and as such unsupported by imagefield upstream. Especially considering the progress that was made in imagefield since this mod was published, It wouldn't be a good idea to use it as we can't assume that it will be updated.

bojanz was kind enough to break all his changes into bite-size patches, see comment #20 for the complete list. The correct procedure is therefore not to have this .zip file updated, but to take a current imagefield and apply the desired patches from the linked issues to that. In case you want to contribute, you can also test, review or improve the ones that haven't been applied yet.

In any case, using a .zip file that is not endorsed or supported by upstream will get you in trouble except if you're a developer yourself (or pay one) and work on getting the changes back into the official version. Just don't do that, and keep the issue closed, please.