LGTM3 On Thu, Feb 13, 2025 at 10:41 AM 'Dave Risney' via blink-dev < blink-dev@chromium.org> wrote:
> Thanks everyone! > > - FWIW, it takes some digging to understand how this matchAll test > failure is realted to Client.url. Do we plan to add a more explicit test > that reads client.url after pushState() and asserts something useful? > - I forgot to add, I agree with Mike that it would be valuable to add > a more minimal web platform test for this specific behavior. > > It seems like the test is intending to validate (among other things) that > pushState doesn't change the Client.url. But I agree that it wasn't easy to > figure that out. I've opened a new crbug for adding a more minimal web > platform test (or please let me know if that's not the appropriate way to > track this work): > > - Add WPT validating history.pushState doesn't modify Service Worker > Client.url [396275853] - Chromium > <https://issues.chromium.org/issues/396275853> > > > Thanks! > -Dave > ------------------------------ > *From:* Philip Jägenstedt <foo...@chromium.org> > *Sent:* Thursday, February 13, 2025 2:55 > *To:* Domenic Denicola <dome...@chromium.org> > *Cc:* blink-dev <blink-dev@chromium.org>; Mike Taylor < > miketa...@chromium.org>; Hiroshige Hayashizaki <hirosh...@chromium.org>; > Dave Risney <david.ris...@microsoft.com> > *Subject:* [EXTERNAL] Re: [blink-dev] Intent to Ship: Service Worker > client URL ignore history.pushState changes > > You don't often get email from foo...@chromium.org. Learn why this is > important <https://aka.ms/LearnAboutSenderIdentification> > LGTM2 > > Thanks Domenic for trying some searches. We really don't have a way to > measure the risk of changes of string properties because we can't > instrument the returned string and "follow it around" or reason about what > code path is the expected one. I think digging deeper isn't justified given > that Firefox and Safari are already shipping this behavior. Being > responsive to any regressions should suffice in this case. > > Best regards, > Philip > > On Thu, Feb 13, 2025 at 5:34 AM Domenic Denicola <dome...@chromium.org> > wrote: > > I forgot to add, I agree with Mike that it would be valuable to add a more > minimal web platform test for this specific behavior. > > On Thursday, February 13, 2025 at 1:33:32 PM UTC+9 Domenic Denicola wrote: > > LGTM1. > > I think there is a small compat risk here, but in my opinion it's > mitigated by the movement toward interop, i.e. the fact that Firefox and > Safari have been shipping this behavior for years. > > I tried to do some searching along the lines of what Mike suggests. GitHub > wasn't very useful ("client.url" is not a unique-enough string), but some > cases on StackOverflow would prefer the Firefox/Safari behavior, e.g. this > one > <https://stackoverflow.com/questions/45309959/service-worker-offline-support-with-pushstate-and-client-side-routing> > . > > I feel that as long as the team stays vigilant about triaging incoming > bugs to watch out for breakage, and uses Finch to flip this off if such > breakage happens, we should feel comfortable pushing forward with this sort > of bug fix. > > > On Thursday, February 13, 2025 at 10:25:20 AM UTC+9 Mike Taylor wrote: > > > On 2/12/25 4:43 PM, 'Dave Risney' via blink-dev wrote: > > Contact emails > david.ris...@microsoft.com, hirosh...@chromium.org > > Explainer > None > > Specification > https://www.w3.org/TR/service-workers/#client-url > > Summary > Modify the service worker Client.url property to ignore document URL > changes via history.pushState() and other similar history APIs. The > Client.url property is intended to be the creation URL of the HTML document > which ignores such changes. > > > Blink component > Blink>ServiceWorker > <https://issues.chromium.org/issues?q=customfield1222907:%22Blink%3EServiceWorker%22> > > TAG review > None: this is a bug fix change to an existing API surface area. > > TAG review status > Not applicable > > Risks > > > Interoperability and Compatibility > The service worker spec says that the Client.url property should be the > creation URL of the document which ignores changes made via > history.pushState and similar, rather than the URL which includes such > changes. Firefox and Safari match the standard. Chromium currently uses the > document URL. Changing Chromium behavior to match the standard could have > compatibility issues for existing web content that expects to see the > existing Chromium behavior of history.pushState API changes applying to the > Client.url property. > > Can you help us estimate the compatibility risk here? One way might be to > find sites or apps (maybe via GitHub? HTTP Archive?) that are working > around this difference, so we can get a sense of what code paths might or > might not be affected. > > > > *Gecko*: Shipped/Shipping (https://results.web-platform- > tests.org/results/service-workers/service-worker/ > clients-matchall-client-types.https.html?label=experimental& > label=master&aligned&q=service-workers%2Fservice- > worker%2Fclients-matchall-client-types.https) > > *WebKit*: Shipped/Shipping (https://results.web-platform- > tests.org/results/service-workers/service-worker/ > clients-matchall-client-types.https.html?label=experimental& > label=master&aligned&q=service-workers%2Fservice- > worker%2Fclients-matchall-client-types.https) > > *Web developers*: Positive (https://issues.chromium.org/issues/41405003) > Web developers have noticed and had to deal with this bug: * > https://github.com/w3c/ServiceWorker/issues/1515 * > https://issues.chromium.org/issues/41405003 > > *Other signals*: > > 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 > None > > > 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 > This Web Platform test below validates aspects of the service worker > Client API. Currently Chromium is failing this test because the Client.url > is the document's URL when it should be the creation URL which ignores > document URL changes via the history API. https://results.web-platform- > tests.org/results/service-workers/service-worker/ > clients-matchall-client-types.https.html?label=experimental& > label=master&aligned&q=service-workers%2Fservice- > worker%2Fclients-matchall-client-types.https > > FWIW, it takes some digging to understand how this matchAll test failure > is realted to Client.url. Do we plan to add a more explicit test that reads > client.url after pushState() and asserts something useful? > > > > Flag name on about://flags > None > > Finch feature name > ServiceWorkerClientUrlIsCreationUrl > > Requires code in //chrome? > False > > Tracking bug > https://issues.chromium.org/issues/41337436 > > Estimated milestones > Shipping on desktop > 135 > Shipping on Android > 135 > Shipping on WebView > 135 > > > 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/4996996949344256?gate=6527947635425280 > > -- > 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 visit https://groups.google.com/a/ > chromium.org/d/msgid/blink-dev/SA6PR00MB235659A2F02466574F2E6 > 5478BFC2%40SA6PR00MB2356.namprd00.prod.outlook.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/SA6PR00MB235659A2F02466574F2E65478BFC2%40SA6PR00MB2356.namprd00.prod.outlook.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 visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/e9d48b3e-661e-427a-bd80-bae907f76b06n%40chromium.org > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/e9d48b3e-661e-427a-bd80-bae907f76b06n%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 blink-dev+unsubscr...@chromium.org. > To view this discussion visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/SA6PR00MB2356F676ED0D7CECFE303A1C8BFF2%40SA6PR00MB2356.namprd00.prod.outlook.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/SA6PR00MB2356F676ED0D7CECFE303A1C8BFF2%40SA6PR00MB2356.namprd00.prod.outlook.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 visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw82%2B3DcLxWDYUC3b6kBPOOFn1JH2MYpOq3VuJsSFnAEeg%40mail.gmail.com.
