Add fallback for File System Access API#4882
Add fallback for File System Access API#4882Manwe-777 wants to merge 1 commit intobetaflight:masterfrom
Conversation
WalkthroughAdds fallback support for browsers lacking File System Access API. Introduces three new helper methods to enable legacy file picking via hidden file inputs and file saving via blob downloads. Modifies existing methods to detect API availability and route through appropriate code paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
🎉 Do you want to test this code? 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/js/FileSystem.js (1)
19-49: Fallback open picker relies oncancelevent, which is not available in older browsers.Line 42 listens for the
cancelevent on<input type="file">, which is supported in Chrome 113+ (2023), Firefox 91+ (2021), and Safari 16.4+ (2021). In older browsers without this event (primarily pre-2021 releases), the Promise may never resolve if the user dismisses the picker. While this fallback only activates when File System Access API is unavailable (limiting the affected audience), adding a safeguard for browsers that support neither API would improve reliability on older or low-resource devices.The proposed fix (settling the Promise on focus + cleanup) is a solid improvement for edge cases, though modern users are unlikely to encounter this issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/js/FileSystem.js` around lines 19 - 49, The _pickOpenFileFallback promise can hang because some browsers don't emit the input "cancel" event; modify _pickOpenFileFallback to also detect picker dismissal via a focus-based fallback: after appending and clicking the hidden input, add a one-time window focus (or input.blur) handler and/or a short timeout that, when triggered, removes the input, unregisters all listeners (change and the fallback), and resolves the promise with null if no file was selected; ensure the existing change handler still resolves with this._createLegacyFile(file) and that all listeners and the DOM node are always cleaned up exactly once to avoid leaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/js/FileSystem.js`:
- Around line 54-63: The immediate call to URL.revokeObjectURL(url) after
a.click() can race with the browser starting the download; change this to defer
revocation (e.g., wrap URL.revokeObjectURL(url) in a short setTimeout such as
100ms) so the browser has time to begin consuming the blob URL; update the block
that creates the blob/url/anchor (variables blob, url, a, and suggestedName) to
revoke the URL asynchronously after the click and still remove the anchor and
resolve(true) as before.
---
Nitpick comments:
In `@src/js/FileSystem.js`:
- Around line 19-49: The _pickOpenFileFallback promise can hang because some
browsers don't emit the input "cancel" event; modify _pickOpenFileFallback to
also detect picker dismissal via a focus-based fallback: after appending and
clicking the hidden input, add a one-time window focus (or input.blur) handler
and/or a short timeout that, when triggered, removes the input, unregisters all
listeners (change and the fallback), and resolves the promise with null if no
file was selected; ensure the existing change handler still resolves with
this._createLegacyFile(file) and that all listeners and the DOM node are always
cleaned up exactly once to avoid leaks.
| const blob = new Blob([contents], { type: "application/octet-stream" }); | ||
| const url = URL.createObjectURL(blob); | ||
| const a = document.createElement("a"); | ||
| a.href = url; | ||
| a.download = suggestedName; | ||
| document.body.appendChild(a); | ||
| a.click(); | ||
| document.body.removeChild(a); | ||
| URL.revokeObjectURL(url); | ||
| resolve(true); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
URL.revokeObjectURL timing issue race condition download Blob
💡 Result:
URL.revokeObjectURL() is synchronous, but a download/navigation started from a blob URL is asynchronous, so revoking immediately after a.click() (or window.location = blobUrl) can invalidate the URL before the browser has finished reading it, causing flaky “race condition” failures (often Safari/Chrome-specific). (stackoverflow.com)
What to do in practice
- Don’t revoke immediately. There’s no standard “download finished” event for
<a download>/ blob URL downloads, so the common workaround is to delay revocation. (stackoverflow.com) - Use a small delay (often enough to let the browser start consuming the blob URL); some libraries use much longer delays for safety (e.g., FileSaver.js is commonly cited as using ~40s). (stackoverflow.com)
const url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = filename;
document.body.appendChild(a);
a.click();
a.remove();
// Delay cleanup to avoid revoking before the browser consumes the blob URL
setTimeout(() => URL.revokeObjectURL(url), 1000); // tune (100ms–a few seconds)Notes / gotchas
- Some browsers are picky about the anchor: append it to the DOM before
click(). (xjavascript.com) - MDN’s guidance is simply: revoke when you’re finished using the object URL—but avoid freeing too early. (developer.mozilla.org)
Citations:
- 1: https://stackoverflow.com/questions/63109880/blob-createobjecturl-download-not-working-in-chrome?utm_source=openai
- 2: https://stackoverflow.com/questions/63109880/blob-createobjecturl-download-not-working-in-chrome?utm_source=openai
- 3: https://stackoverflow.com/questions/63109880/blob-createobjecturl-download-not-working-in-chrome?utm_source=openai
- 4: https://www.xjavascript.com/blog/failed-to-load-resource-frame-load-interrupted-in-safari-browser-while-downloading-multiple-files/?utm_source=openai
- 5: https://developer.mozilla.org/en-US/docs/Web/API/URL/revokeObjectURL_static?utm_source=openai
🌐 Web query:
best practices Blob download JavaScript a.click() URL.createObjectURL cleanup
💡 Result:
Best practices for Blob downloads (a.click() + URL.createObjectURL) and cleanup
-
Create a short‑lived object URL and always revoke it
URL.createObjectURL(blob)allocates a Blob URL; keep it only as long as needed. Revoking it frees the underlying mapping/resources. [1][2]- Blob URLs are also auto‑cleaned up when the document is unloaded, but for long‑lived apps you should revoke explicitly. Chromium notes the URL can keep blob data around until revoked (and blob dereferenced). [5]
-
Trigger download via an
<a download>and a user gesture- Use an anchor with the
downloadattribute to force “download the hyperlink” behavior instead of normal navigation. [4] - In practice, initiate the download from a real user action (e.g., inside a click handler) to avoid popup/download blocking (browser policy; behavior varies).
- Use an anchor with the
-
Don’t revoke “too early”; revoke after the download has started
- If you revoke immediately, some browsers may not have begun consuming the Blob URL yet. MDN’s resource-management guidance frames revocation as “when the download has successfully started.” [3]
- Common pattern:
setTimeout(..., 0–1000)(orrequestAnimationFrame) beforerevokeObjectURL, to give the navigation/download pipeline time.
-
Remove DOM references and dereference big objects
- Remove the temporary
<a>element and clear references to largeBlobs once done to allow GC. Chromium explicitly warns that keeping Blob references (or Blob URLs) can retain memory. [5]
- Remove the temporary
-
Be aware of Safari/iOS Blob-URL download issues
- There have been reports (including iOS 18.x timeframe) of Safari download problems involving Blob URLs not ending up in the Files app / downloads location. If you must support iOS Safari, test and consider a server-sent
Content-Disposition: attachmentdownload as a fallback. [6]
- There have been reports (including iOS 18.x timeframe) of Safari download problems involving Blob URLs not ending up in the Files app / downloads location. If you must support iOS Safari, test and consider a server-sent
Reference implementation (safe cleanup)
async function downloadBlob(blob, filename) {
const url = URL.createObjectURL(blob); // [1]
const a = document.createElement("a");
a.href = url;
a.download = filename; // [4]
a.style.display = "none";
document.body.appendChild(a);
a.click();
// Let the download start, then clean up. [3]
setTimeout(() => {
URL.revokeObjectURL(url); // [2]
a.remove();
}, 250);
}Sources
[1] MDN: URL.createObjectURL() — https://developer.mozilla.org/en-US/docs/Web/API/URL/createObjectURL_static
[2] MDN: URL.revokeObjectURL() — https://developer.mozilla.org/en-US/docs/Web/API/URL/revokeObjectURL_static
[3] MDN: JavaScript resource management (example revoking after click / avoiding leaks) — https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Resource_management
[4] WHATWG HTML Standard (download attribute triggers “download the hyperlink”) — https://html.spec.whatwg.org/dev/links.html
[5] Chromium source docs (Blob pitfalls: leaking blob references / URLs retain data until revoked) — https://chromium.googlesource.com/chromium/src/+/0e2b5d09/storage/browser/blob
[6] Apple Community thread (reports of blob URL download issues on iOS 18.2+) — https://discussions.apple.com/thread/256063732
🌐 Web query:
Firefox URL.revokeObjectURL download immediately revoke issues
💡 Result:
In Firefox, revoking a Blob URL immediately after triggering a download can race with the actual download initiation, so the download may fail, stall, or open a blank/new tab depending on how it’s triggered. The safe pattern is:
- Create the Blob URL and assign it to an
<a download> - Trigger the click from a user gesture (e.g., inside a real button click handler)
- Delay
URL.revokeObjectURL()(there’s no reliable “download finished” event)
function downloadBlob(blob, filename) {
const url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = filename;
// Helps with browsers that ignore clicks on detached anchors
document.body.appendChild(a);
a.click();
a.remove();
// Don’t revoke immediately; allow navigation/download to start
setTimeout(() => URL.revokeObjectURL(url), 1000);
}Why:
URL.revokeObjectURL()is defined as releasing the object URL “when you’ve finished using it” (revoking too early can invalidate the URL before Firefox has fully consumed it). [1]- In practice, delaying revocation (even a short timeout) is a common cross-browser workaround when immediate revocation causes aborted/failed downloads. [2]
- On some Firefox variants (notably iOS), scripted
elem.click()+ Blob/download behavior has known limitations/bugs. [3]
If you still see failures in Firefox: ensure the click is user-initiated (not async after await/XHR completion without user interaction), and keep the Blob URL alive longer (e.g., several seconds) before revoking.
Sources: [1] MDN URL.revokeObjectURL() docs, [2] discussion of delaying revocation to avoid aborts, [3] Firefox iOS Blob download issue thread.
Defer URL.revokeObjectURL() to avoid race condition with download start.
Revoking the blob URL immediately after a.click() is synchronous while the browser's download initiation is asynchronous, causing the download to fail or stall in some browsers (Safari, Chrome, Firefox). Delay revocation with a small timeout to allow the browser to begin consuming the URL.
🔧 Proposed fix
_pickSaveFileFallback(suggestedName, contents) {
return new Promise((resolve) => {
const blob = new Blob([contents], { type: "application/octet-stream" });
const url = URL.createObjectURL(blob);
const a = document.createElement("a");
a.href = url;
a.download = suggestedName;
+ a.style.display = "none";
document.body.appendChild(a);
a.click();
- document.body.removeChild(a);
- URL.revokeObjectURL(url);
- resolve(true);
+ setTimeout(() => {
+ URL.revokeObjectURL(url);
+ if (a.parentNode) {
+ a.parentNode.removeChild(a);
+ }
+ resolve(true);
+ }, 0);
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/js/FileSystem.js` around lines 54 - 63, The immediate call to
URL.revokeObjectURL(url) after a.click() can race with the browser starting the
download; change this to defer revocation (e.g., wrap URL.revokeObjectURL(url)
in a short setTimeout such as 100ms) so the browser has time to begin consuming
the blob URL; update the block that creates the blob/url/anchor (variables blob,
url, a, and suggestedName) to revoke the URL asynchronously after the click and
still remove the anchor and resolve(true) as before.
|
@Manwe-777 IMO this fallback is not needed - as browsers should be up to date due to security reasons ? |
Yeah I think you can close this one - it was required for me using Brave Browser (which is up to date with latest Chromium), I just found out it has security around the file system access I needed to bypass to get it working ! |
2 participants
Summary
Add fallback file picker using traditional
<input type=\"file\">for browsers that don't support the File System Access API (showOpenFilePicker).Problem
The "Load Firmware [Local]" button fails silently in browsers without File System Access API support (Firefox, some PWA contexts).
Solution
showOpenFilePicker/showSaveFilePickerare availableSummary by CodeRabbit