Bug 201046

<rdar://problem/54446087>

Created attachment 377065 [details] Patch

Created attachment 377066 [details] [Image] After Patch is applied

Comment on attachment 377065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377065&action=review r=me > Source/WebInspectorUI/UserInterface/Views/LocalDOMContentView.js:45 > + let content = this._content.replace(/`/g, "\\`"); > + let mimeType = this._mimeType.replace(/"/g, "\\\""); > + return `(new DOMParser).parseFromString(\`${content}\`, "${mimeType}")`; Can these just be `JSON.stringify(x)` calls? return `(new DOMParser).parseFromString(${JSON.stringify(content)}), ${JSON.stringify(mimeType)})`; > Source/WebInspectorUI/UserInterface/Views/LocalDOMContentView.js:56 > + treeOutline[WI.DOMTreeOutline.usingLocalDOMNodeSymbol] = true; Why not make this an API method instead of a symbol property? > Source/WebInspectorUI/UserInterface/Views/LocalRemoteObjectContentView.js:59 > + RuntimeAgent.evaluate.invoke(options, (error, result, wasThrown) => { I wonder if we should give this a `target` now? I think this could be any target, in which case we could just use `WI.mainTarget`. > Source/WebInspectorUI/UserInterface/Views/ResourceClusterContentView.js:408 > + case "text/xml": > + case "application/xml": > + case "application/xhtml+xml": > + case "image/svg+xml": It is very common for arbitrary mime types to have a "+xml" suffix, so we should probably do: if (mimeType.endsWith("/xml") || mimeType.endsWith("+xml")) ...; > Source/WebInspectorUI/UserInterface/Views/ResourceClusterContentView.js:412 > + } catch {} Style: We normally have a space in here: `catch { }`. > Source/WebInspectorUI/UserInterface/Views/ResourceClusterContentView.js:497 > + Request: "request", > + RequestJSON: "request-json", Why not Request DOM? I'm sure someone out there still sends XML to their server!

Comment on attachment 377065 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=377065&action=review >> Source/WebInspectorUI/UserInterface/Views/LocalDOMContentView.js:45 >> + return `(new DOMParser).parseFromString(\`${content}\`, "${mimeType}")`; > > Can these just be `JSON.stringify(x)` calls? > > return `(new DOMParser).parseFromString(${JSON.stringify(content)}), ${JSON.stringify(mimeType)})`; Doh! >> Source/WebInspectorUI/UserInterface/Views/ResourceClusterContentView.js:408 >> + case "image/svg+xml": > > It is very common for arbitrary mime types to have a "+xml" suffix, so we should probably do: > > if (mimeType.endsWith("/xml") || mimeType.endsWith("+xml")) > ...; Ooooo, that's true! >> Source/WebInspectorUI/UserInterface/Views/ResourceClusterContentView.js:497 >> + RequestJSON: "request-json", > > Why not Request DOM? I'm sure someone out there still sends XML to their server! Why not indeed! I'll add it :D

Created attachment 377946 [details] Patch

Comment on attachment 377946 [details] Patch Clearing flags on attachment: 377946 Committed r249451: <https://trac.webkit.org/changeset/249451>

All reviewed patches have been landed. Closing bug.