Thanks for the LGTMs! On Wed, May 29, 2024 at 7:54 AM Daniel Bratell <brat...@sarasas.se> wrote:
> Deprecating seems fine, especially since it's a relatively new API and > less likely to be used on non-maintained sites, but removal seems a bit > premature even if done slowly. Would it be better to let it bake a few > milestones and see if a scary deprecation message threatening removal after > the summer drives the usage down? > I'm starting to get the feeling that this would be a good idea. The Enterprise folks on the chromestatus gate also asked for something similar - 3 milestones of warning period before the deprecation. I'm inclined to just do that - it feels safer, and I don't think there's a huge rush to turn this off. Especially with usage going in the right direction, instead of further up. So I've modified the chromestatus <https://chromestatus.com/feature/5081733588582400> to show a shipping milestone of M129. Does that work? And I'll add a big console error starting in M127. On Wed, May 29, 2024 at 1:22 AM Yoav Weiss (@Shopify) < yoavwe...@chromium.org> wrote: > Your questions prompted me to take a closer look at the sample sites still >>> hitting the use counter. I took a close look at the first 10 entries >>> listed, and I think I found perhaps where the big drop came from. Of those >>> ten sites, seven do not use getInnerHTML or getHTML at all. Likely not >>> coincidentally, all seven are Shopify sites. My guess would be that Shopify >>> very recently removed its usage of getInnerHTML? >>> >> > FWIW, internal code search brought up nothing. It's possible that this is > a 3P app > <https://shopify.dev/docs/apps/build/online-store/theme-app-extensions> that > changed their use. (or that I'm simply failing to find the relevant change > :D) > Thanks for checking that! It's always hard to figure out what happened after the fact like this. > The plan to ramp down usage is a good one, although as we previously >> discussed in a different thread, it may cause some debugging challenges for >> developers. It is worthwhile to also reach out to some of the developers >> whose sites you noticed would throw an exception -- just an FYI email that >> this feature is being removed. Given the fairly low usage, readily >> available fixes (via getHTML() or possibly innerHTML) and your commitment >> to monitor for breakages, this looks good to me. >> > I might want to tweak this plan now that we're extending by 2/3 milestones. Maybe now (after console warnings) it makes more sense to just disable in one shot? I'll try to reach out to the sites I noticed. Thanks, Mason > /Daniel > On 2024-05-29 10:22, Yoav Weiss (@Shopify) wrote: > > LGTM2 > > On Tue, May 28, 2024 at 11:10 PM Vladimir Levin <vmp...@chromium.org> > wrote: > >> >> >> On Tue, May 28, 2024 at 12:30 PM Mason Freed <mas...@chromium.org> wrote: >> >>> >>> >>> On Mon, May 27, 2024 at 8:15 AM Vladimir Levin <vmp...@chromium.org> >>> wrote: >>> >>>> Interoperability and Compatibility >>>>> >>>>> The use counter for getInnerHTML() ( >>>>> https://chromestatus.com/metrics/feature/timeline/popularity/3874) >>>>> peaked at 0.05% of page loads using this function as of January 2024, and >>>>> dropped precipitously toward 0.01% in May, 2024. This is presumably due to >>>>> the shipment of its replacement, getHTML(). >>>>> >>>> >>>> It's great to see the numbers reduce significantly. If the numbers are >>>> being migrated to getHTML() though I would have expected >>>> https://chromestatus.com/metrics/feature/timeline/popularity/4781 to >>>> grow by ~0.04 percentage points, but that one is still significantly lower >>>> (although growing). Is it possible that June 1 numbers would show a better >>>> balance? Do you by any chance know when the next data point is expected to >>>> be visible on chromestatus? >>>> >>>> I'm also assume people are using a readily available replacement as >>>> opposed to just not using getInnerHTML, but it would be nice if number >>>> supported that >>>> >>> >>> Great questions. So AFAIK the use counter plot for the current month is >>> a continuous aggregation. I.e. the 0.0168% I see today (May 28) is as of >>> the 28th, and will change tomorrow (slightly). Given that we're almost to >>> the end of the month, I wouldn't expect a ton of shift. So I think you >>> might be right that this isn't actually a shift to getHTML, but just a >>> shift away from getInnerHTML. See more below. >>> >>> Your questions prompted me to take a closer look at the sample sites >>> still hitting the use counter. I took a close look at the first 10 entries >>> listed, and I think I found perhaps where the big drop came from. Of those >>> ten sites, seven do not use getInnerHTML or getHTML at all. Likely not >>> coincidentally, all seven are Shopify sites. My guess would be that Shopify >>> very recently removed its usage of getInnerHTML? >>> >> > FWIW, internal code search brought up nothing. It's possible that this is > a 3P app > <https://shopify.dev/docs/apps/build/online-store/theme-app-extensions> > that changed their use. (or that I'm simply failing to find the relevant > change :D) > > >>> The real issue is that the remaining three sites *do* still use >>> getInnerHTML, and all three throw exceptions when the feature is disabled. >>> I can't perceive anything broken on the site, but the exception isn't a >>> good sign. A few interesting tidbits: one of the three does appear to >>> (properly) feature-detect getInnerHTML() yet an exception is still thrown >>> that might or might not be related. The other two do not feature detect, >>> and the exception is clear: "getInnerHTML is not a function". Very >>> interestingly, none of the three use getInnerHMTL for anything declarative >>> shadow dom related. They seem to just be using it as a way to get the >>> innerHTML value. All three seem to be hand-written JS, so it's possible the >>> sites were developed on Chrome in the last few years and the developer >>> didn't notice that they should have done foo=el.innerHTML instead of >>> foo=el.getInnerHTML(). >>> >>> Given that the use counter is very low (0.01%), I'd still like to push >>> ahead with this deprecation. The above sites likely represent interop >>> problems, since they'll break on other browsers already today. But I'd like >>> to revise my plan: instead of going immediately to 100% removal, I'd like >>> to use a slow ramp down over time, to monitor for reported breakage. >>> >>> Thoughts? >>> >> >> The plan to ramp down usage is a good one, although as we previously >> discussed in a different thread, it may cause some debugging challenges for >> developers. It is worthwhile to also reach out to some of the developers >> whose sites you noticed would throw an exception -- just an FYI email that >> this feature is being removed. Given the fairly low usage, readily >> available fixes (via getHTML() or possibly innerHTML) and your commitment >> to monitor for breakages, this looks good to me. >> >> LGTM1 >> >> >>> >>> Thanks, >>> Mason >>> >>> >>> >>>> >>>> While 0.01% still represents high usage for deprecation, the numbers >>>>> were significantly worse for the deprecation of the old `shadowroot` >>>>> attribute, and the removal of that feature generated zero bug reports. It >>>>> is my strong belief that since this feature is only shipped in Chrome, the >>>>> vast majority of usage is guarded by feature checks. So this deprecation >>>>> should be safer than it would seem from the numbers. I'd like to remove >>>>> this feature in M127 in code, with a killswitch (a re-enable switch >>>>> really) >>>>> in case of problems. >>>>> >>>>> >>>>> *Gecko*: No signal >>>>> >>>>> *WebKit*: No signal >>>>> >>>>> *Web developers*: No signals >>>>> >>>>> *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 >>>>> >>>>> Flag name on chrome://flags ElementGetInnerHTML >>>>> >>>>> Finch feature name ElementGetInnerHTML >>>>> >>>>> Requires code in //chrome? False >>>>> >>>>> Tracking bug https://crbug.com/1519972 >>>>> >>>>> Estimated milestones >>>>> Shipping on desktop 127 >>>>> Shipping on Android 127 >>>>> Shipping on WebView 127 >>>>> >>>>> 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/5081733588582400?gate=5088451454304256 >>>>> >>>>> 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+unsubscr...@chromium.org. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAM%3DNeDjZJvRAcpSj2cAWi6uW7yYmDV8HdMkqQjFOS3q%3DidB9fQ%40mail.gmail.com >>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAM%3DNeDjZJvRAcpSj2cAWi6uW7yYmDV8HdMkqQjFOS3q%3DidB9fQ%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/CAM%3DNeDh_myM1eudBh_%3DeY4F9UZN9vZxRa9%2BmV9vKSR9Nh1iHhw%40mail.gmail.com >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAM%3DNeDh_myM1eudBh_%3DeY4F9UZN9vZxRa9%2BmV9vKSR9Nh1iHhw%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/CADsXd2MvNLyYnrF2q%2BhEx8EAoiZsC_ws3Bj3%3DOw7K5m-hRAvDQ%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADsXd2MvNLyYnrF2q%2BhEx8EAoiZsC_ws3Bj3%3DOw7K5m-hRAvDQ%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/CAOmohSJ_LsKuzR2FezpC4cxyFsb5nUaG65LeidpyfSmFWVGJaQ%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOmohSJ_LsKuzR2FezpC4cxyFsb5nUaG65LeidpyfSmFWVGJaQ%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/CAM%3DNeDhoYiq3D6HFupxGhQL1SOSKVUFD%3DxQJGXTamyDvPTDr2A%40mail.gmail.com.
