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

Rejection behavior for legacy branch #41

Closed
pandaxtc opened this issue Mar 22, 2021 · 21 comments
Closed

Rejection behavior for legacy branch #41

pandaxtc opened this issue Mar 22, 2021 · 21 comments

Comments

@pandaxtc
Copy link

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
Copy link
Member

tomayac commented Mar 22, 2021

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:

Screen Shot 2021-03-22 at 11 56 58

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);

@tomayac tomayac closed this as completed Mar 22, 2021
@pandaxtc
Copy link
Author

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

@tomayac
Copy link
Member

tomayac commented Mar 23, 2021

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?

@tomayac tomayac reopened this Mar 23, 2021
@pandaxtc
Copy link
Author

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
Copy link
Member

tomayac commented Mar 25, 2021

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
Copy link

KoKuToru commented Mar 25, 2021

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:
image
image

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

@tomayac
Copy link
Member

tomayac commented Mar 25, 2021

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
Copy link

KoKuToru commented Mar 26, 2021

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
Copy link

KoKuToru commented Mar 26, 2021

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
Copy link
Member

tomayac commented Mar 26, 2021

The latest version 0.16.0 rejects, too, but without the focus hack. See specifically

// ToDo: Remove this workaround once
// https://github.com/whatwg/html/issues/6376 is specified and supported.
const rejectOnPageInteraction = () => {
window.removeEventListener('pointermove', rejectOnPageInteraction);
window.removeEventListener('pointerdown', rejectOnPageInteraction);
window.removeEventListener('keydown', rejectOnPageInteraction);
reject(new DOMException('The user aborted a request.', 'AbortError'));
};
window.addEventListener('pointermove', rejectOnPageInteraction);
window.addEventListener('pointerdown', rejectOnPageInteraction);
window.addEventListener('keydown', rejectOnPageInteraction);
input.addEventListener('change', () => {
window.removeEventListener('pointermove', rejectOnPageInteraction);
window.removeEventListener('pointerdown', rejectOnPageInteraction);
window.removeEventListener('keydown', rejectOnPageInteraction);
.

@KoKuToru
Copy link

KoKuToru commented Mar 26, 2021

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
Copy link

KoKuToru commented Mar 26, 2021

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

@tomayac
Copy link
Member

tomayac commented Mar 26, 2021

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
Copy link
Contributor

jmrog commented Apr 29, 2021

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
Copy link

KoKuToru commented Apr 30, 2021 via email

@tomayac
Copy link
Member

tomayac commented Apr 30, 2021

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?

@tomayac tomayac reopened this Apr 30, 2021
@tomayac tomayac changed the title Example site doesn't seem to work in Firefox Rejection behavior for legacy branch Apr 30, 2021
@jmrog
Copy link
Contributor

jmrog commented Apr 30, 2021

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
Copy link
Contributor

jmrog commented May 3, 2021

@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
Copy link
Member

tomayac commented May 5, 2021

Fixed via #45.

@tomayac tomayac closed this as completed May 5, 2021
@tomayac
Copy link
Member

tomayac commented May 5, 2021

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
Copy link
Contributor

jmrog commented May 5, 2021

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants