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/CAARdPYfLNMJXfGBfCz7AuU3AV83335ku_qB%2BT6k5NwnuOoMgNw%40mail.gmail.com.