Rejection behavior for legacy branch

pandaxtc commented 3 years ago

The example site at https://browser-fs-access.glitch.me/ doesn't seem to work on Firefox v86.0.1. The pickers all appear properly, but after selecting files or a directory, nothing happens. Is this intended?

tomayac commented 3 years ago

Note that the directory listing by default is not recursive, that is, if your directory only contains other folders, nothing will be shown. If you, however, have files in it, it will work:

You can also enable recursive mode, but careful since this can be a heavy operation depending on how deeply your folders are nested:

const options = {
  // Set to `true` to recursively open files in all subdirectories,
  // defaults to `false`.
  recursive: true,
};

const blobs = await directoryOpen(options);

pandaxtc commented 3 years ago

@tomayac It doesn't work even if I select directories that contain an image file.

tomayac commented 3 years ago

I tested this on Firefox 86.0.1 (64-bit) on macOS 11.3 Beta (20E5210c). Can you let me know your platform? Do all Open * buttons not work for you, or just some? Can you paste the output of the ls and pwd commands in the directory in question here?

pandaxtc commented 3 years ago

Sure thing!

Browser: Firefox v87.0 (64-bit)
Platform: Windows 10 Education 20H2 Build 19042.804
Only the 'Open Directory' button doesn't work, the others seem to work (i.e. the selected images display on the site)

Output of dir:


Directory: D:\waylon\Pictures\some_images

Mode LastWriteTime Length Name

-a---- 12/2/2017 19:51 653802 1.jpg -a---- 12/2/2017 19:51 813898 2.jpg -a---- 12/2/2017 19:51 652049 3.jpg

* Output of `pwd`:

Path

D:\waylon\Pictures\some_images

tomayac commented 3 years ago

Thanks for the details. It turns out the problem was a code race with the previous onblur hack where sometimes (but not always) Firefox would reject the promise before even a selection was made. There is no proper solution yet, but see the commit for the current workaround.

KoKuToru commented 3 years ago

It works now with selecting the file and clicking on open (in the file select dialog) but.. It doesn't work with double clicking the file (in the file open dialog) on windows. It does work on linux.

In my project I get the following error:

On https://browser-fs-access.glitch.me/ there is no message in the console.. but it doesn't open the file.

tomayac commented 3 years ago

It works now with selecting the file and clicking on open (in the file select dialog) but.. It doesn't work with double clicking the file (in the file open dialog) on windows. It does work on linux.

Could this be just Firefox specific? Could you test with this demo and let me know? It was working fine for me on Windows, but sometimes macOS required me to deselect and reselect the file before it would work.

In my project I get the following error:

Yes, this is to be expected, you need to catch the exception:

try {
  const file = await fileOpen();
} catch (err) {
  console.log(err.message);
}

On https://browser-fs-access.glitch.me/ there is no message in the console.. but it doesn't open the file.

I just tested this successfully on 86 and 87. Could you try again? Note that the demo filters on images.

KoKuToru commented 3 years ago

Could this be just Firefox specific? Could you test with this demo and let me know? It was working fine for me on Windows, but sometimes macOS required me to deselect and reselect the file before it would work.

EDIT: ~~Yeah, it's only on Firefox with Windows.. it is okay on Firefox with Linux.~~ See next comment..

I just tested this successfully on 86 and 87. Could you try again? Note that the demo filters on images.

On Firefox 87 Windows 10 it doesn't work with double click on file but works with file select + open. No Problems on Firefox 87 Linux (Archlinx with Gnome), double click and file select + open works.

Using the "Open Image File"-Button on https://browser-fs-access.glitch.me/

KoKuToru commented 3 years ago

Sorry.. little mistake..

Could you test with this demo and let me know?

I thought the demo linked to https://browser-fs-access.glitch.me/ ..

On the normal <input type="file"> it does select the file correctly on Firefox Windows. No difference between double click and file select + open.

Just as info, I am using now the old version of this library. The one without the "reject"-hack for firefox. Of course you wont get a "reject" if you close the file dialog.. but it works (if you don't need the reject)

tomayac commented 3 years ago

The latest version 0.16.0 rejects, too, but without the focus hack. See specifically https://github.com/GoogleChromeLabs/browser-fs-access/blob/c188acdcac8b0c9d27e1cb4a1dfed31e74f31744/src/legacy/file-open.mjs#L33-L49.

KoKuToru commented 3 years ago

double click file select in open file dialog on firefox windows:

^0.13.0: working but no reject on cancel
^0.14.0: not working but reject on cancel (and reject on double click) EDIT:
^0.15.0: working with reject on cancel
^0.16.0: same as ^0.14.0

KoKuToru commented 3 years ago

^0.15.0 does work with double click and reject on cancel on firefox windows..

tomayac commented 3 years ago

The behavior from 0.16.0 is the best-possible solution we can come up with given that there is no proper cancel event. It rejects upon the first page interaction upon canceling the dialog.

jmrog commented 3 years ago

The behavior from 0.16.0 is the best-possible solution we can come up with given that there is no proper cancel event. It rejects upon the first page interaction upon canceling the dialog.

@tomayac There is a hiccup here. If I use this library in Chrome but at a URL that uses http rather than https as the protocol (and am not on a localhost URL, etc.) -- in short, if I am using this library in Chrome in a context where Chrome falls back to the legacy API -- the new behavior (as of 0.16.0) causes the library to reject (assume the user canceled the dialog) far too early in a lot of cases. Seemingly, Chrome -- unlike Firefox -- fires a pointermove event right when the dialog opens. So, if I click a button to open the dialog, but my mouse is not already positioned in the spot where the dialog shows (e.g., the button was too far away), the library rejects immediately as a result of the pointermove event.

Should I make a new issue for the above?

KoKuToru commented 3 years ago

the reject-thing is pretty broken (the last time i tested it) and there is no clean solution for it (i think). having a reject when the user selects a file is in no way the best-case for me.

i am using an older version with the missing-reject, having no reject is the better-case. it's just something you need to know and probably leaks something in the background but still the better solution (for me).

tomayac commented 3 years ago

Answering to @KoKuToru and @jmrog's comments above:

@tomayac There is a hiccup here. If I use this library in Chrome but at a URL that uses http rather than https as the protocol (and am not on a localhost URL, etc.) -- in short, if I am using this library in Chrome in a context where Chrome falls back to the legacy API -- the new behavior (as of 0.16.0) causes the library to reject (assume the user canceled the dialog) far too early in a lot of cases. Seemingly, Chrome -- unlike Firefox -- fires a pointermove event right when the dialog opens. So, if I click a button to open the dialog, but my mouse is not already positioned in the spot where the dialog shows (e.g., the button was too far away), the library rejects immediately as a result of the pointermove event.

Oh no. This is unfortunate. Is enforcing your site to load over HTTPS a workaround?

if (!isSecureContext) {
  location.protocol = 'https:';
}

Note that the library also exposes a supported flag that tells you if it can use the File System Access API or not.

Should I make a new issue for the above?

I rename the present Issue and re-open it.

the reject-thing is pretty broken (the last time i tested it) and there is no clean solution for it (i think). having a reject when the user selects a file is in no way the best-case for me.

I more and more am convinced that there is indeed no clean solution that works universally.

i am using an older version with the missing-reject, having no reject is the better-case. it's just something you need to know and probably leaks something in the background but still the better solution (for me).

Would making the rejection behavior be configurable be a workable option for either of you?

jmrog commented 3 years ago

Thank you both for the replies.

Oh no. This is unfortunate. Is enforcing your site to load over HTTPS a workaround?

Not at present, unfortunately. It's a longer term goal, but we're not there yet.

Would making the rejection behavior be configurable be a workable option for either of you?

For us, this would be perfect. We're currently inclined to think that we should reject very "lazily" rather than "greedily" -- e.g., not reject at all (which would leak, as @KoKuToru notes, but it would be very tiny), or perhaps reject only after a very long amount of time and/or a number of events are recorded. Our app needs to act when a file is actually opened, but it doesn't really need to do anything in the event of a cancel.

jmrog commented 3 years ago

@tomayac Unsure whether it conforms with what you were thinking, but the PR above is a rough thing I threw together to allow configurably handling cancel/rejection in the legacy branch. It would work well for us. Any thoughts there?

Here is how you could use the API in the PR for custom behavior:

const file = await fileOpen({
  setupLegacyCleanupAndRejection: (rejectionHandler) => {
    // Always reject/cancel in 10 seconds; probably not something you'd really want to do
    const timeoutId = setTimeout(rejectionHandler, 10000);
    return (reject) => {
      // Clear the timeout on both success and failure; not strictly necessary in failure case.
      clearTimeout(timeoutId);
      // And if this is a real cancel/rejection (i.e., rejectionHandler was called), reject the Promise.
      if (reject) {
        reject('My error message here.');
      }
    };
  },
});

tomayac commented 3 years ago

Fixed via https://github.com/GoogleChromeLabs/browser-fs-access/pull/45.

tomayac commented 3 years ago

I have just released v0.17.0 that includes the new configurable behavior. (Note that I have also added this to the directoryOpen() function and added a blurb about this to the README.) Please give it a spin! Thanks everyone for your input on this!

jmrog commented 3 years ago

Thanks, @tomayac! We'll be upgrading to the latest version today.

@KoKuToru This should also allow you to upgrade. If you still want no rejection ever with the legacy API, you can just make the new setupLegacyCleanupAndRejection return a noop:

const file = await fileOpen({
  setupLegacyCleanupAndRejection: () => () => {},
});

But of course you now also have the option of rejecting/cancelling very late (e.g., on timeouts, clicks, focus change, or route changes).

I'm interested in how/whether it works for you!

GoogleChromeLabs / browser-fs-access

Rejection behavior for legacy branch #41

Path