Bug 213253

Created attachment 402016 [details]
Patch

Comment on attachment 402016 [details]
Patch

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

> Source/WebCore/fileapi/BlobBuilder.cpp:74
> -    auto bytes = UTF8Encoding().encodeWithNormalization(text, UnencodableHandling::Entities);
> +    auto bytes = UTF8Encoding().encode(text, UnencodableHandling::Entities);

While this is correct in abstract, I don't think that we can do this before the normalization is performed in all code paths where decomposed text from macOS can enter WebKit. And once we do that, encode can stop normalizing.

(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 402016 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402016&action=review
> 
> > Source/WebCore/fileapi/BlobBuilder.cpp:74
> > -    auto bytes = UTF8Encoding().encodeWithNormalization(text, UnencodableHandling::Entities);
> > +    auto bytes = UTF8Encoding().encode(text, UnencodableHandling::Entities);
> 
> While this is correct in abstract, I don't think that we can do this before
> the normalization is performed in all code paths where decomposed text from
> macOS can enter WebKit. And once we do that, encode can stop normalizing.


This effect only for constructing `Blob` API from string on JavaScript.

The exist `TextEncoding::encode()` do normalization in it and it is widely used in evenrywhere in WebKit.
https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/text/TextEncoding.cpp#L77

I had planned to add `TextEncoding::encodeWithoutNormalization()` but I also thought from the comment that it might be better to rename the exist implementation once, I filed bug 213254 and I created this patch built on it.

Since blobs can be sent over the wire and/or processed by algorithms that were only tested on Windows, their content should not include decomposed strings coming from macOS. It is true that the test is accurate and we need to make it pass, but disabling normalization would be the last step for making it happen.

Umm..... it's a long way. Thank you for your explanation.

*** This bug has been marked as a duplicate of bug 217327 ***