912492 - Consider clearing HTMLInputElement.files when HTMLInputElement.openDirectoryPicker returns

Status: NEW

(Core :: DOM: Core & HTML, defect, P5)

Description

[Jonathan Watt [:jwatt]]

I think we should consider clearing HTMLInputElement.files when HTMLInputElement.openDirectoryPicker returns. Right now we don't clear it until we've finished building up the list of files under the directory that the user picked, which may be some time after openDirectoryPicker returns. I think we should consider doing this because it seems like content authors might do the wrong thing when getting progress events that tell them that X number of files have been processed so far (thinking that .files contains those files, when in fact it contains the old files, if any existed).

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Oh, so the situation here is something like this:

1. Page calls openDirectoryPicker()
2. User chooses a directory
3. We start processing the directory and fire progress events
4. Page calls openDirectoryPicker() again
5. We still keep firing progress events from the initial selected directory
6. User chooses another directory
7. We clear input.files and start processing the new directory

Is that the scenario here?

If so I agree that in step 5 the page could indeed get confused. Clearing the selection and stopping any ongoing processing sounds like a good idea.

Comment 4

[Jonathan Watt [:jwatt]]

Jonas: no, that's not the scenario I'm thinking about. Bug 923922 should prevent step 5 from happening.

The scenario I'm concerned about is simpler than that. I'm thinking that the extra async activity and delay inherent to directory picking (vs file picking) may mean that authors may simply look at HTMLInputElement.files and start reading any old list of files that may still be there thinking that it's the completed result of the directory picking operation (before the potentially long lived operation has completed).

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

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

We need to somehow indicate that the filelist has changed.
So change event is needed.

Comment 7

[Jonathan Watt [:jwatt]]

It seems a bit weird to me that selecting a directory would result in two 'change' events (one on clearing the list when the scan starts, then a second on setting the final list).

The other thing Olli and I discussed on IRC is that maybe we should build .files incrementally, transferring files to the main thread as they become available. Would that mean that we'd want to fire 'change' events each time we appended to .files though (in addition to firing the progress event)? Would we fire 'input' events too?

Comment 8

[Jonathan Watt [:jwatt]]

I thought I qpop'ed the patch for this bug before qfinish-push'ing, but messed that up. Accidentally pushed, then backed out:

https://hg.mozilla.org/integration/mozilla-inbound/rev/a268f8037bb0
https://hg.mozilla.org/integration/mozilla-inbound/rev/f510f2d945c3

Comment 9

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.