[file_selector_web] Listens to file input cancel event. #3683

raju-muliyashiya · 2023-04-11T08:28:52Z

While using package file_selector making for multiple file request in web platform we are not able to get any data if user clicks cancel button on file selection window. This PR fixes it by watching the focus event and if onChange is not fired then return empty array, Empty array will only return if multiple selection is enabled with openFiles().

List which issues are fixed by this PR. You must list at least one issue.
Issue #121328

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
I signed the [CLA].
The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
All existing and new tests are passing.

google-cla · 2023-04-11T08:28:56Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

ditman

Hey @raju8000!

Thanks for this fix, however, there's now a cancel event that AFAIK is supported by almost all browsers (I just verified the event exists in Chrome Canary). See this comment.

Do you mind using that instead of the focus event trick?

I wrote a very small cancel/change event tester app here, because browsers do slightly different things when clicking ok/cancel and selecting the same file or a different one, so some logic will have to be added to figure out if the user was actually canceling the selection:

https://jsfiddle.net/ditman/avdpr9Lu

I think that this now can be solved with a combination of the change/cancel without needing to rely on the window focus.

Also, about the tests, please remove the tests that are not really testing the method that you're adding :)

ditman · 2023-04-12T00:49:21Z

packages/file_selector/file_selector/test/file_selector_test.dart

@@ -144,6 +144,65 @@ void main() {
    });
  });

+  group('cancelOpenFiles', () {


These tests are not testing anything that this change does :)

raju-muliyashiya · 2023-04-12T07:21:39Z

@ditman ,

*https://jsfiddle.net/ditman/avdpr9Lu

I have tried above given test app on latest versions of Safari, Chrome & FIrefox, eventually the cancel event seems to be working in only FIrefox browser, Crome @ Safari are not able to trigger cancel event.

how could this issue be tackled?

ditman · 2023-04-14T02:01:54Z

Crome @ Safari are not able to trigger cancel event.

@raju8000 I can see the "cancel" event in Chrome Beta (v113+) and Safari 16.4, but each browser triggers it differently (that's why "change" and looking at "files" is also needed)

Chrome:
- When the file selection does NOT change, and the selector is closed (with Esc or Cancel):
  - If you didn't select any file, "cancel" triggers, "input.files" is empty.
  - If you select THE SAME FILE AS BEFORE "cancel" triggers, but "input.files" is NOT empty.
- If you had a file selected before, reopen the selector and close it without changes (with Esc or Cancel):
  - "change" triggers, and "input.files" is emptied.
Firefox:
- The one closest to what one would expect ("cancel" when the dialog is closed, "change" when a file is selected)
  - Never clears "input.files" after selecting for the first time.
    - When selecting the same file, firefox fires "change" with the same "input.files" that was selected before (this is different than in Chrome, and will need browser detection to tell apart the browsers!)
Safari:
- Never clears "input.files" after selecting for the first time.
  - When selecting the same file as before, Safari fires "cancel", and input is NOT empty (same as Chrome)

I think the above covers all the possible use cases, but I'm not 100% sure. The idea is that we need to look at both the "change" and "cancel" events, and the "files" of the input to know if the user canceled or not.

andreisas06 · 2023-04-14T12:03:31Z

@raju8000 use Google Chrome Canary and Safari Technology Preview, and you will see the cancel event. I noticed it behaves different depending on the browser, just like @ditman said.

Here are some examples (for each example, the first action is at the bottom and the the most recent at the top):

Chrome Canary:

From what I can observe, after selecting the file and opening the dialog again to press cancel, it still sees it as a change event. Only after I open the dialog again and press cancel, it registers it as a cancel event.

Safari Technology Preview:

Even if i click cancel in the dialog after I selected an image, if shows me that I used a cancel event which is good, but even if I cancel, the browser still sees it as a selected file, although it is a canceled event with no files selected.
The first cancel event turned out as expected because I haven’t selected any images/files, but after i select a file/image and decide later on to cancel after I selected that file/image, it will keep the previously selected file name, even if u haven’t selected anything in the dialog and pressed cancel.

Firefox(stable not the Nightly version):

Same story sa Safari Technology Preview.

Edge Canary:

Behaves as expected, imo.

stuartmorgan-g · 2023-05-09T19:48:49Z

@raju8000 Are you still planning on updating this PR based on the discussion above?

ditman · 2023-05-12T20:13:40Z

Thanks @raju8000! I'll review this ASAP! In the meantime, please check the "repo_checks" failure, and try to fix whatever errors it's pointing out. They're usually easy to fix!

packages/file_selector/file_selector_web/lib/src/dom_helper.dart

packages/file_selector/file_selector_web/CHANGELOG.md

ditman · 2023-07-05T23:01:44Z

cancel seems to be enough in this case because the package never reuses the same input tag, so there's never a case of "the user has selected the same files as before" or any of the corner cases described above.

ditman · 2023-07-06T02:13:37Z

I'll try to get this fixed and landed tomorrow, and have a similar fix applied to image_picker.

ditman

I think this is ready to go! (PTAL @stuartmorgan)

…ncel" in system UI file picker #121328

stuartmorgan-g

LGTM

…ter/packages#3683)

flutter/packages@2508714...aa1eace 2023-07-13 ian@hixie.ch [rfw] Restore RFW to 100% coverage (flutter/packages#4355) 2023-07-12 65381000+raju8000@users.noreply.github.com [file_selector_web] Listens to file input cancel event. (flutter/packages#3683) 2023-07-12 sam.rawlins@gmail.com [cross_file] Correct sorting of import starting with dot-slash (flutter/packages#4449) 2023-07-12 ian@hixie.ch [metrics_center] Remove Equatable dependency (flutter/packages#4444) 2023-07-12 engine-flutter-autoroll@skia.org Roll Flutter from 3ec96a8 to 544d30d (66 revisions) (flutter/packages#4448) 2023-07-12 stuartmorgan@google.com [ci] Move snippet checks to LUCI (flutter/packages#4446) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

## Changes This PR listens to the `cancel` event from the `input type=file` used by the web implementation of the image_picker plugin, so apps don't end up endlessly awaiting for a file that will never come **in modern browsers** (Chrome 113, Safari 16.4, or newer). _Same API as #3683 Additionally, this PR: * Removes all code and tests mentioning `PickedFile`. (Deprecated years ago, and unused since #4285) **(Breaking change)** * Updates README to mention `XFile` which is the current return type of the package. * Updates the dependency on `image_picker_platform_interface` to `^2.9.0`. * Implements all non-deprecated methods from the interface, and makes deprecated methods use the fresh ones. * Updates tests. ### Issues * Fixes flutter/flutter#92176 ### Testing * Added integration testing coverage for the 'cancel' event. * Tested manually in Chrome with the example app running on web.

raju-muliyashiya requested review from ditman and stuartmorgan-g as code owners April 11, 2023 08:28

github-actions bot added p: file_selector platform-web labels Apr 11, 2023

ditman requested changes Apr 12, 2023

View reviewed changes

iapicca mentioned this pull request May 16, 2023

[image_picker][web] cancel cause picker to never return flutter/flutter#92176

Closed

stuartmorgan-g requested a review from ditman May 30, 2023 20:05

ditman reviewed Jun 1, 2023

View reviewed changes

packages/file_selector/file_selector_web/lib/src/dom_helper.dart Show resolved Hide resolved

stuartmorgan-g requested a review from ditman June 29, 2023 18:47

stuartmorgan-g reviewed Jun 29, 2023

View reviewed changes

packages/file_selector/file_selector_web/CHANGELOG.md Outdated Show resolved Hide resolved

ditman force-pushed the fix_file_selector_web_returns_null_on_presses_cancel branch from c0b5ec1 to bbf0c9e Compare July 12, 2023 00:58

ditman approved these changes Jul 12, 2023

View reviewed changes

ditman requested a review from stuartmorgan-g July 12, 2023 01:08

ditman self-assigned this Jul 12, 2023

ditman mentioned this pull request Jul 12, 2023

Api.HTMLInputElement.cancel_event - Wrong availability data? mdn/browser-compat-data#20342

Closed

ditman changed the title ~~[FileSelector][web] Fix-openFiles never returns when user presses "cancel" in system UI file picker~~ [file_selector_web] Listens to file input cancel event. Jul 12, 2023

raju-muliyashiya and others added 6 commits July 12, 2023 13:21

Fix-[FileSelector][web] openFiles never returns when user presses "ca…

de10ce6

…ncel" in system UI file picker #121328

version bump 0.9.0+5

eb833ef

changes as per requested

eeafb5c

removed print statement

3f9ce2e

conflict resolved

ce4bde4

Add note about showPicker API.

c8101e6

ditman added 6 commits July 12, 2023 13:21

openFile now returns null when no files are selected, following the API.

9ebad61

Test simulating a cancel event.

7e83a4c

Test return null behavior.

1a24e81

Update CHANGELOG

41db1ab

Add documentation to README

1ad6926

Add TestOn chrome to not confuse tooling on this web-only package.

fde3f95

ditman force-pushed the fix_file_selector_web_returns_null_on_presses_cancel branch from e00b088 to fde3f95 Compare July 12, 2023 20:22

Add vm flag to pathified unit test run

5df1943

stuartmorgan-g approved these changes Jul 12, 2023

View reviewed changes

stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 12, 2023

auto-submit bot merged commit 2a508cb into flutter:main Jul 12, 2023

This was referenced Jul 13, 2023

[image_picker_web] Listens to file input cancel event. #4453

Merged

[FileSelector][web] openFiles never returns when user presses "cancel" in system UI file picker flutter/flutter#121328

Closed

engine-flutter-autoroll mentioned this pull request Jul 13, 2023

Roll Packages from 250871431bed to aa1eace00188 (6 revisions) flutter/flutter#130495

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 13, 2023

2a508cb [file_selector_web] Listens to file input cancel event. (flut…

71f5ce1

…ter/packages#3683)

reidbaker mentioned this pull request Jul 14, 2023

[tooling] mark infra steps as infra steps in yaml files #4473

Merged

11 tasks

reidbaker mentioned this pull request Jul 26, 2023

[Tooling] Add google owned cache for dependencies as an option in ci #4567

Merged

11 tasks

reidbaker mentioned this pull request Aug 4, 2023

[All] Expand artifact hub to all plugins #4645

Merged

11 tasks

loic-hamdi mentioned this pull request Sep 29, 2023

Doesn't seem to work flutter/flutter#135733

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[file_selector_web] Listens to file input cancel event. #3683

[file_selector_web] Listens to file input cancel event. #3683

Uh oh!

raju-muliyashiya commented Apr 11, 2023

Uh oh!

google-cla bot commented Apr 11, 2023

Uh oh!

ditman left a comment

Uh oh!

ditman Apr 12, 2023

Uh oh!

raju-muliyashiya commented Apr 12, 2023 •

edited

Loading

Uh oh!

ditman commented Apr 14, 2023 •

edited

Loading

Uh oh!

andreisas06 commented Apr 14, 2023 •

edited

Loading

Uh oh!

stuartmorgan-g commented May 9, 2023

Uh oh!

ditman commented May 12, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

ditman commented Jul 5, 2023

Uh oh!

ditman commented Jul 6, 2023

Uh oh!

ditman left a comment •

edited

Loading

Uh oh!

stuartmorgan-g left a comment

Uh oh!

Uh oh!

[file_selector_web] Listens to file input cancel event. #3683

[file_selector_web] Listens to file input cancel event. #3683

Uh oh!

Conversation

raju-muliyashiya commented Apr 11, 2023

Pre-launch Checklist

Uh oh!

google-cla bot commented Apr 11, 2023

Uh oh!

ditman left a comment

Choose a reason for hiding this comment

Uh oh!

ditman Apr 12, 2023

Choose a reason for hiding this comment

Uh oh!

raju-muliyashiya commented Apr 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ditman commented Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andreisas06 commented Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stuartmorgan-g commented May 9, 2023

Uh oh!

ditman commented May 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ditman commented Jul 5, 2023

Uh oh!

ditman commented Jul 6, 2023

Uh oh!

ditman left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

raju-muliyashiya commented Apr 12, 2023 •

edited

Loading

ditman commented Apr 14, 2023 •

edited

Loading

andreisas06 commented Apr 14, 2023 •

edited

Loading

ditman commented May 12, 2023 •

edited

Loading

ditman left a comment •

edited

Loading