Thanks for all the comments! I'll do a revisit later. On Wed, Mar 30, 2022 at 5:53 AM Philip Jägenstedt <foo...@chromium.org> wrote:
> Hi Xiaocheng, > > Thanks for doing this analysis, I think it's the first time I've seen > categorization done this thoroughly using BigQuery, and I'm impressed :) > > While one could also look at "e.path", I think what you have should be > enough to get a good idea of the risk. Having identified a bunch of sites > that might be broken, I think the next step is to take a random sample of > them, say 10 sites. For each site, understand when that code path is > reached in current Chrome, and then take a locally built version of Chrome > with the API removed and reach that code path. What happens? > > It sounds a lot that we have ~2500 site in the "hard fail" category, but I > would bet that on closer inspection you will find: > > - Code that you can't figure out how to even reach, which might be > dead code > - Code that throws errors but everything works fine from the user's > point of view > - Broken tracking or ads that the user could notice, but wouldn't > really care about > > And then finally, you might find cases that really are broken, but which > are already broken in Firefox and Safari. If that is most of the remaining > cases, then we have to weigh breaking those sites in Chrome and likely > pushing them to be fixed (in all browsers) soonish, vs. them remaining > broken in some browsers for what is very likely a longer time. In this > case, I would say LGTM to minimizing overall breakage, for users of all > browsers, by breaking the sites in Chrome after a reasonable deprecation > period. > > Best regards, > Philip > > On Wed, Mar 9, 2022 at 4:04 PM Yoav Weiss <yoavwe...@chromium.org> wrote: > >> Thanks for the analysis! :) I think it may make sense to do a bit of a deeper >> dive >> <https://docs.google.com/document/d/1JUeNc-ZxxWTn2tz9M_DJ4cV-M8lBdp40xlnfQ2-Mhgg/edit?disco=AAAAVgAWwQ8> >> into HA data to get a tighter bound on usage before coming up with a date. >> >> On Thursday, March 3, 2022 at 9:23:50 PM UTC+1 Xiaocheng Hu wrote: >> >>> Hi blink-dev, >>> >>> I've done an analysis on HTTP Archive (https://bit.ly/3uHALsd), and >>> found that at least 0.2% pages will break if Event.path is removed. The >>> actual number should be higher (maybe 1%?) since the analysis is based on >>> searching the regex r'(ev|event)\.path' in JavaScript code, so the analysis >>> couldn't find a usage if the code has been aggressively minified with >>> variables renamed. >>> >>> My current plan is that we'll target M109, the first release in 2023. >>> Before its removal, we'll: >>> - Use on a deprecation message in DevTools to decrease the usage >>> - Do outreach if needed >>> - Starting from September 2022, or if the known breakage number drops >>> below a threshold (mabye 0.05%?), disable it on all non-Stable channels >>> (via finch) to further speed up decreasing the usage. >>> >>> On Wed, Feb 9, 2022 at 9:59 AM Xiaocheng Hu <xiaoche...@chromium.org> >>> wrote: >>> >>>> I'm still working on HTTP Archive to see the actual usage patterns of >>>> Event.path. I also suspect that many won't really break with Event.path >>>> removed, since I'm not aware of any Apple bugs, either. >>>> >>>> I'll update this thread when I'm done. >>>> >>>> On Wed, Feb 9, 2022 at 9:28 AM Mike Taylor <miketa...@chromium.org> >>>> wrote: >>>> >>>>> Thanks for digging into this Philip! Agree that HTTPArchive analysis >>>>> will be pretty helpful to help us make a decision. >>>>> >>>>> Thinking again about the lack of (many) Firefox and Safari bugs - that >>>>> gives me some hope that it might be safe to remove from Chromium (assuming >>>>> we don't find that sites are UA-sniffing to only use event.path for >>>>> Chromium browsers). >>>>> >>>>> On 2/9/22 11:46 AM, Philip Jägenstedt wrote: >>>>> >>>>> Here's a first cut at httparchive analysis: >>>>> >>>>> SELECT page, url >>>>> FROM `httparchive.latest.response_bodies_desktop` >>>>> WHERE STRPOS(body, 'event.path') > 0 >>>>> AND STRPOS(body, 'event.composedPath()') = 0; >>>>> >>>>> That finds over 32k cases. Categorizing these based on context and >>>>> analyzing how each case would break would really help here. >>>>> >>>>> The very first case I looked at was like this: >>>>> >>>>> function getRealEventTarget(event, ELEMENT) { >>>>> var path = [] >>>>> if (event.path) { >>>>> path = event.path >>>>> } >>>>> if (event.originalEvent && event.originalEvent.path) { >>>>> path = event.originalEvent.path >>>>> } >>>>> if (path && path.length > 0) { >>>>> if (ELEMENT && path.indexOf(ELEMENT) >= 0) { >>>>> return ELEMENT >>>>> } else { >>>>> return path[0] >>>>> } >>>>> } >>>>> return event.target >>>>> } >>>>> >>>>> At first glance this seems like a problem and that things could break, >>>>> but then we need to look at if it's already broken in Firefox and Safari. >>>>> It may well turn out that as actually used, it's harmless. >>>>> >>>>> Best regards, >>>>> Philip >>>>> >>>>> On Wed, Feb 9, 2022 at 5:37 PM Philip Jägenstedt <foo...@chromium.org> >>>>> wrote: >>>>> >>>>>> On Wed, Feb 9, 2022 at 10:50 AM Yoav Weiss <yoavwe...@chromium.org> >>>>>> wrote: >>>>>> >>>>>>> >>>>>>> >>>>>>> On Wednesday, February 9, 2022 at 2:49:55 AM UTC+1 Mike Taylor wrote: >>>>>>> >>>>>>>> Hey Xiaocheng, >>>>>>>> >>>>>>>> Thanks for working on improving interop! A few thoughts and >>>>>>>> questions below. >>>>>>>> >>>>>>>> On 2/8/22 7:25 PM, Xiaocheng Hu wrote: >>>>>>>> >>>>>>>> Contact emails xiaoche...@chromium.org >>>>>>>> >>>>>>>> Explainer None >>>>>>>> >>>>>>>> Specification None. Not a standard feature. >>>>>>>> >>>>>>>> Summary >>>>>>>> >>>>>>>> Event.path is a non-standard API that returns the event's path, >>>>>>>> which is an array of the objects on which listeners will be invoked. >>>>>>>> It is >>>>>>>> supported by Blink only, causing web compatibility issues. Web >>>>>>>> developers >>>>>>>> should switch to the equivalent standard API Event.composedPath(), >>>>>>>> which >>>>>>>> returns the same result. >>>>>>>> >>>>>>>> >>>>>>>> Blink component Blink>DOM >>>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EDOM> >>>>>>>> >>>>>>>> TAG review >>>>>>>> >>>>>>>> TAG review status Not applicable >>>>>>>> >>>>>>>> Risks >>>>>>>> >>>>>>>> >>>>>>>> Interoperability and Compatibility >>>>>>>> >>>>>>>> The removal of this API should improve interoperability, as it's >>>>>>>> supported by Blink only. It still has 18% usage as of Feb 2022 ( >>>>>>>> https://chromestatus.com/metrics/feature/timeline/popularity/345), >>>>>>>> so we will only deprecate it for now, and will not remove it before the >>>>>>>> usage drops low enough. We expect low compatibility risks, since there >>>>>>>> is >>>>>>>> an equivalent standard API (Event.composedPath()) by all browsers, and >>>>>>>> the >>>>>>>> following polyfill should also keep existing sites functioning with >>>>>>>> minimum >>>>>>>> changes: >>>>>>>> >>>>>>>> 18% is a _lot_ of usage. So much that I'm surprised there aren't >>>>>>>> dozens of compat bugs reported against Firefox. In >>>>>>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1697590#c0 there are >>>>>>>> only 2 linked site bugs. And there's only 3 in >>>>>>>> https://github.com/webcompat/web-bugs/issues?q=is%3Aissue+composedPath >>>>>>>> (the last one being from 2019). >>>>>>>> >>>>>>>> I wonder how much of that 18% is feature detection and fallback >>>>>>>> codepaths >>>>>>>> <https://github.com/search?l=JavaScript&q=composedPath+event.path&type=Code> >>>>>>>> . >>>>>>>> >>>>>>> >>>>>>> Yeah, that 18% is unreasonably high... >>>>>>> I think it'd be good to add a non-IDL based counter that counts when >>>>>>> both Event::path >>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/events/event.cc;l=287;bpv=1;bpt=0> >>>>>>> and >>>>>>> Event::composedPath >>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/dom/events/event.cc;l=293;bpv=1;bpt=0> >>>>>>> are >>>>>>> accessed. (e.g. by adding 2 booleans or a bitmask on Event indicating >>>>>>> when >>>>>>> any of them was touched and triggering the counter when both have been) >>>>>>> Then we can assume that counts feature-detection/fallback and >>>>>>> subtract that counter from the 18%. >>>>>>> >>>>>> >>>>>> Based on past cases, I'm pretty sure most of the usage here is due to >>>>>> event copying, where all of the properties of event instances are copied >>>>>> to >>>>>> a new object. So I think we should basically ignore the 18% number and >>>>>> treat the risk as unknown. >>>>>> >>>>>> A use counter that tries to better measure the risk here would be >>>>>> nice, but I actually don't see how to measure it. Consider this code: >>>>>> >>>>>> function(event) { >>>>>> var path = event.path; >>>>>> if (!path) { >>>>>> path = event.composedPath() >>>>>> } >>>>>> doStuffWith(path); >>>>>> } >>>>>> >>>>>> This would never hit the composedPath() code path, and is >>>>>> indistinguishable to code that only relies on event.path, from the point >>>>>> of >>>>>> view of use counter instrumentation. >>>>>> >>>>>> I think that httparchive analysis here is our best tool, and in fact >>>>>> that *only* httparchive analysis could be enough to convince us that >>>>>> there's no problem here, if we fail to find any breaking cases. But even >>>>>> then, we should probably have a fairly long deprecation period. >>>>>> >>>>>> If we go ahead and deprecate this, what do you think the deprecation >>>>>> message should say about eventual removal? >>>>>> >>>>>> Best regards, >>>>>> Philip >>>>>> >>>>> >>>>> -- 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/CAFqEGhYvk2is68e_DUtZ%3DgwxbONfoO0AQCn99w7DF%2BzLY-iw1Q%40mail.gmail.com.
