1085158 - FilePicker.js: "File is not defined" after selecting file to upload

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[joejob357]

User Agent: Mozilla/5.0 (Android; Tablet; rv:35.0) Gecko/35.0 Firefox/35.0
Build ID: 20141019004001

Steps to reproduce:

* Load: http://validator.w3.org/#validate_by_upload
* Click Browse
* Select file

This is using Aurora on CyanogenMod Nightly on the Nexus 10. It is reproducible on this and at least one other site with a file upload field.


Actual results:

* The file upload field continued to report "No file specified"
* Logcat displayed

W/GeckoConsole( 5002): [JavaScript Error: "ReferenceError: File is not defined" {file: "jar:jar:file:///data/app/org.mozilla.fennec_aurora-2.apk!/assets/omni.ja!/components/FilePicker.js" line: 146}]
W/GeckoConsole( 5002): [JavaScript Error: "NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "File is not defined" {file: "jar:jar:file:///data/app/org.mozilla.fennec_aurora-2.apk!/assets/omni.ja!/components/FilePicker.js" line: 146}]'[JavaScript Error: "File is not defined" {file: "jar:jar:file:///data/app/org.mozilla.fennec_aurora-2.apk!/assets/omni.ja!/components/FilePicker.js" line: 146}]' when calling method: [nsIFilePicker::domfile]" {file: "jar:jar:file:///data/app/org.mozilla.fennec_aurora-2.apk!/assets/omni.ja!/components/FilePicker.js" line: 247}]

I have had a similar problem running the release version of Firefox on this platform, but zooming out the page then zooming back forced the field to update. This no longer works.


Expected results:

The firm field should have updated to reflect the chosen file.

Comment 1

[joejob357]

To expand, this workflow succeeds on v33 on the same platform; I haven't tested on v34.

Comment 2

[Teodora Vermesan (:TeoVermesan)]

After performing your steps, firefox crashes: https://crash-stats.mozilla.com/report/index/c6329104-8e64-4496-8f4f-3943f2141020
Tested with: 
Device: Samsung S5 (Android 4.4)
Build: Firefox for Android 36.0a1 (2014-10-19)

Comment 4

[Aaron Train [:aaronmt]]

[Tracking Requested - why for this release]: File uploading is broken

Comment 5

[Aaron Train [:aaronmt]]

I crash too. I'm using http://encodable.com/uploaddemo/ as a more appropriate uploader for example here. Looking for a range now.

Comment 6

[Wesley Johnston (:wesj)]

The "File" constructor was removed from JSComponents here (bug 1047483):

https://hg.mozilla.org/releases/mozilla-aurora/rev/53a14a912960

I'm assuming its old and deprecated and we should never have used it or something. Was intentional? bz?

Comment 7

[Aaron Train [:aaronmt]]

Last good revision: dd7637cc42d5 (2014-10-08)
First bad revision: f0bb13ef0ee4 (2014-10-09)
Pushlog:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dd7637cc42d5&tochange=f0bb13ef0ee4

Comment 8

[Boris Zbarsky [:bzbarsky]]

We had a shim File constructor, looks like.  I'm not quite sure why exactly it got removed.  Bobby, should DOM File be exposed in JSMs by default?

In any case, for now you should be able to do |Cu.importGlobalProperties(['File'])| to import the 'File' constructor.  That _should_ have happened in bug 1047483 but perhaps the fact that this file is named .js, not .jsm, confused things?

Comment 9

[Andrea Marchesini [:baku]]

> In any case, for now you should be able to do
> |Cu.importGlobalProperties(['File'])| to import the 'File' constructor. 
> That _should_ have happened in bug 1047483 but perhaps the fact that this
> file is named .js, not .jsm, confused things?

Actually, this File() is exposed to content so probably we should do:

return new this._domWin.File(file);

instead: return File(file);

But it seems that this._domWin can be null (can it really be?)

I'll propose a patch for this.

Comment 10

[Andrea Marchesini [:baku]]

Attachment: file.patch

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8508126 [details] [diff] [review]
file.patch

r=me

Comment 12

[Andrea Marchesini [:baku]]

Attachment: file.patch

Comment 13

[Carsten Book [:Tomcat]]

Hi, do you have a try link for this change? thanks!

Comment 14

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] from comment #8)
> We had a shim File constructor, looks like.  I'm not quite sure why exactly
> it got removed.  Bobby, should DOM File be exposed in JSMs by default?

Not AFAIK, at least until we add Exposed=System.

Comment 15

[Andrea Marchesini [:baku]]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b2fc50d00744

Comment 16

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1abd1e91bcdf

Comment 17

[Boris Zbarsky [:bzbarsky]]

> Not AFAIK, at least until we add Exposed=System.

The question is whether this interface should have Exposed=System, I guess.

Comment 18

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] from comment #17)
> > Not AFAIK, at least until we add Exposed=System.
> 
> The question is whether this interface should have Exposed=System, I guess.

Fine by me, as long as we write tests.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1abd1e91bcdf

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

Can we get an uplift request to Fx35?

Comment 21

[Andrea Marchesini [:baku]]

Comment on attachment 8508280 [details] [diff] [review]
file.patch

Approval Request Comment
[Feature/regressing bug #]: 1047483
[User impact if declined]: android filepicker is broken
[Describe test coverage new/current, TBPL]: none. it seems that filepicker doesn't have tests in general.
[Risks and why]: the patch is very simple and it has been already verified. 
[String/UUID change made/needed]: none

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f42870da259b

Comment 23

[Cristina Madaras, QA [:CristinaM]]

Firefox crashes after performing steps from comment 0: https://crash-stats.mozilla.com/report/index/e1fe4531-3d49-4290-b330-48f312141202

Tested with:
Firefox for Android 35 Beta 1
Firefox for Android 36.0a2 (2014-12-01)
Firefox for Android 37.0a1 (2014-1-01) is also affected.

Device: Nexus 4 (Android 4.4.4). See Bug 1031429.

Comment 24

[Cristina Madaras, QA [:CristinaM]]

I also tested this with the following devices:
Motorla Razr (Android 4.1.2) and Asus Transformer Pad TF300T (Android 4.2.1)

On Firefox for Android 35 Beta 1
With URL: http://encodable.com/uploaddemo/ - after a file is selected, 'No files selected' text is changed to reflect the chosen file.
With URL from comment 0: http://validator.w3.org/#validate_by_upload - after a file is selected, 'No file selected' text is not updated to reflect the chosen file.

Firefox for Android 36.0a2 (2014-12-02) works as expected with both URLs. I will mark as verified only Firefox 36.

Comment 25

[u421692]

Verified as fixed on Firefox 35 Beta 4 on Samsung Galaxy Nexus (Android 4.2.1)

Comment 26

[Stefan Deiac]

Based on comment 24 and 25, I will remove the qe-verify flag, thanks.