-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Ff111 More OPFS related updaes #19056
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When doing this kind of check, I highly recommend searching for the pref in searchfox inside dom/webidl: https://searchfox.org/mozilla-central/search?q=dom.fs.enabled&path=dom%2Fwebidl&case=true®exp=false
Per the result DataTransferItem and Window members are not covered (as they are not part of https://fs.spec.whatwg.org/)
And it seems spec_url
s are missing too, can you add them while you are at it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Submitted a bit early.)
641ccc0
to
96666bb
Compare
Hi @saschanaz Thanks so much - good tip on the searchfox.
The APIs now covered appear to have a spec-urls unless they are non-standard - such as I think we're good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! One comment:
"move": { | ||
"__compat": { | ||
"mdn_url": "https://developer.mozilla.org/docs/Web/API/FileSystemHandle/move", | ||
"support": { | ||
"chrome": { | ||
"version_added": false | ||
}, | ||
"chrome_android": "mirror", | ||
"edge": "mirror", | ||
"firefox": { | ||
"version_added": "111" | ||
}, | ||
"firefox_android": "mirror", | ||
"ie": { | ||
"version_added": false | ||
}, | ||
"oculus": "mirror", | ||
"opera": "mirror", | ||
"opera_android": "mirror", | ||
"safari": { | ||
"version_added": false | ||
}, | ||
"safari_ios": "mirror", | ||
"samsunginternet_android": "mirror", | ||
"webview_android": "mirror" | ||
}, | ||
"status": { | ||
"experimental": true, | ||
"standard_track": false, | ||
"deprecated": false | ||
} | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@queengooborg FYI support for this was added in FF111 in https://bugzilla.mozilla.org/show_bug.cgi?id=1811001. However appears to be explicitly non-standard at this point as it is in a "PR" state - see the big banner when you access the URL https://whatpr.org/fs/10.html#api-filesystemhandle
Upshot, I've added it because that makes the compatibility story easier to track later, but there is no spec URL and it is marked as experimental and non-standard.
@saschanaz Added with above caveats ^^^. I don't plan to add documentation for this right now; any idea on when this PR is likely to go in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea, maybe @janvarga knows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saschanaz - cool - not urgent - just useful so we get a ping when it becomes standard.
BTW, thanks for all your help and advice on this. I will do better next time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem, you're welcome 👍
@queengooborg I think this can merge now. |
FF111 added support for OPFS in https://bugzilla.mozilla.org/show_bug.cgi?id=1811001. I added the main API in #18907 but it never occurred to me that the rest of the file system around this was not supported. So as per https://bugzilla.mozilla.org/show_bug.cgi?id=1811001#c11 this updates the interfaces in the File System Access API to indicate they are also supported.
Note, WritableStream is already marked as supported from FF100
@saschanaz Can you confirm this issue delivers full support for the API / that this is the right set of things to mark as supported in FF111?