Summary: | Web Inspector: application/xml content not shown | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Olivier Blin <olivier.blin> | ||||||
Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, hi, inspector-bugzilla-changes, joepeck, loic.yhuel, mcatanzaro, webkit-bug-importer, zan | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Olivier Blin
2019-07-17 08:18:29 PDT
Created attachment 374294 [details]
Patch
Comment on attachment 374294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374294&action=review r=me, nice catch! If need someone else to cq+ (or r+ again), just let me know. > Source/WebInspectorUI/UserInterface/Base/MIMETypeUtilities.js:290 > + "application/xml": "xml", We already have a mapping above for "text/xml" (>173), so please move just this line to be after that. > Source/WebInspectorUI/UserInterface/Base/MIMETypeUtilities.js:325 > + if (extension === "js" || extension === "json" || extension === "xml") This shouldn't be added to this `if`, as it doesn't apply to the comment. If anything, I'd restructure a bit of this function: ```js let extension = WI.fileExtensionForMIMEType(mimeType); if (extension === "xml") return true; // Various script/JSON mime types. if (extension === "js" || extension === "json") return true; // Various media text mime types. if (extension === "m3u8" || extension === "m3u") return true; ``` Created attachment 374316 [details]
Patch
Comment on attachment 374294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374294&action=review >> Source/WebInspectorUI/UserInterface/Base/MIMETypeUtilities.js:290 >> + "application/xml": "xml", > > We already have a mapping above for "text/xml" (>173), so please move just this line to be after that. Ok, even if content with application/xml mimetype does not really belong to the "Document" group. >> Source/WebInspectorUI/UserInterface/Base/MIMETypeUtilities.js:325 >> + if (extension === "js" || extension === "json" || extension === "xml") > > This shouldn't be added to this `if`, as it doesn't apply to the comment. If anything, I'd restructure a bit of this function: > ```js > let extension = WI.fileExtensionForMIMEType(mimeType); > if (extension === "xml") > return true; > > // Various script/JSON mime types. > if (extension === "js" || extension === "json") > return true; > > // Various media text mime types. > if (extension === "m3u8" || extension === "m3u") > return true; > ``` Done Thanks for the review! Comment on attachment 374316 [details]
Patch
r=me, thanks :)
Comment on attachment 374316 [details] Patch Clearing flags on attachment: 374316 Committed r247533: <https://trac.webkit.org/changeset/247533> All reviewed patches have been landed. Closing bug. |