945952 - Cannot upload non-multimedia files

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Tomasz Szatkowski]

Attachment: filepicker.patch

Prerequirements: install Explorer app
1. Launch something with input[type="file"] (app, website, doesn't matter).
2. Click on that input.
3. Select Explorer.
4. Select some non-multimedia file (not music, video or image) from SD card.

Excepted: input state should be changed.
Actual result: input still indicate no file.

Bug is only reproducible on actual device. In simulator it works ok. That's beacuse getFromTypeAndExtension method is called with mime='' and extension=''. Extensions in WebActivites (which is used by file input) are unknown. So root cause is empty mime. Probably problem lies in this file: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/devicestorage.properties

I made small patch, which resovle that bug (my first and I hope not last :) ).

But still empty mime can cause another bugs in the future.

Comment 1

[Michael]

Comment on attachment 8341973 [details] [diff] [review]
filepicker.patch

You will want to have this reviewed.

Comment 2

[[:fabrice] Fabrice Desré]

Comment on attachment 8341973 [details] [diff] [review]
filepicker.patch

Review of attachment 8341973 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for reporting that and working on a fix!

That doesn't look like the right fix yet, but I'm happy to help you there.
Can you tell me where I can install your Explorer app?

::: b2g/components/FilePicker.js
@@ +169,5 @@
>      var mimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
> +    var mimeInfo = '';
> +    if (data.result.blob.type) {
> +	  mimeSvc.getFromTypeAndExtension(data.result.blob.type, '')
> +    }    

mimeInfo should not be a string, and will always be '' with this patch. Look at https://mxr.mozilla.org/mozilla-central/source/netwerk/mime/nsIMIMEService.idl#42 for the expected type.

Comment 3

[Tomasz Szatkowski]

Explorer is not my app. It's available in Marketplace (https://marketplace.firefox.com/app/explorer).

Oops, I forgot about assignment. I'll upload new version of patch. BTW, in newest commit someone placed object in prototype, I fixed it too.

Comment 4

[Tomasz Szatkowski]

Attachment: filepicker-rev2.patch

Comment 5

[[:fabrice] Fabrice Desré]

Comment on attachment 8342606 [details] [diff] [review]
filepicker-rev2.patch

Review of attachment 8342606 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/FilePicker.js
@@ +180,3 @@
>      }
>  
>      var mimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);

move that declaration where it's use, and s/var/let

@@ +183,4 @@
>  
>      var name = 'blob';
> +    if (data.result.blob.type) {
> +	  let mimeInfo = mimeSvc.getFromTypeAndExtension(data.result.blob.type, '');

nit: 2 spaces indent please.

@@ +187,1 @@
>        name += '.' + mimeInfo.primaryExtension;

you still need to check that mimeInfo is not falsy there.

Comment 6

[Tomasz Szatkowski]

Attachment: filepicker.patch

Comment 7

[[:fabrice] Fabrice Desré]

Comment on attachment 8342636 [details] [diff] [review]
filepicker.patch

Review of attachment 8342636 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/FilePicker.js
@@ +186,1 @@
>        name += '.' + mimeInfo.primaryExtension;

that will fail if mimeInfo is null.

Comment 8

[Tomasz Szatkowski]

Attachment: filepicker.patch

Comment 9

[[:fabrice] Fabrice Desré]

Comment on attachment 8344262 [details] [diff] [review]
filepicker.patch

Review of attachment 8344262 [details] [diff] [review]:
-----------------------------------------------------------------

thanks!

Comment 10

[Tomasz Szatkowski]

(In reply to Fabrice Desré [:fabrice] from comment #9)
> Comment on attachment 8344262 [details] [diff] [review]
> filepicker.patch
> 
> Review of attachment 8344262 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> thanks!

Thank you too.

One more question... what's next? :)
How to send it to central?

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/b936197a443c

Thanks for the patch, Tomasz! One request - please make sure that future patches include commit information. It makes life much easier for those landing on your behalf. Thanks!
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b936197a443c