Add compat data for FormData Web API #1487
Conversation
teoli2003
added
the
data:api
Generated via https://github.com/dontcallmedom/webidl2mdnbcd from https://xhr.spec.whatwg.org/ Skipping FormData since its addition is pending in mdn#1487
Generated via https://github.com/dontcallmedom/webidl2mdnbcd from https://xhr.spec.whatwg.org/ Skipping FormData since its addition is pending in #1487
Generated via https://github.com/dontcallmedom/webidl2mdnbcd from https://xhr.spec.whatwg.org/ Skipping FormData since its addition is pending in mdn#1487
There was a problem hiding this comment.
This looks pretty good so far. I spotted some possible notes improvements for your consideration, but that was it. Thank you, @theCalibrius!
api/FormData.json
Outdated
| }, | ||
| "firefox": { | ||
| "version_added": "4", | ||
| "notes": "Prior to Gecko 7.0 (Firefox 7.0 / Thunderbird 7.0 / SeaMonkey 2.4), if you specified a <a href='https://developer.mozilla.org/docs/Web/API/Blob'><code>Blob</code></a> as the data to append to the object, the filename reported in the 'Content-Disposition' HTTP header was an empty string; this resulted in errors being reported by some servers. Starting from Gecko 7.0, the filename 'blob' is sent." |
There was a problem hiding this comment.
A few things here:
- For clarity, it helps to trim down notes to refer only to the browser that the note is attached to (e.g., Firefox, not Gecko or other browsers). So in this case, I'd rework this as something along the lines of "Before Firefox 7… Starting from Firefox 7, the filename 'blob' is sent."
- A minor thing, but the text formatting could be improved a little by putting
Content-Dispositionandblobin<code>tags instead of single quotes - Finally, the version numbers in notes should match the convention for that browser (e.g., no trailing zero for Firefox)
This comes up in a few places, so I'll 🚩 them too
api/FormData.json
Outdated
| "notes": "Prior to Gecko 7.0 (Firefox 7.0 / Thunderbird 7.0 / SeaMonkey 2.4), if you specified a <a href='https://developer.mozilla.org/docs/Web/API/Blob'><code>Blob</code></a> as the data to append to the object, the filename reported in the 'Content-Disposition' HTTP header was an empty string; this resulted in errors being reported by some servers. Starting from Gecko 7.0, the filename 'blob' is sent." | ||
| }, | ||
| "firefox_android": { | ||
| "version_added": "4" |
There was a problem hiding this comment.
The note for Firefox also applies to Firefox for Android, so copying the revised note here would be ideal.
api/FormData.json
Outdated
| "support": { | ||
| "webview_android": { | ||
| "version_added": "3", | ||
| "notes": "XHR in Android 4.0 sends empty content for FormData with blob." |
There was a problem hiding this comment.
It'd be nice to see <code>blob</code>, to match the previous note like this one
api/FormData.json
Outdated
| }, | ||
| "firefox": { | ||
| "version_added": "4", | ||
| "notes": "Prior to Gecko 7.0 (Firefox 7.0 / Thunderbird 7.0 / SeaMonkey 2.4), if you specified a <a href='https://developer.mozilla.org/docs/Web/API/Blob'><code>Blob</code></a> as the data to append to the object, the filename reported in the 'Content-Disposition' HTTP header was an empty string; this resulted in errors being reported by some servers. Starting in Gecko 7.0 the filename 'blob' is sent." |
There was a problem hiding this comment.
🚩 This note could be revised too (browser name, formatting version numbers)
api/FormData.json
Outdated
| }, | ||
| "firefox_android": { | ||
| "version_added": "4", | ||
| "notes": "Prior to Gecko 7.0 (Firefox 7.0 / Thunderbird 7.0 / SeaMonkey 2.4), if you specified a <a href='https://developer.mozilla.org/docs/Web/API/Blob'><code>Blob</code></a> as the data to append to the object, the filename reported in the 'Content-Disposition' HTTP header was an empty string; this resulted in errors being reported by some servers. Starting in Gecko 7.0 the filename 'blob' is sent." |
There was a problem hiding this comment.
🚩 This note could be revised too (browser name, formatting version numbers)
api/FormData.json
Outdated
| }, | ||
| "ie": { | ||
| "version_added": "10", | ||
| "notes": "With the'Include local directory pass when uploading files to a server' option enabled, IE will change the filename inside the <a href='https://developer.mozilla.org/docs/Web/API/Blob'><code>Blob</code></a> on the fly. To have direct control of the sent filename, the developer should send the filename as the third parameter value, i.e. <i>formData.append(name, value, filename)</i>." |
There was a problem hiding this comment.
A couple little quirks in the formatting here. It looks like there's a space missing before "Include" and the example call at the end might read a little nicer in <code> instead of <i>. Also, instead of single quotes around the option text, you can use double quotes with a backslash, like this \", or an HTML entity (like ")
api/FormData.json
Outdated
| "AppendWithFilename": { | ||
| "__compat": { | ||
| "mdn_url": "https://developer.mozilla.org/docs/Web/API/FormData/append", | ||
| "description": "append with filename", |
There was a problem hiding this comment.
Perhaps <code>append</code> with filename here?
api/FormData.json
Outdated
| "AppendWithFilename": { | ||
| "__compat": { | ||
| "mdn_url": "https://developer.mozilla.org/docs/Web/API/FormData/append", | ||
| "description": "append with filename", |
There was a problem hiding this comment.
Perhaps <code>append</code> with filename too?
|
Thanks Daniel. Changes made. |
There was a problem hiding this comment.
@theCalibrius Thanks for your patience while I got back to you on this. Also, I'm sorry, but I missed a couple of things in my earlier review. Hopefully these should be the last changes needed before we can merge. Thank you!
api/FormData.json
Outdated
| } | ||
| } | ||
| }, | ||
| "AppendWithFilename": { |
There was a problem hiding this comment.
This appears appears to be a duplicate of the FormData.append.AppendWithFilename data above. You can remove this one.
| } | ||
| } | ||
| }, | ||
| "WebWorkers": { |
There was a problem hiding this comment.
The WebWorkers subfeature doesn't need to be duplicated throughout—so you can leave in only this one top-level feature and remove the others (I got clarification on this separately, which is why I didn't mention it before)
|
Thanks again, Daniel. Requested changes have been committed. |
|
Thank you very much for your contribution, Brian! |
I missed a few things while reviewing the original PR mdn#1487: - descriptions for methods (not strictly required, but nice to have) - workers consistency
These entries were added separately at different times: - SupportForOf in mdn#1487 - @@iterator in mdn#8104 The data is the same (or missing) except for Edge and Safari. The @@iterator entry is based on mdn-bcd-collector results, but https://software.hixie.ch/utilities/js/live-dom-viewer/saved/9119 was used to confirm support of for...of loops in Edge 18 (but not 17) and in Safari 11.1 (but not 11). In other words, no data needed updating on the @@iterator entry.
These entries were added separately at different times: - SupportForOf in #1487 - @@iterator in #8104 The data is the same (or missing) except for Edge and Safari. The @@iterator entry is based on mdn-bcd-collector results, but https://software.hixie.ch/utilities/js/live-dom-viewer/saved/9119 was used to confirm support of for...of loops in Edge 18 (but not 17) and in Safari 11.1 (but not 11). In other words, no data needed updating on the @@iterator entry.
No description provided.