638940 - File Upload: we should not change file names and extensions

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P2)

Description

[Thomas Arend [:tarend]]

Attachment: File Upload Glitches: Stock (correct) vs. Fennec

(1) go to a file upload site like xup.in or imageshack.com
(2) Upload an image from your Gallery (pick any existing photo)
(3) Upload a text file (you may need a file manager helper app, like OI File Manager, or similar)

Results: 
(a) Firefox changes the original image path to /data/data/org.mozilla.fennec 
(b) Firefox changes the file name from IMG_20110227_131650.jpg into tmp_625_1039733099.jpeg
(c) For the text file, Firefox changes the path (like in a) and the file name from CHANGES.txt into tmp_552-1484119723.plain

Expected:
(a) the path should not be changed
(b) the file name should not be changed
(c) the file extension should not be changed

See the attached screenshot that compares stock browser (correct behavior) with Fennec for the exact same upload files.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

we don't get access to the original file from the system, instead we get an abstracted object which we can read from. So we read that, write it to a temp file and upload it.

As implemented, we cannot change this behavior. I can look to see what the stock browser does, but in many ways they get to cheat, so I'm not sure that this is fixable.

Comment 2

[Thomas Arend [:tarend]]

Changing file extensions (c above) is probably the most critical issue here. The stock browser seems to do it right for all files and extensions. Thanks for looking into this!

Comment 3

[Thomas Arend [:tarend]]

Another user just reported this behavior:
Timo writes: "Hello,I've got the problem if I ipload a file, the name of the file will be replaced with a temp-filename from firefox.
If it will be fixed, it's the first browser for Android wir an upload-function inside."

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

We still have the issue of whether this is even do-able. The file extension problem was improved in bug 638523. Marking as a nom so we can talk about it in triage.

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

mostly cribbed this from the stock browser, see:
http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/2.2.1_r1/android/webkit/WebViewCore.java#293

Comment 6

[Doug Turner (:dougt)]

Comment on attachment 532871 [details] [diff] [review]
patch


>                 Uri uri = data.getData();
>-                String mimeType = cr.getType(uri);
>-                String fileExt = "." +
>-                    GeckoAppShell.getExtensionFromMimeType(mimeType);


>+                Cursor cursor = GeckoApp.mAppContext.getContentResolver().query(
>+                    uri, new String[] { OpenableColumns.DISPLAY_NAME },
>+                    null, null, null);


new lines for the parameters please.


>+                String name = null;

>+                if (cursor != null) {
>+                    try {
>+                        if (cursor.moveToNext()) {
>+                            name = cursor.getString(0);
>+                        }
>+                    } finally {
>+                        cursor.close();
>+                    }
>+                }


can the cursor.getString(0) ever return ""?

>+                if (name == null) {


assuming no "" strings.

Comment 7

[Jesse Ruderman]

I don't understand the complaint about "changing" the path. Fennec shouldn't be telling the web site the path to the file; desktop Firefox doesn't.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

you're right, and we don't

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/mozilla-central/rev/2169fe74d62f

Comment 10

[Andreea Pod]

Verified fixed on Nightly: Mozilla /5.0 (Android;Linux armv7l;rv:8.0a1) Gecko/20110712 Firefox/8.0a1 Fennec/8.0a1 

and Aurora: Mozilla /5.0 (Android;Linux armv7l;rv:7.0a2) Gecko/20110712 Firefox/7.0a2 Fennec/7.0a2

Device: LG Optimus 2X