Bug 219612 – Parse content after # in data URLs with HLS mime types

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Parse content after # in data URLs with HLS mime types

[Alex Christensen]

Created attachment 415576 [details]
Patch

[Alex Christensen]

<rdar://problem/71039282>

[Darin Adler]

Comment on attachment 415576 [details]
Patch

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

> Source/WebCore/platform/network/DataURLDecoder.cpp:48
> +static bool shouldRemoveFragmentIdentifier(const String& mediaType)

I’d like to see a "why" comment mentioning the reason for the MIME type special cases here.

Also wondering if we will keep this forever.

> Source/WebCore/platform/network/DataURLDecoder.cpp:55
> +    return !equalIgnoringASCIICase(mediaType, "video/mpegURL")
> +        && !equalIgnoringASCIICase(mediaType, "audio/mpegURL");

This could instead be written in this slightly more efficient way:

    return !equalLettersIgnoringASCIICase(mediaType, "video/mpegurl")
        && !equalLettersIgnoringASCIICase(mediaType, "audio/mpegurl");

> Source/WebCore/platform/network/DataURLDecoder.cpp:127
> -    const String urlString;
> +    URL url;

This is a much bigger object. Is that OK?

[Darin Adler]

Comment on attachment 415576 [details]
Patch

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

> Source/WebCore/platform/network/DataURLDecoder.cpp:54
> +    return !equalIgnoringASCIICase(mediaType, "video/mpegURL")

Would be nice if this was turned off if "Disable Site-specific Hacks" was set. While we are not limiting this to a particular website, turning this off to test if the website works without the quirk is something that fits the mission of that menu item and switch.

[Darin Adler]

Comment on attachment 415576 [details]
Patch

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

>> Source/WebCore/platform/network/DataURLDecoder.cpp:54
>> +    return !equalIgnoringASCIICase(mediaType, "video/mpegURL")
> 
> Would be nice if this was turned off if "Disable Site-specific Hacks" was set. While we are not limiting this to a particular website, turning this off to test if the website works without the quirk is something that fits the mission of that menu item and switch.

Do non-Apple WebKit ports want this quirk? I’m guessing no, since they don’t have HTTP Live Streaming.

[Alex Christensen]

From what I understand, this is not site-specific or likely to go away because HLS manifests use # in the middle and aren't about to stop.  I will restrict it to cocoa platforms, though.

[Alex Christensen]

A URL is 32 bytes bigger than a String, but that's not significant compared to sizeof(DecodeTask) plus typical data URL sizes, and it's a very short-lived object.  We need the URL to know where to start removing the fragment that we've already found when parsing the URL.

[Alex Christensen]

Created attachment 415581 [details]
Patch

[Alex Christensen]

Created attachment 415588 [details]
Patch

[Darin Adler]

Comment on attachment 415588 [details]
Patch

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

> Source/WebCore/platform/network/DataURLDecoder.cpp:58
> +    return !equalIgnoringASCIICase(mediaType, "video/mpegurl")
> +        && !equalIgnoringASCIICase(mediaType, "audio/mpegurl")
> +        && !equalIgnoringASCIICase(mediaType, "application/x-mpegurl")
> +        && !equalIgnoringASCIICase(mediaType, "vnd.apple.mpegurl");

This lowercases things as required by equalLettersIgnoringASCIICase, but didn’t add the word "Letters" to the function name.

[Alex Christensen]

Created attachment 415591 [details]
Patch

[EWS]

Committed r270526: <https://trac.webkit.org/changeset/270526>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415591 [details].