Thanks for digging into this. Given your investigation, I agree that a PSA+flag is sufficient here.
On Tue, Aug 22, 2023, 23:08 David Bokan <[email protected]> wrote: > Apologies, I wasn't aware the PSA guidance has changed. I've created > https://chromestatus.com/feature/5168429241204736 to track the change. > > As to an intent, I believed the breakage risk to be small so thought PSA > was sufficient. I think so since we already can perform this kind of merge > (from https://crrev.com/2fdffd306d488, granted, the case that CL merges > is very unlikely to happen). Additionally, if this case does get hit, the > resulting data would still be consistent, applications are likely to > concatenate the CDATA sections anyway. > > But I didn't have hard data so I did some digging today: > > Via the XMLDocument UseCounter, 0.18% of page loads include an XMLDocument > (this is the *upper bound* on potentially affected page loads). > > I went looking through HTTPArchive data using the `requests` and > `resposne_bodies` tables from 2023_08_01_desktop. I filtered requests to: > > Content-Type header with "application/xhtml+xml" > request_type "Document" > > Which I believe is the case where XMLDocument is used. > > I then looked at the response bodies for those requests looking for > adjacent CDATA sections: looking for instances of the string "]]><![CDATA[" > and found none. (note, any kind of space, comment, other text would insert > a node between them and thus prevent the merge). > > Certainly not definitive but I expect this to be encountered very rarely > and even when encountered, it's unlikely to cause an issue unless the page > is using some kind of reflection (e.g. looking at `node.innerHTML` and then > relying on there being separate CDATA sections for some reason). > > Of all the cases I've seen in HTTPArchive, CDATA is used simply to ensure > that <script> and <style> content is parsed correctly when loaded as XHTML. > > Given all this, I think breakage is very unlikely (and we have a flag > guard just in case) so I think PSA is enough but if anyone thinks a full > intent is justified please let me know. > > Thanks, > David > > > > On Tue, Aug 22, 2023 at 2:37 AM Yoav Weiss <[email protected]> wrote: > >> In particular, we recently clarified our process RE changes to existing >> behavior >> <https://www.chromium.org/blink/launching-features/#behavior-changes>. >> It'd be good to answer the questions in "step 2" to understand if the >> breakage potential here is very unlikely. >> >> On Tue, Aug 22, 2023 at 8:29 AM Mathias Bynens <[email protected]> >> wrote: >> >>> Given that this presents a Web compat risk, is there a ChromeStatus >>> entry + Intent thread for this? >>> >>> On Mon, Aug 21, 2023 at 7:05 PM David Bokan <[email protected]> wrote: >>> >>>> Hi blink-dev, >>>> >>>> I just landed https://crrev.com/f848cbce89b422 which will cause an XML >>>> document to merge sibling CDATA section nodes into a single CDATA section >>>> during parsing. >>>> >>>> The reason for this is that libxml emits separate nodes at 300 byte >>>> boundaries for a CDATA section that's spread across more than one input >>>> chunk. If the CDATA is split across input chunks, libxml also emits >>>> separate nodes and this merging behavior was already applied to this case >>>> by https://crrev.com/2fdffd306d488 - the change I'm making generalizes >>>> this to also cover the 300 byte chunking behavior (see CL commit message >>>> for more details). >>>> >>>> I'm hoping this is web-compatible but have added a flag-guard if we >>>> need to turn it off. If anyone sees any breakage related to this or knows >>>> specific sites/apps to check please let me know. >>>> >>>> Thanks! >>>> David >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "blink-dev" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/4bfd9115-b843-4dc7-b10f-26b87d215689n%40chromium.org >>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/4bfd9115-b843-4dc7-b10f-26b87d215689n%40chromium.org?utm_medium=email&utm_source=footer> >>>> . >>>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "blink-dev" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADizRgZRWinxYVJORErQpqiQFYhRba%2BLMoUwFeq3-7%3DRwtx8hQ%40mail.gmail.com >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADizRgZRWinxYVJORErQpqiQFYhRba%2BLMoUwFeq3-7%3DRwtx8hQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfW3AAnrt8CCsQ-%2Ba4BMvvSMW925CmxPWT9xT7mSGPxv1Q%40mail.gmail.com.
