Can't cancel upload when collective.cover is installed

[hvelarde]

Plone 4.3 with collective.upload v1.3b1 installed:

• select Add files or images… in the folder_contents view or in the Add new… menu item
• select various files or images
• select Cancel on one of the items; the element is removed from the list of items that have to be uploaded

now install collective.cover v1.2b1 in the same instance:

• select Add files or images… in the folder_contents view or in the Add new… menu item
• select various files or images
• select Cancel on one of the items; nothing happens

probably related with #51.

[hvelarde]

I can confirm the same issue on a different site with Plone 4.3 and collective.upload v1.2b1 installed, so we broke this before.

we will need to add a RF test for this case.

[rodfersou]

@hvelarde I added a breakpoint in the cancel button callback and I confirm that it is fired. for every download it calls the abort method:
https://github.com/collective/collective.upload/blob/master/src/collective/upload/static/jquery.fileupload-ui.js#L536

I think the version of jquery upload we are using was written with some unsafe code, I mean, some way to code that didn't become specification, and now we can see some problems.

I suggest to try again to update the jQuery-File-Upload library.

[rodfersou]

@hvelarde now I see that cancel button cancel the upload before it starts (and clean the files listed after selection). Isn't it the desired behavior?

[hvelarde]

as mentioned above, is not working in production in two sites.

[rodfersou]

I didn't find an easy way to fix this. I suggest or to Update jQuery File Upload version or to remove cancel button.

[hvelarde]

we're not going to update the library because we have no resources for that and because the problem has nothing to do with that.

I just tested again and this only happens if you have collective.cover installed in the same instance that collective.upload is installed; because of that, I suspect the problem is related with #51.

I updated the issue description to include this information.

[rodfersou]

It looks like the collective.upload javascript files should be loaded before bootstrap.min.js file, changing the order it starts to work again.

Maybe we should add again the dependency of collective.js.bootstrap just to ensure the order of the javascript insertion (with "insert-before" attribute in jsregistry.xml) and add an upgrade step to fix the order.

[hvelarde]

instead, I think we must copy the code related with the feature to the package JS so we do not depend on any external package, neither on an specific loading order.

[rodfersou]

IMHO it looks like bootstrap is a dependency of jquery file upload, so it makes no sense to remove this dependency.

But I'm still not sure with what the problem is. In one specific commit this javascript order change works, but in the master branch it didn't work.. So I'm still trying to figure out what is going on here.

Besides, I try to add a Robot Framework test using the Choose File keyword, it fill the input with the file, but didn't trigger jquery file upload events for some reason.

[rodfersou]

My bad.. the problem is not related with the position of bootstrap.min.js in jsregistry, but the real problem is the lack of bootstrap.min.css in the cssregistry.

I realized it after diff the entire generated html and see that the only significant difference in the working and not working was the <link> tag with bootstrap.css file.

To make sure, in the console, when I run this code inserting back the bootstrap.css file the cancel button start to work:

$('head').append($('<link rel="stylesheet" type="text/css" media="screen" href="'+portal_url+'/portal_css/Sunburst%20Theme/++resource++collective.js.bootstrap/css/bootstrap.min.css">'))

The problem now is to identify what css rule makes the difference, and add it into main.css file.

Pretty weird right? I think so..

[rodfersou]

@hvelarde with the last version of collective.cover 1.4b1 and collective.upload 1.4b1.dev0 the bug is gone.

[hvelarde]

sorry to give you bad news but the problem is still present.

[rodfersou]

@hvelarde it is definitely working here, could you test again please?

[hvelarde]

those are the versions present on customer's staging site; test it there and you'll see the issue.

[rodfersou]

I did some tests at one of our customer's staging site, and conclude that updating two packages the problems are solved:

collective.js.bootstrap = 3.3.6
plone.app.jquery = 1.8.3

[hvelarde]

yes, because the offending classes will be gone.

[rodfersou]

looking now some erros at console, and plone.app.jquery should be a version greater than 1.9.1