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

AbortError on opening files on legacy browsers #53

Closed
dwelle opened this issue Jun 17, 2021 · 13 comments
Closed

AbortError on opening files on legacy browsers #53

dwelle opened this issue Jun 17, 2021 · 13 comments

Comments

@dwelle
Copy link
Contributor

dwelle commented Jun 17, 2021

It seems the #36 isn't fully fixed as we're still getting reports on Excalidraw. Can't reproduce on my system.

One confirmed case: excalidraw/excalidraw#3740 (comment), and one probable case excalidraw/excalidraw#3513. Both Firefox.

@tomayac
Copy link
Member

tomayac commented Jun 17, 2021

I just downloaded Firefox 78.11.0esr (64-bit) on macOS 12.0 Beta (21A5248p) and ran through the demo. All features worked, but for directoryOpen() (not used by Excalidraw) it seems like there is some filtering in place for certain directories like Pictures. It works fine with subfolders, though. I have a hard time figuring out where fileOpen() could fail. Maybe at this point it's worth opening a bug for Firefox directly?

@dwelle
Copy link
Contributor Author

dwelle commented Jun 17, 2021

Yeah, I cannot reproduce either but that doesn't help us. Before I'd open a Firefox issue I'll want to take some time to review the hack we're using to polyfill the rejection, and whether it's reasonable to expect browsers to behave deterministically in this case.

@tomayac
Copy link
Member

tomayac commented Jun 18, 2021

I have created a reduced test case that you can try and step through in Firefox. For all the Firefox instances I could test this with, it worked correctly (the screenshots below are from Firefox 89.0.1 (64-bit) on macOS 12.0 Beta (21A5248p)):

  • Pressing Cancel:
    Screen Shot 2021-06-18 at 09 37 52
  • Selecting file(s):
    Screen Shot 2021-06-18 at 09 37 36

@teramawi
Copy link

I'm the reporter of the linked excalidraw issue excalidraw/excalidraw#3740 and I am able to reproduce the AbortError problem using your reduced test case in 78.11.0esr.

The problem Is, that selecting a file using double click leads to a pointer event being triggered before the change event of the file dialog, therefore aborting the selection.

I was able to watch this by enabling event logging in the debugger for "Control -> change" and "Pointer -> Move". There were no other events interfering:

pointermove { target: html, buttons: 1, clientX: 717, clientY: 72, layerX: 717, layerY: 72 }      script.js:17:35
change { target: input, isTrusted: true, srcElement: input, currentTarget: input, eventPhase: 2, bubbles: true, cancelable: false, returnValue: true, defaultPrevented: false, composed: false, … }         script.js:24:6

The selection does work if I submit my selection using the "open" button rather than doing a double click. I was also able to reproduce this behaviour in excalidraw as well.

@tomayac
Copy link
Member

tomayac commented Jun 18, 2021

I tried doubleclick file selection and it just worked fine for me. No matter if the file open dialog is over the Firefox window, just overlapping with it, or completely distant from it. It worked in all cases. What platform are you on?

@dwelle
Copy link
Contributor Author

dwelle commented Jun 18, 2021

I can intermittently reproduce in Firefox when using the double-click on the file.

And another interesting thing. Trying to reproduce in Brave I was getting a different bug: the AbortError was sometimes shown right after the click, before the file dialog was opened. And since the promise was rejected beforehand, the ensuing file open didn't resolve and stayed rejected. EDIT: tried on excalidraw.com in Brave and indeed I can reproduce there as well (took ~10 tries).

So Chromium is affected too, except in a different manner.

@teramawi
Copy link

I'm on Windows 10 64bit 20H2 (Build 19042.804) with Firefox 78.11.0esr (32-Bit) and I can reproduce the issue reliably.

I can not reproduce the issue using Google Chrome Version 91.0.4472.101 (Offizieller Build) (32-Bit) using the same platform.

Let me know if i can help you somehow pinpointing the problem.

@dwelle
Copy link
Contributor Author

dwelle commented Jun 18, 2021

I'm getting the AbortError on FF even with single click (not sure if more or less often than double click). Leads me to believe the Chromium and FF bugs may be the same, except FF doesn't render the error in time before the dialog opens.

@dwelle
Copy link
Contributor Author

dwelle commented Jun 18, 2021

How does the polyfill even work ever?

window.addEventListener('pointermove', rejectionHandler);
window.addEventListener('pointerdown', rejectionHandler);
window.addEventListener('keydown', rejectionHandler);
}
input.addEventListener('change', () => {
cleanupListenersAndMaybeReject();
resolve(input.multiple ? Array.from(input.files) : input.files[0]);
});
input.click();

var handler = () => console.log('click');
document.body.addEventListener('click', handler);
document.body.click();
document.body.removeEventListener('click', handler);

The above catches the programmatic click event because the listener is bound synchronously.

Maybe the browser-fs-access hack depends on the file dialog blocking the thread? But how is the ensuing change event fired before the click event.

Actually, from @teramawi's comment #53 (comment) it seems the click event doesn't come to play, and it's the pointermove event. Though it's a comparable problem.


EDIT: ok, checking the source, we're not listening on click, which is why the click() does not trigger the rejection.

@dwelle
Copy link
Contributor Author

dwelle commented Jun 18, 2021

I can confirm the Brave bug relates to pointermove as well. And it's 100% reproducible by slide-clicking (moving your mouse a bit when clicking) on the button:

brave_fs_access_bug

@seanaye
Copy link
Contributor

seanaye commented Jun 29, 2021

I get this issue 100% of the time on Brave. Even when not moving the mouse at all and triggering the event with tab+enter, although unlike the screencap above it errors only after I select a file and click open.

Brave: Version 1.25.73 Chromium: 91.0.4472.106 (Official Build) (arm64)
MacOS: 11.1

@RomLAURENT
Copy link

RomLAURENT commented Jun 29, 2021

Hi,

Same issue here with FF.

Here is my custom legacy setup to circumvent this matter :

export const setupLegacyCleanupAndRejection = rejectionHandler => {
   const timeoutHandle = setTimeout(() => {
       window.addEventListener("pointermove", rejectionHandler);
       window.addEventListener("pointerdown", rejectionHandler);
       window.addEventListener("keydown", rejectionHandler);
   }, 500);

   return rej => {
       clearTimeout(timeoutHandle);
       window.removeEventListener("pointermove", rejectionHandler);
       window.removeEventListener("pointerdown", rejectionHandler);
       window.removeEventListener("keydown", rejectionHandler);
       if (rej) {
           rej(new DOMException("The user aborted a request.", "AbortError"));
       }
   };
};

Basically I'm just delaying the start of listening for "abort" events.

@tomayac tomayac closed this as completed in 78e2d3f Jul 1, 2021
@tomayac
Copy link
Member

tomayac commented Jul 1, 2021

I have now removed the rejection handling for legacy browsers entirely since it caused more harm than good. People who need the throwing behavior can configure their own handler, as per the option introduced in #45.

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

5 participants