[webkit-reviews] review granted: [Bug 192731] Web Inspector: m3u8 content not shown, it should be text : [Attachment 357383] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 192731: Web Inspector: m3u8 content not shown, it should be text
https://bugs.webkit.org/show_bug.cgi?id=192731

Attachment 357383: [PATCH] Proposed Fix

https://bugs.webkit.org/attachment.cgi?id=357383&action=review




--- Comment #8 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 357383
  --> https://bugs.webkit.org/attachment.cgi?id=357383
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=357383&action=review

r=me, nice catch :)

> Source/WebCore/inspector/NetworkResourcesData.cpp:190
> +    if (content.isEmpty())

Should this be `isNull`?  Early returning with an empty string could mean that
we return the error "No data found for resource with given identifier", as we
wouldn't return true for `NetworkResourceData::ResourceData::hasContent`. 
Isn't an empty string "" considered a valid response (e.g. a request that just
returns 200)?

> Source/WebCore/platform/MIMETypeRegistry.cpp:506
> +	   auto subtype = StringView { mimeType }.substring(applicationLength);

Is there a reason to use this syntax over `StringView(mimeType)`?

> Source/WebCore/platform/MIMETypeRegistry.cpp:514
> +	   auto subtype = StringView { mimeType }.substring(audioLength);

Ditto (>506).

> Source/WebInspectorUI/UserInterface/Base/MIMETypeUtilities.js:325
>      if (mimeType.startsWith("application/"))
>	   return mimeType.endsWith("script") || mimeType.endsWith("json");

We should probably modify this to use `extension` and just check for "js" and
"json" (also maybe "ps").