LGTM2 On Wednesday, February 21, 2024 at 6:19:50 AM UTC+1 Domenic Denicola wrote:
> 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/28b97986-2078-4cfb-aae2-58514a35bff6n%40chromium.org.
