LGTM1. I recall these methods getting lots of good design review and discussion in the PR, from multiple parties. I'm excited to see them ship.
Thanks Luke for spotting the trusted types interaction, and fixing it! On Saturday, February 17, 2024 at 2:15:09 AM UTC+9 Joey Arhar wrote: > > Is this the relevant explainer (referenced from the PR below): > https://github.com/WICG/sanitizer-api/blob/main/explainer.md > > Yes, as far as I know. > > > This seems positive, right? > > Whoops, meant to put positive. I updated the chromestatus. > > > Both of these look like "Shipped/Shipping", per > https://bit.ly/blink-signals. That status is a little odd, because it > doesn't look like they've actually made it to a stable release, but if I'm > reading the bug trackers right they're both merged, so they're past "In > Development". > > Ok, I'll change them to shipped/shipping. > > On Thu, Feb 15, 2024 at 9:35 AM Luke <lwar...@igalia.com> wrote: > >> Just to keep everyone up to date, you can disregard my remarks above I've >> landed a patch which addresses the lack of trusted types protection, thanks >> for the quick review Joey. >> >> Regards, >> Luke >> >> On Wednesday, February 14, 2024 at 10:49:23 PM UTC Luke wrote: >> >>> Hi, >>> >>> In it's current form Chromium's implementation of these functions >>> bypasses trusted types protection. >>> >>> The below WPT tests cover this behaviour: >>> >>> >>> https://wpt.fyi/results/trusted-types/block-string-assignment-to-ShadowRoot-setHTMLUnsafe.html?label=experimental&label=master&aligned >>> >>> https://wpt.fyi/results/trusted-types/block-string-assignment-to-Element-setHTMLUnsafe.html?label=experimental&label=master&aligned >>> >>> https://wpt.fyi/results/trusted-types/block-string-assignment-to-Document-parseHTMLUnsafe.html?label=experimental&label=master&aligned >>> >>> This should be addressed before shipping, else it will be an unexpected >>> security regression. >>> >>> On Wednesday, February 14, 2024 at 10:23:01 PM UTC Vladimir Levin wrote: >>> >>>> On Wed, Feb 14, 2024 at 1:53 PM Jeffrey Yasskin <jyas...@chromium.org> >>>> wrote: >>>> >>>>> Non-API-owner opinions inline: >>>>> >>>>> On Wed, Feb 14, 2024 at 1:42 PM 'Vladimir Levin' via blink-dev < >>>>> blin...@chromium.org> wrote: >>>>> >>>>>> I just had some clarifying questions >>>>>> >>>>>> On Wed, Feb 14, 2024 at 1:13 PM Joey Arhar <jar...@chromium.org> >>>>>> wrote: >>>>>> >>>>>>> Some additional notes: >>>>>>> - This API is tested in the declarative ShadowDOM tests in >>>>>>> interop2024, and it is counting against us to not have it enabled by >>>>>>> default. >>>>>>> - The future sanitization options will be added as an optional >>>>>>> second parameter to both methods, so there will not be any compat >>>>>>> issues >>>>>>> with shipping now. >>>>>>> >>>>>>> On Wed, Feb 14, 2024 at 1:11 PM Joey Arhar <jar...@chromium.org> >>>>>>> wrote: >>>>>>> >>>>>>>> Contact emailsjar...@chromium.org >>>>>>>> >>>>>>>> ExplainerNone >>>>>>>> >>>>>>> >>>>>> Is this the relevant explainer (referenced from the PR below): >>>>>> https://github.com/WICG/sanitizer-api/blob/main/explainer.md >>>>>> >>>>>> >>>>>>> >>>>>>>> >>>>>>>> Specification >>>>>>>> https://html.spec.whatwg.org/C/#unsafe-html-parsing-methods >>>>>>>> https://github.com/whatwg/html/pull/9538 >>>>>>>> >>>>>>>> Summary >>>>>>>> >>>>>>>> The setHTMLUnsafe and parseHTMLUnsafe methods allow Declarative >>>>>>>> ShadowDOM to be used from javascript. In the future, they may also get >>>>>>>> new >>>>>>>> parameters for sanitization. >>>>>>>> >>>>>>>> >>>>>>>> Blink componentBlink>HTML >>>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EHTML> >>>>>>>> >>>>>>>> TAG reviewNone >>>>>>>> >>>>>>>> TAG review statusNot applicable >>>>>>>> >>>>>>> >>>>>> There seems to be consensus within browser vendors that this is a >>>>>> good idea, but I'm just wondering why you decided against filing TAG >>>>>> here? >>>>>> >>>>> >>>>> IMO, either Firefox or Safari folks should have filed a TAG review for >>>>> this before they merged their patches. Now that they've merged, I think >>>>> it >>>>> falls into the "[already specified && already shipped]" exception >>>>> category >>>>> <https://www.chromium.org/blink/guidelines/api-owners/process-exceptions/>, >>>>> >>>>> and it's probably too fixed to ask the TAG to spend time on it. >>>>> >>>> >>>> (also non-api-owner, but responding anyway): if that is in fact >>>> shipping then I agree that this should be the exception here, thanks. >>>> >>>> >>>>> >>>>>> Risks >>>>>>>> >>>>>>>> >>>>>>>> Interoperability and Compatibility >>>>>>>> >>>>>>>> None >>>>>>>> >>>>>>>> >>>>>>>> *Gecko*: No signal ( >>>>>>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1850675) >>>>>>>> https://github.com/whatwg/html/pull/9538#issuecomment-1728947778 >>>>>>>> >>>>>>> >>>>>> This seems positive, right? >>>>>> >>>>> >>>>> >>>>>> *WebKit*: Positive (https://bugs.webkit.org/show_bug.cgi?id=261143) >>>>>>>> >>>>>>> >>>>>> I'm not sure how to read this properly, but is this a positive signal >>>>>> or "shipped/shipping" signal? >>>>>> >>>>> >>>>> Both of these look like "Shipped/Shipping", per >>>>> https://bit.ly/blink-signals. That status is a little odd, because it >>>>> doesn't look like they've actually made it to a stable release, but if >>>>> I'm >>>>> reading the bug trackers right they're both merged, so they're past "In >>>>> Development". >>>>> >>>> >>>> Yeah, that's my thought here too. My understanding is that all of the >>>> patches here are merged, but I just wanted to double check in case I'm >>>> misunderstanding what those bugs are implying. >>>> >>>> >>>>> >>>>> >>>>>> *Web developers*: No signals >>>>>>>> >>>>>>>> *Other signals*: >>>>>>>> >>>>>>>> Ergonomics >>>>>>>> >>>>>>>> This API will likely be used in tandem with Declarative ShadowDOM. >>>>>>>> The default usage of this API will not make it hard for chrome to >>>>>>>> maintain >>>>>>>> good performance. >>>>>>>> >>>>>>>> >>>>>>>> Activation >>>>>>>> >>>>>>>> It will not be challenging for developers to use this feature >>>>>>>> immediately. >>>>>>>> >>>>>>>> >>>>>>>> Security >>>>>>>> >>>>>>>> There are no security risks. This API just does declarative >>>>>>>> ShadowDOM. There is an "unsafe" in the name because there are future >>>>>>>> plans >>>>>>>> to add sanitization options. >>>>>>>> https://github.com/WICG/sanitizer-api/issues/185 >>>>>>>> https://github.com/whatwg/html/issues/8627 >>>>>>>> https://github.com/whatwg/html/issues/8759 >>>>>>>> >>>>>>>> >>>>>>>> WebView application risks >>>>>>>> >>>>>>>> Does this intent deprecate or change behavior of existing APIs, >>>>>>>> such that it has potentially high risk for Android WebView-based >>>>>>>> applications? >>>>>>>> >>>>>>>> None >>>>>>>> >>>>>>>> >>>>>>>> Debuggability >>>>>>>> >>>>>>>> This API does not need any special DevTools features. You can call >>>>>>>> the method from the console panel. >>>>>>>> >>>>>>>> >>>>>>>> Will this feature be supported on all six Blink platforms (Windows, >>>>>>>> Mac, Linux, ChromeOS, Android, and Android WebView)?Yes >>>>>>>> >>>>>>>> Is this feature fully tested by web-platform-tests >>>>>>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md> >>>>>>>> ?Yes >>>>>>>> >>>>>>>> Flag name on chrome://flagsHTMLUnsafeMethods >>>>>>>> >>>>>>>> Finch feature nameHTMLUnsafeMethods >>>>>>>> >>>>>>>> Requires code in //chrome?False >>>>>>>> >>>>>>>> Estimated milestones >>>>>>>> DevTrial on desktop 120 >>>>>>>> DevTrial on Android 120 >>>>>>>> >>>>>>>> Anticipated spec changes >>>>>>>> >>>>>>>> Open questions about a feature may be a source of future web compat >>>>>>>> or interop issues. Please list open issues (e.g. links to known github >>>>>>>> issues in the project for the feature specification) whose resolution >>>>>>>> may >>>>>>>> introduce web compat/interop risk (e.g., changing to naming or >>>>>>>> structure of >>>>>>>> the API in a non-backward-compatible way). >>>>>>>> None >>>>>>>> >>>>>>>> Link to entry on the Chrome Platform Status >>>>>>>> https://chromestatus.com/feature/6560361081995264 >>>>>>>> >>>>>>>> This intent message was generated by Chrome Platform Status >>>>>>>> <https://chromestatus.com/>. >>>>>>>> >>>>>>> -- >>>>>>> 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 blink-dev+...@chromium.org. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAK6btwJiEbhk_YGbVhuUg0emSJTfT%3D20_1bTDMFJxcH5i9tbMQ%40mail.gmail.com >>>>>>> >>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAK6btwJiEbhk_YGbVhuUg0emSJTfT%3D20_1bTDMFJxcH5i9tbMQ%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 blink-dev+...@chromium.org. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADsXd2MH_fZddPf6c_QwhEP5JU767nEy1ck338Cx_HYFsytO4w%40mail.gmail.com >>>>>> >>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADsXd2MH_fZddPf6c_QwhEP5JU767nEy1ck338Cx_HYFsytO4w%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 blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/ecd95303-33be-4315-8a06-92c1330cfa09n%40chromium.org.
