SPAN inserted as a child of UL

[powerbuoy]

When specifying the session endpoint in order to pre populate FineUploader with already uploaded files, I noticed that the JS puts all the previously uploaded images inside a SPAN. The problem with this is that the SPAN is then appended as a direct child of the UL so you get a structure like:

ul
    span
        li
        li
        li
    li
    li
    li

Where the previously uploaded images are inside the span li and images uploaded afterwards are added properly directly to the UL.

Not only is this invalid HTML and probably breaks rendering in some browsers, it makes styling much harder. Instead of being able to just style ul > li and use things like nth-child(odd) (etc) we have to jump through hoops to get the styling we want.

I noticed that simply removing fileList.appendChild(fileBatch.content); on line 7407 and also changing fileContainer = batch ? fileBatch.content : fileList to fileContainer = false ? fileBatch.content : fileList on line 7345 adds all the images directly to the UL. I'm afraid this will break something else though of course.

Any ideas?

[rnicholus]

Can you either close this issue or delete the Stack Overflow question?

[rnicholus]

Actually, since this is a bug report, this should only be filed here, and not on Stack. Please delete the duplicate question you opened on Stack.

[powerbuoy]

I've deleted the one on SO. Do you think it's safe to make the changes I suggested? Thanks

[rnicholus]

Do you think it's safe to make the changes I suggested?

No. This is likely a regression caused by work on #1466. The trick will be to fix this without re-introducing the issue addressed in #1466. The solution involved building up the initial file list disconnected from the document and then adding everything at once in one operation. I'll have to give this some thought.

[rnicholus]

After a few moments of reflection, I think the fix will be simple and appropriate.

1. Instead of building up the initial file <li>s in a disconnected <span>, build them in a DocumentFragment
2. appendChild the DocumentFragment of <li>s when all initial files in a batch are ready.

[powerbuoy]

That sounds perfect. Any ETA on the fix? Or should I give it a go myself? Thanks for the extremely quick reply!

[rnicholus]

Any ETA on the fix?

I'd like to push this out as a hotfix release in the next day or two. I'd much rather do so only after verification that the fix works and that it doesn't negate the work done to address #1466. Will you be available to verify the fix in the next day or two?

[powerbuoy]

I will, yes. Thanks a lot

[rnicholus]

Ok, If the fix goes as easy as I hope, I think I'll be able to push this out in the next couple days. If anything goes wrong, I'll have to wait until mid-December, as I'm out of the office for a few weeks starting at the end of this month and I don't want to cause problems for those monitoring the project during my absence.

If you are comfortable adjusting the code on your end and letting me know if you run into any issues, that will be helpful. Either way, it's late here and I'm offline for the next 8 hours.

[rnicholus]

Fix is being staged on branch hotfix/5.4.1. Let me know if you need any help verifying.

[powerbuoy]

I'm sorry if I sound like a complete noob, but I couldn't find where to download the complete fine-uploader.js (minified or otherwise), perhaps I need to run grunt on the project myself?

Anyway, I saw the change you made (unless I missed something) and applied it myself in 5.4.0 (changed document.createElement("span") to document.createDocumentFragment() in two places (a0461a8)) and it completely fixes the problem I had.

I can't say for sure whether it introduces some new problem, but with the testing I've done I haven't noticed anything out of the ordinary at least.

Btw, you deserve some kind of medal for the speed at which you reply to (and FIX!) issues. Kudos!

[rnicholus]

Sounds like you've tested this well enough. Normally, you could either pull down the built JS files from S3, or build yourself using grunt. I don't think the build process pushes anything up to S3 on any branch other than develop or master though.

[rnicholus]

fixed in 5.4.1

[powerbuoy]

Awesome work. Thanks!