Fix uploading multiple images from Photo Library on iOS 9.

[AndrewSouthpaw]

When selecting images from the Photo Library in iOS 9 (say, using Safari), all images are incorrectly given the name image.jpeg. This bug was corrected in iOS 10. Unfortunately, this bug makes it impossible to upload multiple photos from the Photo Library using iOS 9.

This PR addresses that bug and postfixes a simple index counter to disambiguate the filenames. It borrows inspiration from a similar bugfix for a different library.

Bug repro steps

1. Open Simulator running iOS 9 on any device (e.g. iPad 2).
2. Attempt to upload multiple photos from the Photo Library.

3. All photos are named image.jpeg and only one will be uploaded. 🐞

Other notes

I tried writing a test for it, but I'm unfamiliar with the testing suite and didn't see an obvious way to test it, based on the tests that are currently there. I am very receptive to guidance on how to write a test that would capture this case. (I tried putting it next to some of the tests that invoke _onChange but couldn't get the input to contain multiple files for the test.)

Also, I recognize the check is rather primitive and not exhaustive, but I also anticipated edge cases (e.g. uploading image.jpeg, image.jpeg, and foo.jpg would result in strange behavior) was highly unlikely. Happy to revamp the check if you think it's necessary. I also wasn't sure what basic JS functionality would be available to me for maximum browser compatibility -- I'm so used to working in ES2015 and above these days. This solution was good enough for us, so I thought I'd share it, but if it needs to be more generalized LMK and I can poke at it some more.

[blueimp]

Thanks for your contribution, @AndrewSouthpaw !

I'm not sure if the client-side is the right location to fix this issue.
As far as I'm aware, it is technically correct to create a multipart message with duplicate parts, including duplicate filenames sent via content-disposition header.
Although the issue might be fixed in iOS 10 and on Safari, other clients might still send you duplicate filenames.

So my advice would be to handle this on the server-side and rename files accordingly if there are duplicate filenames.
e.g. the example PHP code does it in the get_unique_filename function.

Is there any reason why you wouldn't be able to fix this on the server-side?

[AndrewSouthpaw]

Interesting, I didn't know about being able to upload multipart message with duplicate parts. Thanks for the feedback!

I have a couple thoughts still in favor of a client-side solution.

One is that the UI end state looks confusing and different from a normal case. It's not exactly wrong, but it's kinda not right either.

And the animations along the way are pretty perplexing as well.

The other thought is that you're totally right, you could send a multipart message and have it parsed by the server, and that's definitely not obvious to the casual user of this library. People want it to Just Work, and we never would've thought to check iOS 9 compatibility. It wound up causing a lot of unexpected frustration for our users who are stuck with older devices that can only go up to iOS 9.

We didn't even know the multipart message with duplicate parts was a possibility, and wouldn't have thought to parse it into separate files on the server, and it seems like more work and more error-prone than disambiguating on the client-side.

(I would also be happy to rework the PR to be a more general solution to handle duplicate filenames on the client side should they arise, if that helps the PR along.)

[blueimp]

Can you reproduce the animation issue on the demo?
I could not reproduce any such issue when testing on Chrome with same-named files (you can test this easily by adding the same file from different folders).
It is also likely unrelated to file naming.

There is documentation on how to provide alternative filenames in the Wiki:
https://github.com/blueimp/jQuery-File-Upload/wiki/API#how-to-provide-alternative-file-names
The core library does already provide support for the uploadName property as alternative name - please note that the name property itself is read-only, see https://developer.mozilla.org/en-US/docs/Web/API/File#Properties

I agree that the UI listing for files with the same name is not ideal.
But I also don't think listing the same file name with increasing suffix numbers is much of an improvement.
I'm thinking that for mobile devices, it's probably best to not display any filename at all, since filenames on mobile devices are usually not set by the user.

So yeah, I think the solution for this issue is a UI change, not a change in the core library.
I've released a new version that adjusts the sample template to hide filenames on mobile devices:
b29ff8e

[AndrewSouthpaw]

Interesting! The demo worked fine on iOS 9 + Safari. I'll have to dig around in our code to see how it's being used and what is the difference from the example.

If you specify an uploadName would that mean not having to parse duplicate names on the server as you mentioned previously? We don't have the liberty of parsing it on the server because we're using pre-signed keys for direct uploads to S3.

(Also, window.orientation is deprecated, it might be good to find a different way to identify mobile browsers.)

[blueimp]

Yes, the uploadName is specifically meant to transmit an alternative file name to the server (the UI will still show the original name).

I did not know that window.orientation is deprecated.
Can you point me to some info on this?
Did not find a deprecation mention on https://developer.mozilla.org/en-US/docs/Web/API/Screen/orientation

[AndrewSouthpaw]

That's window.screen.orientation, you were using window.orientation. Link :-)

I guess the question for me remains... this still seems like a gotcha for iOS 9 users. We never would've thought of checking for this, until we received some complaints from users. The library says it's compatible for Apple Safari on iOS 6+, but it effectively breaks for iOS 9 without special code either on the client or server.

Are you viewing it differently?

[blueimp]

Thanks for the heads up regarding the window.orientation deprecation.
I've added a new commit (d1f2707) that uses window.innerWidth instead.
Since the cutoff size is 480, that means tablet devices (e.g. iPad) will still show the file names.

Regarding non-unique filenames, my opinion is still the same:
Proper handling should be done on the server-side.

If non-unique filenames pose an issue for you, you might have the same problem no matter which client-side solution you choose if
a) The same user uploads files with the same name at different times
b) The same user uploads files with the same name from different browsers
c) Multiple users upload files with the same name to the same storage directory

In addition, should you support Internet Explorer (e.g. the only still supported version: IE11), you will receive filenames that contain the original directory path as part of the filename.

If you can mitigate these problems and you still have a need to upload files in one session with unique filenames from client-side, you can always make use of the library's callbacks to set the uploadName property as explained in https://github.com/blueimp/jQuery-File-Upload/wiki/API#how-to-provide-alternative-file-names.

Since you're doing direct uploads to S3, overriding uploadName does make sense - I would recommend you to generate random, unique filenames and store the original filenames in a database.
In general, I would treat the original, sanitized filenames as labels that are helpful to users when downloading files, but not ideal as storage identifiers if it can be avoided.

[AndrewSouthpaw]

All of that makes complete sense. From what I understand of your position, there are many cases in which non-unique filenames may need to be handled, as discussed in reasons a), b), and c), all of which may not be addressed by a client-side solution, but could be fully addressed by either a server-side solution or using the uploadName trick.

I'm going to press back a little more, and I hope this discussion still feels constructive to you. I understand if you want to just be done with the conversation and I'll drop it. 😅

Dealing with situations a), b), and c) are different from our situation, as I see it. For us in each of those cases, it's okay for the file to be overwritten, which is what would and should happen in our case. (e.g. User uploads img_001.jpg and then later uploads img_001.jpg again, we'd want it to overwrite.) In my eyes, we're dealing with a case D, where they're uploading multiple files that shouldn't be the same filename but are because of this iOS 9 bug. We did not know we'd need to handle this case. We thought it would be impossible, because a directory cannot contain duplicate filenames and we could only choose from one directory at a time, and that's where the bug tripped us up.

So... I agree with you that a client-side solution is not the correct way to solve for situations A, B, and C, and also it leaves unsuspecting folks supporting iOS 9 in case D at risk for running into this problem.

The uploadName trick does solve for this problem, but it also means that the library doesn't support iOS 9 the way it is advertised; without additional code as shown in the demos and on the README, photos from iOS 9 Safari would not upload correctly.

However, if an actual change to the code isn't warranted, what would you think about adding a caveat in the README somewhere, perhaps a disclaimer about supporting iOS Safari browsers and point to this discussion?

[blueimp]

Don't worry @AndrewSouthpaw, I think your comments and this discussion are definitely constructive.

I am somewhat reluctant to make changes to this project, since I consider its API stable and the project itself in maintenance mode.
That being said, I agree with you that good software should not surprise developers in unexpected ways.

I am open to fixes for specific Operating System or Browser versions, if they affect a larger number of users.
According to Apple, versions below iOS11 account for less than 5% of the user base - however, iOS9 is the latest supported version on iPhone 4S and similar older Apple mobile devices.

So I think there is definitely a need for at least a workaround for use cases like yours:
Fixing non-unique filenames for direct uploads to S3 (and direct uploads to S3 make a lot of sense) for users on older iOS devices.

I think to to do this properly would be to add all selected files to a map and for each file, check if its file name already exists and if it does, adjust the uploadName property for that file, adding a counter suffix similar to the PHP upcount_name method.

I'm still not sure if this code should be included in the core library, but am definitely up for a note in the README and an updated example in the Wiki.
If this should be added to the core library, I'm thinking it should be something that has to be enabled with an option setting.

[AndrewSouthpaw]

Ok! Sorry to let that go stale.

I added some more code, de-duping in the way you requested, which makes sense.

I also added a snippet to simply check if they're all duplicate names, and dedupe them if that's the case (because it's indicating a bug). Happy to drop if you think that's not worth it.

Wasn't sure if this.options.dedupeFilenames is how you access it or the name you'd want, happy for suggestions.

Once we square away the implementation details, then I can make an edit to the README (e.g. if turning on a flag is required, then I'd mention that in a caveat about browser compatibility perhaps)

[blueimp]

Hey @AndrewSouthpaw,
I've added a new commit that hopefully solves this problem, but also helps to address some of the issues I mentioned in previous comments here.

This implementation also has some performance benefits, as it doesn't add any additional files array loops.

I've introduced a new option called uniqueFilenames that can be seeded with a map of existing filenames, e.g.:

{ "image.jpg": true, "image (1).jpg": true }

It will override non-unique filenames on upload (via content-disposition header) by adding increasing number suffixes, e.g. "image (2).jpg".

The map of existing filenames can also simply be the empty object ({}), which will also enable the unique filenames feature.

Since this option specifically addresses direct file uploads to a storage that doesn't support duplicate filename handling (like Amazon S3), maybe it makes sense to create a new Wiki page, explaining the circumstances and how to enable and use this option.

[AndrewSouthpaw]

Hm, interesting. Solves the problem in a different way, but gets the job done either way. 😄 Mind if I submit a PR to update the README and add the caveat for iOS 9?

[AndrewSouthpaw]

Actually... scratch that. The solution fixes the issue about uploading to S3 (thank you!), but it doesn't address the wiggly uploading animation as shown in the GIFs of the OP.

FWIW my PR did address that issue. If there's something you'd like me to change about it (e.g. only do it when uniqueFilenames is true and never any other time), I'm very happy to adjust the logic.

[blueimp]

Regarding the animation issues you encounter, we did establish that this was not reproducible on the demo, as posted in your comment above.

And yes, feel free to provide a PR with an updated README - please make sure to point out that this affects mainly iOS9 and should usually be addressed on server-side, but the option is there for cases where no server-side support is given, like direct uploads to S3.