The default bug view has changed. See this FAQ.

Selecting a large number of files in <input type="file"> hangs browser

VERIFIED FIXED in mozilla7

Status

()

Core
Widget: Cocoa
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Scott Hughes, Assigned: mounir)

Tracking

({hang})

Trunk
mozilla7
All
Mac OS X
Points:
---
Bug Flags:
in-testsuite -
in-litmus ?

Firefox Tracking Flags

(firefox5 affected, firefox6 affected)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_7) AppleWebKit/534.24 (KHTML, like Gecko) Chrome/11.0.696.71 Safari/534.24
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

Selecting ~1000 files in a multiple file input causes Firefox Mac to hang temporarily. After ~1 minute, the UI becomes responsive again.

Reproducible: Always

Steps to Reproduce:
1. Load the test case URL.
2. Click the "Browse" button on the file input.
3. Select a large number (e.g. 1000) of files in the file dialog.
4. Click the file dialog's "Open" button.

Actual Results:  
Firefox hangs for a period of time proportional to the number of files selected.

Expected Results:  
Firefox should not hang. Chrome and Safari do not hang with the above steps.

This appears to only affect Firefox Mac. Windows and Ubuntu do not hang with the above steps.

Updated

6 years ago
Component: File Handling → DOM: Core & HTML
QA Contact: file-handling → general

Comment 1

6 years ago
I'm not seeing any such behaviour on Fedora 12 nightlies.
(Assignee)

Comment 2

6 years ago
I've been able to reproduce this on MacOS X 10.6 with a Nightly.

I will try with my Gentoo and check were we are spending our CPU cycles.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
(Assignee)

Updated

6 years ago
Keywords: hang
(Assignee)

Comment 3

6 years ago
By the way, thank you very much for this report Scott ;)
(Assignee)

Comment 4

6 years ago
This hang is MacOS X specific and appears here:
https://mxr.mozilla.org/mozilla-central/source/widget/src/cocoa/nsFilePicker.mm#403

On GTK, I got a very quick hang but I believe it happens when copying the file names in the <input type=file>.
Component: DOM: Core & HTML → Widget: Cocoa
QA Contact: general → cocoa
Hardware: x86 → All
(Assignee)

Comment 5

6 years ago
I have a fix. I'm looking at something and will attach here for review.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
(Assignee)

Comment 6

6 years ago
Created attachment 536262 [details] [diff] [review]
Patch v1
Attachment #536262 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #536262 - Flags: review? → review?(joshmoz)
(Assignee)

Updated

6 years ago
Whiteboard: [needs review]
(Assignee)

Updated

6 years ago
status-firefox5: --- → affected
status-firefox6: --- → affected

Comment 7

6 years ago
Comment on attachment 536262 [details] [diff] [review]
Patch v1

>+  for (NSURL* url in [thePanel URLs]) {

We don't use Obj-C 2.0 anywhere yet, which means no "in" loop form. Please just write this the old way. We'll have to look into whether we want to allow 2.0 features at some point.
Attachment #536262 - Flags: review?(joshmoz) → review-
(Assignee)

Comment 8

6 years ago
Created attachment 536303 [details] [diff] [review]
Patch v2

That's a shame because Fast Enumeration seems nice. I didn't knew it was in Obj-C 2.0. Though, I didn't knew there was a 2.0 version of Obj-C ;)
Attachment #536262 - Attachment is obsolete: true
Attachment #536303 - Flags: review?(joshmoz)

Comment 9

6 years ago
Comment on attachment 536303 [details] [diff] [review]
Patch v2

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

::: widget/src/cocoa/nsFilePicker.mm
@@ +404,5 @@
>    
>    if (result == NSFileHandlingPanelCancelButton)
>      return retVal;
> +
> +  // Converts data from a NSArray af NSURL to the returned format.

Spelling.

@@ +406,5 @@
>      return retVal;
> +
> +  // Converts data from a NSArray af NSURL to the returned format.
> +  // We should be careful to not create the array for each iteration in the loop to prevent
> +  // hangs when a lot of files are selected.

This comment is misleading because it isn't obvious what array creation you're talking about. I assume you mean to explain that getting URLs from the panel causes it to create a new array - which is an implementation detail if it's true, not something you'd be able to know from reading the code.
Attachment #536303 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 10

6 years ago
I'm going to push that with this comment instead:
// We should be careful to not call [thePanel URLs] more than once given that it
// creates a new array each time.
(Assignee)

Updated

6 years ago
Flags: in-testsuite-
Flags: in-litmus?
Whiteboard: [needs review]
(Assignee)

Updated

6 years ago
Whiteboard: [fixed in cedar]
(Assignee)

Comment 11

6 years ago
Pushed in m-c:
http://hg.mozilla.org/mozilla-central/rev/b9924776a728
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Target Milestone: --- → mozilla7
(Assignee)

Comment 12

6 years ago
Comment on attachment 536303 [details] [diff] [review]
Patch v2

Asking for approval in Aurora/Firefox 6 given that the fix is very simple and safe and might fix hangs.
Attachment #536303 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 13

6 years ago
Scott, thank you very much for filing this bug :) It will be fixed in Firefox 7 (maybe in Firefox 6 depending on the answer to my request). You will be able to test the fix if you use a Nightly build tomorrow, they are available here: https://nightly.mozilla.org/
(Assignee)

Updated

6 years ago
Depends on: 662164
(Reporter)

Comment 14

6 years ago
You're welcome. Thanks for such a fast response. :)

I've verified this is fixed on the 2011-06-05 nightly.

Comment 15

6 years ago
Comment on attachment 536303 [details] [diff] [review]
Patch v2

Not the kind of important regression fix or finishing off a feature that would otherwise be backed out so we'll wait on this to reach users in Firefox 7.
Attachment #536303 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-

Comment 16

6 years ago
Setting resolution to Verified Fixed on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0 beta 3

Firefox isn't hanging when a large number of files is loaded.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.