Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a "cancel" event for when file upload selection is unchanged #6735

Merged
merged 3 commits into from
Jul 8, 2021

Conversation

domenic
Copy link
Member

@domenic domenic commented Jun 3, 2021

Closes #6376.

Specific choices made:

  • I chose "cancel" for the event name since we already have that (for <dialog>) and it seems nicer to reuse that than introduce something confusingly-similar (like "abort").
  • I left bubbles as false since most modern events don't seem to bubble. Maybe this is confusing though since both change and input bubble. I'm very open to changing this. Flipped based on below feedback.
  • I left composed as false. Interestingly input is composed but change is not. input event is composed but change isn't #5453 I'm not sure what the right thing to do here is.

(See WHATWG Working Mode: Changes for more details.)


/indices.html ( diff )
/input.html ( diff )

Copy link
Contributor

@adroitwhiz adroitwhiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fulfills the use case I had in mind in #6376!

With regards to event bubbling, I prefer API surfaces that are consistent rather than following the practices at the time each new feature is added, and I think that would be less confusing for web developers (though I'm not sure who would attach event handlers to a parent element rather than the input element itself). I'm a bit uneducated as to how event bubbling works, but are there any cancel events that could already be emitted by parent elements and listened for, that could cause compatibility issues if the input element starts emitting them as well and bubbling?

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

I would be okay with this being a composed event as I mentioned in that issue for its counterpart, but it's probably best to decide on them jointly.

cc @smaug----

source Show resolved Hide resolved
source Show resolved Hide resolved
@domenic domenic added the agenda+ To be discussed at a triage meeting label Jun 17, 2021
@domenic
Copy link
Member Author

domenic commented Jun 23, 2021

@mfreed7 do you want to review this? Any thoughts especially on the "Specific choices made" section would be welcome.


<li><p>Wait for the user to have made their selection.</p></li>

<li><p><span>Update the file selection</span> for the <code>input</code> element.</p></li>
<li><p>If the user dismissed the prompt without changing their selection, then <span>queue an
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to be more specific here? As written, this would fire "cancel" if a user selected a file, clicked OK, re-opened the file picker, selected the same file again, and clicked OK. In that case, the selection "would not be changed". Is that intentional? If so, perhaps this case could be explicitly folded into the new note below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that's right. Such a case would fire "cancel" and not "change", since nothing changed. I'll add it to the example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, well, it kind of depends; something might have changed, since maybe the files changed on disk? But I don't think the event should need to branch on that.

So I guess now I'm thinking it's up to the UA. As long as it picks one of the two events it's OK. I'll try to come up with some phrasing...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this a bit more, I'm thinking the more correct thing would be to fire change in this case (re-selecting the same file). The issue is that otherwise, a developer would have no way to detect this situation. There are two possibilities:

  1. Re-selection triggers cancel: There is no way to disambiguate re-selection from hitting cancel.
  2. Re-selection triggers change: The developer can, if they like, store the previously-selected file, and compare them upon receipt of change.

It seems plausible that a developer might want to do something different when the user clicks "ok" vs "cancel".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that's up to the UA. It seems like a valid user interface to say that if the user didn't change anything (i.e. re-selected the same files), then that shouldn't be distinguishable from hitting cancel.

@mfreed7
Copy link
Contributor

mfreed7 commented Jul 8, 2021

Overall this looks pretty good to me. I added a question about re-selecting the same file. And here are my thoughts on the choices:

Specific choices made:

  • I chose "cancel" for the event name since we already have that (for <dialog>) and it seems nicer to reuse that than introduce something confusingly-similar (like "abort").

I think "cancel" makes sense, both because of parallelism with other APIs but also because it maps to the name of the button ("Cancel") used by most UAs.

  • I left bubbles as false since most modern events don't seem to bubble. Maybe this is confusing though since both change and input bubble. I'm very open to changing this.

I was surprised by this one, I would have thought we'd set bubbles=true here. But no strong opinions.

From our last spec triage meeting, I think the consensus was that composed=false is the right thing here. Cancelling a file dialog is not a "super global user action" like click and keydown.

@domenic
Copy link
Member Author

domenic commented Jul 8, 2021

I'll change it to bubble since we've gotten a couple of small pieces of feedback in that direction.

@domenic domenic merged commit 9c4374f into main Jul 8, 2021
@domenic domenic deleted the file-upload-cancel branch July 8, 2021 16:45
@tomayac
Copy link

tomayac commented Jul 19, 2021

Follow-up question: is there a way to feature-detect support for this? My initial approach…

  'oncancel' in (i = document.createElement('input'), i.type = 'file', i)
  // `true`

…was not helpful.

@domenic
Copy link
Member Author

domenic commented Jul 19, 2021

I don't believe so, no.

foolip added a commit to foolip/browser-compat-data that referenced this pull request Jul 4, 2022
queengooborg added a commit to mdn/browser-compat-data that referenced this pull request Jul 10, 2022
@dbaron
Copy link
Member

dbaron commented Nov 23, 2022

I'm curious if it was intended that this change also applied to a drag/drop action that leaves the file selection unchanged. The current spec text for that says:

If the element is mutable, the user agent should allow the user to change the files on the list in other ways also, e.g., adding or removing files by drag-and-drop. When the user does so, the user agent must update the file selection for the element.

which seems to imply that cancel events should never fire for that case. This difference seems a little odd... and also seems to expose the mechanism the user used to change the selection, which may be undesirable.

@domenic
Copy link
Member Author

domenic commented Dec 5, 2022

Interesting question. When would you expect the cancel event to fire? After all, most of the time the user is not dragging-and-dropping files. It seems like it's the two-stage nature of (1) opening a picker, (2) then closing it, which lets us definitively decide at (1) that we're definitely going to fire an event, and at (2) which event to fire. Dropping is a single action, so I don't know how we would determine this...

@dbaron
Copy link
Member

dbaron commented Feb 24, 2023

I guess if the user drops the same set of files as the current set, I'd expect either (a) a cancel event or (b) no events at all to fire at that time. Firing an input and a change event when the files didn't change exposes (I think) that the action was drag/drop rather than via the picker. (Also, for the file picker case, Chromium, at least, fires the input and the change events at the same time, after the user has closed the picker.)

(Also, for the picker case, I don't recall if there are currently APIs that allow the page to open the file picker, but if there are, that's a good reason that a cancel event should be exposed.)

@dbaron
Copy link
Member

dbaron commented Feb 24, 2023

(Or, to more directly answer the question, I'd expect that cancel event to fire at the same time that the spec currently fires the input and change events.)

@tomayac
Copy link

tomayac commented Feb 24, 2023

Not sure if it's immediately relevant, but I just encountered an interesting corner case: pointing an <input type="file" webkitdirectory="" /> at an empty directory causes no change event. Should it cause a cancel event?

@dbaron
Copy link
Member

dbaron commented Feb 24, 2023

I should also note that Chromium uses the same code to handle both receiving dropped files and a result from the file chooser. So fixing the latter to fire cancel events in some cases will also fix the former at the same time.

(Not sure about the webkitdirectory case... but the above CL might change that behavior.)

@dbaron
Copy link
Member

dbaron commented Feb 24, 2023

Also, the older version of https://chromium-review.googlesource.com/c/chromium/src/+/4048886 has a test that might be worth adding to WPT if this discussion concludes.

@annevk
Copy link
Member

annevk commented Feb 24, 2023

@dbaron @tomayac could you please file new issues to track these concerns?

@dbaron
Copy link
Member

dbaron commented Feb 24, 2023

Filed #8945.

aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 24, 2023
See html spec [1] [2]. If the file upload selection for input type=file
is unchanged, we should fire the cancel event with bubbles true.

[1] https://html.spec.whatwg.org/#show-the-picker,-if-applicable
[2] whatwg/html#6735

Change-Id: I8fb0307df75c58c17ff1d180071cd77634a08a65
Fixed: 1227424
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4048886
Commit-Queue: Di Zhang <[email protected]>
Reviewed-by: David Baron <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1109778}
@xiaoxiyao
Copy link

How should I detect whether the browser supports this event?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements agenda+ To be discussed at a triage meeting topic: events topic: forms
Development

Successfully merging this pull request may close these issues.

Specify cancelling a file upload from a file input
8 participants