Drupal 6 port

Thanks, highly appreciated. Here's my counterpatch :]

I committed field_file.inc as well as the .info and .install files to CVS HEAD, but even in the current version (slightly better) I can't get myself to commit the .module file. Thus, find it attached to this issue still.

Status update:
- Resolved (hopefully all) conflicts that derived from my spectacular DRUPAL-5--2 vs. HEAD syncing sk1llz
- Made the formatter work in a CCK-for-Drupal-6 way.
- Reworked the widget so that it also suits the new CCK, copy'n'pasting from imagefield, example_field.php and my own duration module.

Your patch didn't work for me, so I just proceeded to change stuff where I found it reasonable, and after porting to the new widget/element separation (including quite a few other modifications, I guess) uploading seems to work. As well as listing and deleting files, and showing them on the node after the List checkbox is checked. Node view is back to original beauty, whereas the widget has lost all its theming and is just plain ugly right now.

No idea what went wrong before, but realistically viewed it couldn't have possibly worked in that state anyways. There's still an awful lot of deprecated code in there, and for everyone who wants to have a look without being a developer - don't even try it at this point. Crell, with just a slight bit of luck you should be able to get your tech demo roughly working in time.

/me hugs crell and jpetso....

@jpetso: if you haven't already feel free to bring head up to the latest patch.
.darrel.

I committed Crell's latest patch to HEAD, with a few more improvements. Now features dopry's latest field_file.inc tweaks, filtering out empty items in hook_field($op='load') (yeah, CCK still doesn't do that) and 'list' being enabled by default.

Remaining challenges:
- submits the node on pressing "Upload", instead of just updating the edit form
- proper theming
- AJAX uploads
- getting rid of all the legacy code in there

But yeah, one could theoretically use it already. Keep the patches coming, everyone!

Unassigning from Crell, this is now a free-for-all (including myself). Everyone is invited to check out HEAD and fix stuff by posting further patches in here.

STUPID question... how do I get a copy of the HEAD with the latest drupal 6 port over? sorry.. sorta drupal noob...

@Crell:
Nope, not figured out yet.

@dnorthcott:
The current state is pretty much unusable for everyone but developers. If you consider yourself a "noob" then you should probably wait until we get to a state where it doesn't eat your children and relicenses all your proprietary applications under the GPL. (Actually, that might be a slight exaggeration... anyways, you get the point.) If you want to help, finding out how to get the HEAD version is a necessary precondition and your first task :P

Status update:
The "submits node on pressing Upload" bug is fixed... it seems the cause was that Forms API doesn't consider buttons that are only expanded in a process callback. So after a good amount of refactoring (bye bye, filefield_combo element) the "Upload" button actually uploads stuff without saving the node. Yay.

@jpetso : "filtering out empty items in hook_field($op='load') (yeah, CCK still doesn't do that)"

Not sure my remarks is relevant : actually it does, but on node_save. content.module calls hook_content_is_empty() on the field module so that it can determine whether some of the user-entered values should be considered empty, and filters them properly (i.e : saving *one* NULL value). This ensures empty values are stored consistently across field modules.

Since this only applies on node_save, legacy data from a D5 install might still suffer from the fact that until D5, content.module saved whatever values field modules handed back to it.

Status update:
I replaced CCK's default multiple value widget with a custom one, because CCK's version cannot possibly handle stuff like "if there is one file in a single-value field, display that one plus an upload widget that's going to replace this file if the user uploads a new one". CCK's "Add new item" functionality is now also unnecessary, whereas the nice drag-n-drop table is not ported yet and would certainly be nice to have.

Hm... filefield is nearly usable this way. In case we get the theming sorted out, the casual user should not notice all too many drawbacks compared to 5.x-2.0.

Remaining challenges:
- proper theming
- AJAX uploads
- getting rid of all the legacy code in there

@yched:
Yes, I got that. However, single-value fields are still stored as lots of NULLs in the database. That's ok, but CCK does nothing to filter out these NULL items before the field gets to see them. So yeah, storing is ok, but loading remains my responsibility (even if it shouldn't be, imho).

OK, I see what you mean. I'm not exactly sure right now what would be the possible cons of hiding empty values from field modules altogether, but it would be worth discussing as a feature request.

Additionally, it seems the current code in filefield_field('insert'/'update') could be moved to filefield_content_is_empty() (and the "// compact deltas" part is now not needed)

Sorry to hear you need to make your own handling of multiple widgets / d-n-d reordering. Filefields / imagefields having to deal with an upload element, they are their own kind of special animals... I guess having the form designed as [value 1, upload element for value 1] , [value 2, upload element for value 2] is not possible ?

I'm not exactly sure right now what would be the possible cons of hiding empty values from field modules altogether, but it would be worth discussing as a feature request.

For field modules there are no further implication of hiding these kinds of empty fields, because they need to deal with that situation anyways - the respective item is still missing for nodes that don't yet have a database row written. For CCK, I guess the only questions would be how to decide if an item is "database-empty" (all values NULL? content_is_empty() == TRUE?) and performance implications, I would guess the latter is negligible (no hard facts, though).

Additionally, it seems the current code in filefield_field('insert'/'update') could be moved to filefield_content_is_empty() (and the "// compact deltas" part is now not needed)

Good catch, I'll try that out next time. Sounds good to me.

I guess having the form designed as [value 1, upload element for value 1] , [value 2, upload element for value 2] is not possible ?

Well it might be a possibility, assuming that it can be hidden by some friendly JavaScript and replace the edit part of the widget when a "Replace" (or rather, "Delete") button is pressed. I still need to think more about how the 6.x user interface is going to look like, also considering that at some point we'll probably integrate "foreign" widgets for image/audio/video files in the same UI.

Actually, there is still a more or less large problem facing the built-in multiple value handler: how to make deleted files disappear (including zebra table rows) while still keeping the delta (with values) until the node is submitted. On the other hand, if I scrap the "Deleted" state as such and instead present a standard upload widget (aka "Replace") for all deltas where no file exists (yet, or anymore) then that might work as well. Those deltas would only be cleaned up at submit time, and until that happens, hang around in the multiple value table.

Crell, dopry, any further input from your side?

Is the plan for filefield and imagefield to continue to co-exist, or is this one eventually going to sort of take over? I'm in a need of an imagefield like solution for a current project but am unsure where I should be focusing my efforts. On filefield, or imagefield at this point.

For example, should the changes that have been made above so that filefield works with multiple images be ported over to imagefield as well?

Good to know. Would love to see any sort of battle plan that you guys come up with and where I might be able to help.

The short-term goal is to share back-end code (in field_file.inc), dopry suggested that this file might be put into CCK when it's sufficiently worked out. If you need an imagefield like solution, you might be better off focusing on imagefield at this point.

The long-term goal, as far as I understand (and work towards), is to deprecate imagefield by making filefield extensible enough so that it can display and edit images in a similar fashion to imagefield. Lots of details have to be worked out in order to get to this point, but maybe we can happen before the Drupal 7 port is on the plate.

@Crell:
Hidden form values (or actually, '#value' elements) is what I currently use for remembering deleted files. I'm not sure it'll work sufficiently in a table though, as that will probably break the zebra coloring of the table rows. (CCK doesn't know if something visible is in there or not... right?)

Status update:
I went back to CCK's multiple value handling by implementing the approach detailed above. So, yay for automatic reordering of the files, less code complexity and an easier road to AJAX support (we now have a "Delete" button instead of a "Delete" checkbox). However, the "Add another item" button seems to produce weird results (empties the fields...) and curiously the "Upload" and "Delete" buttons account for additional items in the multiple value table. I would assume that bug lies somewhere in CCK this time.

Remaining challenges (same as twice before):
- proper theming
- AJAX uploads
- getting rid of all the legacy code in there

But now for real. Let's have a go at theming... haaaa!

Status update:
I had a go at the stylesheets, and much of the legacy code is also gone - not all of it though, because some code includes stuff that needs to be ported still.

Remaining challenges:
- Bug hunting: why is CCK's multiple value handler being so mean to me?
- AJAX uploads
- Port Views integration to D6
- Clean up the @todo items, and make sure there are no regressions

It's slowly getting pretty sweet. Nearly there, right?

Status update:
Hardly work on filefield itself today (apart from making the "Always list files" option work again), but instead debugging and bugfix patches for CCK itself, or rather for its multiple-value handling. Boy, that code is waaay buggy and untested.

CCK issues:
- #273476: 3F #1: Fix item count for "Add more"
- #273481: 3F #2: "Add more" does not submit without JS
- #273539: 3F #3: Value callbacks not executed on JS "add more"

I don't yet have a patch for the last one, but if those are fixed then CCK's multiple value handling will probably be usable for our purposes.

Remaining challenges:
- Fix the last one of the issues above
- AJAX uploads and deletions
- Port Views integration to D6
- Clean up the @todo items, and make sure there are no regressions

Status update:
Views integration is ported in a vastly improved way (at last I've been able to grok relationships, and whoo they're funky), more CSS refinement.

Next up: AJAX uploads and deletions.

Status update:
No AJAX uploads and deletions yet. Instead, I've been battling all day with the above-mentioned issue (#273539: 3F #3: Value callbacks not executed on JS "add more"), with limited success. Still needs more work and insight - if you feel comfortable with AHAH, I need you to help out. (In Soviet Russia, bug fixes You!!) Apart from that, a few minor fixes in filefield; most notably uploading a file only when the "Upload" button is pressed, instead of any arbitrary button causing the upload.

Remaining challenges:
- Fix the issue above, for good
- AJAX uploads and deletions
- Clean up the @todo items, and make sure there are no regressions

I might just take a break from that CCK bug, it's really exhausting. On the other hand, with the knowledge about AHAH callbacks that I learned the hard way there, getting filefield-internal AJAX to work should be a breeze. Or nearly so.

Status update:
Implemented AHAH uploads and deletions, and learned a whole lot more about AHAH. Maybe I'm now able to fix the CCK bug a bit better. I feel like filefield_js() is a masterpiece of proper form handling in a JavaScript callback... but let's come back to that when the bugs below are fixed.

Bugs? Well:
- Uploads work in Firefox but not in Konqueror, that is a regression compared to the D5 version. An ahah.js bug? (Drupal isn't even invoked.)

- The AHAH submit-replacement behaviour won't attach to the new form, it seems. I experienced the same thing with the CCK "add more" button, need to have a look what goes wrong there.

Remaining challenges:
- Still waiting to be fixed: issue #273539: 3F #3: Value callbacks not executed on JS "add more".
- Port filefield.js (only including a JavaScript extension validator) from $(document).ready() to AHAH behaviours.
- Clean up the @todo items, and make sure there are no regressions.

You know what, this stuff is pretty usable now, apart from a few bugs. I feel like rolling an alpha release for adventurous testers and people in dire need of a half-working filefield release. [Update: here it is.]

Status update:
- Edit widgets (the thing with the icon and the description textfield, but not "List" and "Delete") are now always encapsulated form elements.
- The CSS has been tuned with no end, most notably causing the icon to appear in the vertical center of the table row. Friggin' cumbersome compared to desktop widget toolkits? Hell yeah.
- Major new feature: Let users upload even if they are not user 1! (That is, I fixed the permission name in hook_menu().)
- File extension restrictions work again, both server- and client-based. Of course with the client-based JavaScript validator better than ever - passive error message instead of annoying alert() dialog.

Remaining challenges:
- JS bug, Firefox: Have a node with pre-populated file. Delete file (using JS). Upload file, fails. Upload file again, works.
- Still waiting to be fixed: issue #273539: 3F #3: Value callbacks not executed on JS "add more".
- Revive hook_filefield().
- Clean up the @todo items, and make sure there are no regressions.

Status update:
Implemented better permission checking for private file downloads. The even better news is that I've been able to get completely rid of the annoying 'view filefield uploads' permission by replacing it with the more fine-grained field-level permissions from content_permissions which ships with CCK 6.x. (Everyone else falls back to 'access content', and node access for private file downloads.) So the permissions are now context sensitive, which means they're not built into theme_filefield() anymore too but checked at the field level.

Plus completed the last item marked as @todo: writing prosa for watchdog entries. Yay.

Remaining challenges:
- All the broken JS stuff, you know the drill.
- Revive hook_filefield().
- Make sure there are no regressions. Like, you know, ...revision handling. dum dumdidum.

Apart from hook_filefield() and the JavaScript bugs, we're probably on feature parity (hah! much further, I'd say) with the D5 version. So let's roll another alpha release, and when hook_filefield() is back in there, I'll be going for beta. [Update: here's 6.x-1.0-alpha2, for your personal pleasure.]

Right, that was the idea. The 'load' and 'delete' ops (plus the new 'reference' op) of hook_file() are now implemented by field_file.inc, no need to implement those anymore.

The 'save' op needs to be reinstated as file_save_upload() now actually writes to the {files} table but doesn't invoke other modules. The 'validate' op is also needed, although it now only needs to get an array of validation callbacks that is passed to file_save_upload(). The 'prepare' op (extend file properties directly after uploading) is yet missing as well.

And finally, I want to make filefield genuinely extensible for other modules to provide their own widgets and formatters, so that imagefield, audiofield and videofield can be deprecated in favor of a unified solution. The latest D5 version had the 'form' op for altering the file edit form (not needed anymore, I've implemented a cleaner solution for that now) and hook_filefield_widget_settings() for extending the settings form. I'd also like to have an indirection for formatters, so that the most appropriate formatters are chosen for each file separately.

So much for the plan on hook_filefield() - it's more of an umbrella term for me now, as some of those do serve separate purposes and should definitely not go into one same hook. (I don't like kind of "stick everything into a hook" anyways.)

Status update:
On my way to fixing the JavaScript issues.
- As part of #274791: Upload field not required even if indicated as such using CCK (yeah, accomplished too), I fixed issues with '#required' not working after AHAH replacements. (Solution: use content_field_form() in order to get '#required' and other CCK defaults merged into the form element.)
- Also fixed the "AHAH behaviours are not attached after replacing the form element" bug, where the main issue was that AHAH didn't know the new button as it hadn't been included in the Drupal.settings object which had been populated in the HTML head tag. (Solution: get the new JavaScript settings and merge them into Drupal.settings using another script tag.)

Filefield itself should be free of AHAH bugs now, I believe. If one doesn't take the Konqueror upload inability into account, I think that's not caused by filefield but rather AHAH or some other code.

In addition to the above, I came up with a revised patch for the easier CCK issues that I had already fixed above (but with a better fix now). And finally, I learned StGIT today. Amazing tool for patch management, you should have a look at it too. If you're managing patches for Drupal core or CCK, this is what you want to use.

Next up: fixing the "add more" bug in CCK. I now feel confident enough to solve that one for good. Fear the petso, evil beastie!

Status update:
Splendid, all known bugs are fixed. A few subtleties had remained in filefield itself, and I fixed them in tandem with the Great CCK Bug. Also came up with a port of the AHAH behavior attachment issue for CCK as well, patch posted to #275192: 3F #5: Attach AHAH behaviors to newly inserted widgets.

yched applied #274038: 3F #4: "Add more" for only one field, so the worst issue affecting a lot more modules than just filefield is now solved. Hopefully yched also likes my other patches (they're really nice now) - with those applied, filefield now works correctly with and without JavaScript. I almost feel like an AHAH / form builder guru now. Well, almost :P

That means it's time for another release. [Update: link to alpha3.] Still no beta, as hook_filefield() is still not ported. That's the next item on my list, probably I can get to it tomorrow.

Remaining challenges (we all love this part):
- Revive hook_filefield().
- Make sure there are no regressions. Like, you know, ...revision handling. dum dumdidum.

Oh, and before I forget it once more:

Additional remaining challenge:
- Integrate the "Description" and "List" properties with Views.

I'm not sure if this is the correct place to post this, but after installing the alpha3 (no previous version was installed) version I'm recommended to 'upgrade' to the alpha2 version.

ahallez: I'm pretty sure that's a delay in recognizing releases on the drupal.org side. I'm pretty convinced if you try tomorrow, at latest one day after comment #30, it will probably work as expected. (alpha3 is already marked as recommended release on the project page.)

Not sure why, but with current filefield HEAD, 'edit' widgets for multiple-valued filefileds don't play well with tableheader.js :
the icon, textfield and 'delete' button stay above the floating table header - see attached screenshot.

Everything back to normal if I comment out this part of filefield.css :

.filefield-file-edit {
  position: relative;
}

(but then the icon goes away)

@yched:
Argh. And I spent so much time on the CSS. I am seriously considering to drop that icon for good. Really, who needs this in the *edit* form? A survey, please.

In other news, status update:
Revived hook_filefield(), or rather what was left of it. I'm still a bit uncomfortable to code around in field_file.inc without dopry's advice, but he wasn't there when I needed to push this further so I did it on my own. That means field_file.inc in imagefield and filefield are a bit out of sync... but on the other hand, the API is still the same and just has the one or the other function/hook more.

So the outcome is that filefield is more and more easily extensible than ever, with validators, widgets and formatters able to be chosen on a per-file basis. The method for determining which widgets and validators are actually chosen needs something better than hardcoding the generic widget at the very bottom of the list and randomly deciding between the other available ones. Of course, that will hardly hurt in practice as images, audio files and videos are pretty much mutually exclusive.

In other news, this. [and beta1]
Imagefield is so dead.

Ok, I dropped the "position: relative" CSS - which was necessary for vertically centering the icon, or at least I found no other way, not being a CSS guru. So that means we're back to the former (simpler) state, and hey, I think I might like it that way too.

Coder issues and PHP E_NOTICE warnings have been fixed.

Next up:
- Find out if revisions work properly, and fix those if they don't, which is probable.
- Pursuade yched to commit #273539: 3F #3: Value callbacks not executed on JS "add more". Any thoughts on my further points that I brought up there?
- Make it possible for other modules to hook into the widget/field settings.
- Make it possible to resort and enable/disable file formatters and widgets.
- Plus, get back to fix 5.x-2.x issues one last time.

I haven't got a whole lot of time left for filefield, so if you help out especially on those last two features, you can go to sleep with the nice feeling of having deprecated like three or four field modules in one blow. Advance Drupal, code for filefield!

Man, I should really take a break, probably.

sub

It's your lucky weekend, because I didn't drive home as planned. Which means you get some more filefield goodness before I work on different stuff (starting from tomorrow).

Status update:
- Ported more revision handling from imagefield (hook_file() with $op='references'), and fixed it so that it actually works.
- Seems my arguments were compelling enough to have yched commit my patch at #273539: 3F #3: Value callbacks not executed on JS "add more", with minor modifications. Thanks again, dude! That means that CCK HEAD now officially works bugfree for our purposes, please update and enjoy.
- Made it possible for other modules to hook into the widget/field settings.
- Made it possible to resort and enable/disable file formatters and widgets. In the same stroke, file (especially mimetype) restrictions based on the set of enabled file widgets.
- As testing ground (not anymore) and proof that the above works: Imagecache support like in imagefield, as a separate module in the filefield_image project (but usable independently from it).
- A bit of code cleanup, including a split into three .inc files in addition to the .module file (filefield.(theme|widget|formatter).inc).

Remaining challenges:
- Get up early tomorrow and work on something completely different.
- Get back to fix 5.x-2.x issues one last time. (Later, obviously, but still pending for the near future.)
- Port remaining imagefield features to filefield_image (most importantly, image size restrictions, then, default files - not default images :P) and see if instance-level add-on properties (-> custom image title) can be made possible in some way.
- Related to image size restrictions, make it possible to hook further upload restriction notes into the upload widget so that they're displayed to the user. (easy)
- Implement hook_file_download() support for newly uploaded files that are not yet stored in the node but are only hanging around in the form state. This not being implemented is the reason that image previews are only working after the node is initially saved.
- Port filefield_meta, in order to make sure hook_file() works as expected. (easy, assuming there are no hook_file() bugs)

That's it for now, buddies. I feel like what I did is pretty solid now, way cool, rock stable (one can hope, no?) and a splendid base to work from. I leave with a good conscience, and hopefully will be able to get back to some of these issues before going on vacation for a month on July 19th. Most importantly, I feel like the API might not change in incompatible ways anymore. Plus, with a bit of luck, I might be able to spend a minor bit of time on some of these issues. Still available to apply patches if they're nice and clean - keep them coming!

Update: the well-deserved filefield beta2, and the earth-shattering filefield_image beta1. Go catch them.

Wow, you've done a lot of work. However, I think you meant for this to be your second link: filefield_image beta1.

Oops. Thanks, corrected also in my above comment.

hurrah to jpetso for excellent productivity. :-)

I think it's time to close this issue. All of filefield's 5.x-2.x functionality has been ported over, and it's now merely a question of getting the additional features right. So the port is done, and other issues are already creeping up everywhere. Thanks for the fish, I hope you had a good time here :)