Images are sorted by name, if You change order (drag up or down) and then save node, images sorted by name again.
If disable SWFUpload widget, works as expected.

Comments

matulis’s picture

Title: Can't reorder images » Rearrange images does not work in Opera & Chrome

Works as expected in Firefox & IE.

patrickroma’s picture

Same here. Chrome and Opera doesn't work. Anybody knows where to start for a solution?

David Lesieur’s picture

Works as expected in FF, IE, and Safari. Fails in Chrome and Opera.

skilip’s picture

Any JavaScript errors in Chrome?

David Lesieur’s picture

No JavaScript errors reported by Chrome.
Unfortunately, I have not taken any time yet to investigate this bug.

radamiel’s picture

This bug appears because Chrome & Opera sort the hashtable with images.
For example,
var arr = new Array();
arr[250] = 'image1';
arr[0] = 'image2';
arr[46] = 'image3';

for(i in arr) {
alert(i);
}

in Firefox, IE and Safari we get 250, 0, 46
in Chrome & Opera - 0, 46, 250

CyrilD-1’s picture

Hi,

I have the same issue.

Is it fixed with 6.x-2.0-beta8 version?

Regards.

sansui’s picture

Drags fine in Chrome but doesn't save the order. I am noticing on some sites that all instances of tabledrag are affected, and some sites they are not.

sansui’s picture

Some good background info on what seems to be the case, supported by what stager is reporting: http://blog.bmn.name/2008/09/google-chrome-array-eval-out-of-order/

Vote for the chrome issue here: http://code.google.com/p/v8/issues/detail?id=164

This seems to be the primary bug report thread at google, although several others exist as well. I'm not 100% on this, but this seems like the issue at hand with swfupload and chrome - although perhaps Stager could offer some input on this since he reported the issue with Chrome's sort vs. other browsers

roderik’s picture

Version: 6.x-2.0-beta7 » 6.x-2.0-beta8
Status: Active » Needs review
StatusFileSize
new6.06 KB

Instead of waiting for people to stop arguing about whether or not Chrome should be changed, or programmers should change their JavaScript... I decided to just change the JavaScript.

This patch makes swfupload_add_js() pass data in an array, rather than an object.
(Note that the code which gets back the data hasn't changed. The FormAPI/CCK code which calls swfupload_widget_value() apparently doesn't mind whether it gets an object or an array, and what the keys in this object/array are.)

swfupload_widget.js is -obviously- adjusted to work with an array too.
...and in the process of rewriting ref.updateStack(), I discovered that it was buggy to begin with:
- If you add an extra file to a collection of already-existing files, all existing data in the hidden input field would be overwritten with (several times) the data from the file that was just added
- ref.upload_stack_size was just... wrong.

Fix attached. Please check my code flow/syntax for sanity; I'm far from a JS guru.

roderik’s picture

(oh - if your spacing in swfupload.module is a bit messed up and parts of the patch apply with offsets, that's because I have #1050506: prevent adding js setting twice (causing white screen on node preview) applied. Shouldn't influence anything else.)

sansui’s picture

Thanks Roderik - I'll give your patch a test this weekend and report back.

sansui’s picture

Patch applied cleanly after applying first patch you linked.

Seems to work ok in IE8 and Firefox from what I can tell.

In Chrome I uploaded a very small number of images - 3 - uploaded but did not keep sort order

I uploaded another 12 or 13 images, and I received these two warnings:

warning: array_merge() [function.array-merge]: Argument #1 is not an array in /home/mysite/public_html/sites/all/modules/swfupload/swfupload_widget.inc on line 60.
warning: array_merge() [function.array-merge]: Argument #1 is not an array in /home/mysite/public_html/sites/all/modules/swfupload/swfupload_widget.inc on line 60

At that point, I edited the node again, and uploaded one more image. After saving the node, the only image that remained was the new image I uploaded, and I received an array_merge warning for each of the images that disappeared.

The first upload seems to work fine, any subsequent save causes these errors

sansui’s picture

StatusFileSize
new12.59 KB

Also something else interesting I noticed in Chrome when trying to create a new node with images.

I uploaded 3 images, then clicked save - forgot to enter the title for the node. So I enter the title, go to save, and notice that my swfupload area has no thumbnails and repeats the first filename for each image instead of the actual files

ducktape’s picture

I applied the patch from #10 (and #11, for good measure).

This fixed the problem for me in Chrome (in FF all still works fine, as before). I have tested with adding and deleting images multiple times, but I could not reproduce the problems mentioned in #13. I also did not get any error messages.

I have tried to reproduce the error Sansui mentioned in #14. I can confirm the disappearing of the thumbnails, but the filenames were ok for me. Also entering a title and submitting the node form had the expected result. This behaviour is not limited to Chrome though, I could reproduce this in Firefox as well (haven't tried any other).

sansui’s picture

What version of PHP are you running ducktape? Not sure if it matters, but I'm on 5.2.6

I will try a base drupal install with just swfupload and see if I can reproduce the errors I'm getting on this site when I have a chance.

sansui’s picture

I think I fixed my issue. I had to change line 51 in swfupload_widget.inc to typecast the array

      $form_state['values'][$element['#field_name']][$key] = array_merge(
      field_file_load($file['fid']), // Load all fields, such as 'filepath'.

to

      $form_state['values'][$element['#field_name']][$key] = array_merge(
        (array) field_file_load($file['fid']), // Load all fields, such as 'filepath'.
sansui’s picture

I applied the patch line by line to a customized version of swfupload I have on another site (has custom rotation options in the js), as well as my stock install, and with the change mentioned in my last post, it also functions well in Chrome.

Tested both sites in IE8, Firefox, Chrome, so far no errors or issues with creating, reordering, and saving. The issue with no thumbnails after leaving out a required field doesn't seem to be a big issue, still saves fine.

SchwebDesign’s picture

subscribing, glad to see i'm not the only one noticing this. i will likely attempt the patch soon as well.

roderik’s picture

StatusFileSize
new21.8 KB

@Sansui: I will assume that the errors you saw, have already been fixed outside of swfupload... because field_file_load() always returns an array.

The patch from #10 has been extended:

1) it retains the thumbnails over previews;

2) it does not take lots of time/CPU anymore when a checkbox / edit link is clicked in a table row. ( Drupal.attachBehaviors() was called -apparently- unnecessarily.)

3) the 'edit' link has been changed from '<a href="#">edit</a>' to '<a id="..." class="active">edit</a>' (.active purely added to evoke a default CSS rule). This was necessary so that the page wouldn't start jumping around when clicking the link, after making the 2nd change.

4) #1242238: Display POST errors on widget initialization (I really didn't want to split that out.)

5) lots of commenting and some rearranging of code, in the process of getting to understand the whole thing.

designguru’s picture

I tried applying the patch in #20 with the aim of allowing users to save the order of imagefield uploads using the swfupload module in Chrome but the patch can't be applied for some reason; getting the following error msg via ssh at the sites/all/modules/swfupload directory:

git apply -v 875706-20.patch
Checking patch swfupload_widget.js...
error: swfupload_widget.js: No such file or directory
Checking patch swfupload.css...
Checking patch swfupload.module...
error: while searching for:
        $flash_url = base_path() . str_replace('.js', '.swf', array_shift($swfupload_library->scripts['2.2.0.1']));
      }

      $settings['swfupload_settings'][$element['#id']] = array(
        'module_path' => $path,
        'flash_url' => $flash_url,

error: patch failed: swfupload.module:162
error: swfupload.module: patch does not apply
Checking patch swfupload_widget.inc...

Was this simply because I was patching from the wrong directory or...?

q./

roderik’s picture

You must be running a complete different version of the swfupload module, if you have no swfupload_widget.js. Are you sure you're running Drupal 6? :-)

If you are, then try 'patch < 875706-20.patch'. (First run 'patch --dry-run < 875706-20.patch' to try.)
I don't know 'git apply' well enough yet, to know it's details - and this patch wasn't made with git yet. (My next one will be...)

designguru’s picture

Roderik,

I just tried the dry run and am still running into errors with your patch not finding the right files, though I have them on the server (and yes, am running the latest version of the module and drupal 6). Here's what I got with the dry patch command you suggested:

patch --dry-run < 875706-20.patch
can't find file to patch at input line 4
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|=== modified file 'sites/all/modules/swfupload/js/swfupload_widget.js'
|--- js/swfupload_widget.js	2010-10-19 00:39:29 +0000
|+++ js/swfupload_widget.js	2011-08-09 03:52:06 +0000
--------------------------
File to patch: js/swfupload_widget.js
patching file js/swfupload_widget.js
patching file swfupload.css
patching file swfupload.module
Hunk #1 FAILED at 162.
Hunk #2 FAILED at 190.
2 out of 2 hunks FAILED -- saving rejects to file swfupload.module.rej
patching file swfupload_widget.inc

Any ideas? This time the patch failure seems to be related to the module, looking at this yes, it does seem that my swfupload.module file is different from yours - I'm using 6.x-2.0-beta8 how about you?

q./

roderik’s picture

Could you try to apply the patch from #1050506: prevent adding js setting twice (causing white screen on node preview) before the above one?

Not all too professional maybe... I'll get a git repo to clone, when I have time :)

roderik’s picture

StatusFileSize
new534 bytes

Fixed another bug:

With editable descriptions, if your description contained a double quote - this would lead to a PHP fatal error on submit/preview.

The following small patch is in addition to #20. I'll fix the patch mess later.

biblos’s picture

It will be great to have one patch to fix problem on clear module install.

pfournier’s picture

@roderik: a user reported the same error as Sansui in #13. You said:

field_file_load() always returns an array

It may return FALSE if $file['fid'] is not found in the files table. I do not really understand why this would happens, but it happens.

roderik’s picture

StatusFileSize
new21.05 KB

Hit & run post. I'm only 'gitifying' my stack of patches.

@pfournier: thank you for this observation. I'll have a look at it when I have more time.

Below patch is #20 & #25 rolled into one. Nothing changed. It applies with "patch -p1" now, like all 'git diff's.

* The patch fails if #1050506: prevent adding js setting twice (causing white screen on node preview) is not applied.

(And I've never run this #875706 without #1050506 already applied. So you must apply it first (or reroll it yourself) ... I'm blackmailing you into testing / RTBC'ing #1050506; after that's done I'll consider uploading a patch that applies against the unpatched module ;-) )

* It applies with offset -15 in the .js file. This is because of #1242238: Display POST errors on widget initialization already being applied on my side. You don't need to apply that one in order to make things work.

jstnhffmn’s picture

I just tried #1050506 then #875706-28. I get the "warning: array_merge() [function.array-merge]: Argument #1 is not an array in /home/itravlr/subdomains/stage/sites/all/modules/swfupload/swfupload_widget.inc on line 60" error.

Also, after submitting the node edit form (without even touching the photos table) all the photos are gone. I don't know if this is because of the above error or not.

I have the 6.x-2.0-beta8 version of SWFUpload, and Drupal 6.26.

Ignore the whole thing about files disappearing. It never occurred to me that this is a result of live site content being copied to a development site. (The node points to non-existent files.)

Would it be suitable to change this line

$form_state['values'][$element['#field_name']][$key] = array_merge(
        field_file_load($file['fid']), // Load all fields, such as 'filepath'.
        array(
          'list' => $file['list'],
          'data' => array(
            'description' => $file['description'],
            'alt' => $file['alt'],
            'title' => $file['title'],
          ),
        )
      );

to

$form_state['values'][$element['#field_name']][$key] = array_merge(
        ($file_loaded = field_file_load($file['fid']) ? $file_loaded ? array()), // Load all fields, such as 'filepath'.
        array(
          'list' => $file['list'],
          'data' => array(
            'description' => $file['description'],
            'alt' => $file['alt'],
            'title' => $file['title'],
          ),
        )
      );

or

$form_state['values'][$element['#field_name']][$key] = array_merge(
        ($file_loaded = field_file_load($file['fid']) ? $file_loaded ? field_file_load(0)), // Load all fields, such as 'filepath'.
        array(
          'list' => $file['list'],
          'data' => array(
            'description' => $file['description'],
            'alt' => $file['alt'],
            'title' => $file['title'],
          ),
        )
      );

I'm not very familiar with the rest of the code so I don't know what kind of values are expected in the first parameter of this array_merge call.

Note: field_file_load(0) will return array('fid' => 0, 'filepath' => '', 'filename' => '', 'filemime' => '', 'filesize' => 0).

jstnhffmn’s picture

Anyone else getting this error: "warning: htmlspecialchars() expects parameter 1 to be string, array given in /home/itravlr/subdomains/stage/includes/bootstrap.inc on line 860"? (This is within the Drupal defined function check_plain.)

I did make some minor changes to my custom code, but a search through my local code only shows calls to check_plain within the CCK and FileField modules. (ie. It's hard to believe that my minor changes are the cause of this error as they are just valid changes to a couple of unrelated queries.)

mauritsl’s picture

Issue summary: View changes
StatusFileSize
new4.95 KB

Patch from #20 cleared all my files after saving.
Another approach may be to add a 'weight' attribute, instead of rewriting the object to an array. This has less impact on the rest of the code. Patch applied uses this approach and applies to beta8.