Thanks for doing this analysis. According to our guidelines <https://docs.google.com/document/d/1RC-pBBvsazYfCNNUSkPqAVpSpNJ96U8trhNkfV0v9fk/edit#heading=h.4k9u8ddizqrq> this is in the low risk range of <0.001% of pages in HTTP Archive. Personally I think we should just go for it, but be prepared to revert (and rename the interface instead) if we get even a single bug report in beta. LGTM1
On Wed, Nov 23, 2022 at 4:50 PM Ian Clelland <[email protected]> wrote: > So I've run through HTTPArchive with these queries (shown for desktop, > mobile is analogous): > > SELECT APPROX_COUNT_DISTINCT(pageid) > FROM `httparchive.summary_requests.2022_10_01_desktop` AS REQ > JOIN `httparchive.response_bodies.2022_10_01_desktop` AS RESP on REQ.url > = RESP.url > WHERE ( > REQ.resp_content_type = 'application/javascript' OR > REQ.resp_content_type = 'text/javascript' OR > REQ.resp_content_type = 'application/html' OR > REQ.resp_content_type = 'text/html') > AND ( > REGEXP_CONTAINS(RESP.body, r"\bwindow\.Reporting\b") OR > REGEXP_CONTAINS(RESP.body, r"\bself\.Reporting\b")); > > > And found 161 pages on desktop (out of ~10M pages), and 185 on mobile (of > ~15M). (For comparison, replacing "Report" with "ReportingObserver", yields > ~123k pages on desktop referring to that symbol) > > Coalescing distinct resource URLs in this case (as many may be from the > same third-party library) yields just 110 URLs on mobile, including many > clearly repeated resources hosted on different subdomains of kindful.com, > and > several named simply '/js/common.js': > > > https://cookerevivals.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js > > https://atlast.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js > > https://indushospitalca.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js > > https://stridereducationfoundation-bloom.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js > (And many more; these ones unconditionally overwrite Report) > > https://tesotunes.com/js/common.js?version=2.1.5.8-beta-22 > http://arabgramz.com/js/common.js?x=37 > https://www.yavedo.com/js/common.js?x=55 > https://shakulive.com/js/common.js?x44 > (These ones do check for Report's existence, and use "window.Report || ( > window.Report = {} );" before defining new methods. This will also continue > to work, but will add new methods to the interface object, rather than the > presumed empty dict.) > > On Wed, Nov 23, 2022 at 12:48 PM Rick Byers <[email protected]> wrote: > >> >> On Wed, Nov 23, 2022 at 12:22 PM Yoav Weiss <[email protected]> >> wrote: >> >>> >>> On Wed, Nov 23, 2022 at 6:13 PM Ian Clelland <[email protected]> >>> wrote: >>> >>>> Well, that is exactly the situation outlined as a risk. I'm still >>>> running some HTTPArchive queries, but didn't immediately see much there. >>>> >>>> I expect that +Domenic Denicola <[email protected]> may have more >>>> context on the changes here, but my understanding is that WebIDL has been >>>> moving towards this for some time. >>>> >>>> The issues I'm aware of are on WebIDL: >>>> https://github.com/whatwg/webidl/issues/350 renames >>>> "NoInterfaceObject" to "LegacyNoInterfaceObject" >>>> https://github.com/whatwg/webidl/issues/365 starts mandating the use >>>> of "Exposed" in all interfaces >>>> >>>> Reporting specifically was updated in 2019, with >>>> https://github.com/w3c/reporting/pull/173, after >>>> https://github.com/whatwg/webidl/pull/423 made Exposed mandatory. >>>> >>> >>> Is it possible to rename these exposed interfaces to something less >>> generic, that's less likely to collide? >>> >> >> Since it's not exposed today, nobody should be depending on the name. So >> I'm sure we could. >> >> But I'd like to understand why we should expose it at all. I tried >> digging through the history of discussions on this topic and the contents >> of the WebIDL spec but couldn't get clarity on what the thinking has been. >> I filed https://github.com/whatwg/webidl/issues/1236 to hopefully have >> that captured somewhere. >> >> On Wed, Nov 23, 2022 at 11:18 AM Rick Byers <[email protected]> wrote: >>>> >>>>> I did a quick GitHub search and quickly found this >>>>> <https://github.com/xuehuibo/MobileQueryPlatform/blob/master/MobileQueryPlatform/MobileQueryPlatform/Views/Home/ReportMenu.cshtml>: >>>>> :-( >>>>> >>>>> if (window.Report == undefined) { >>>>> Tools.Namespace.register("window.Report"); >>>>> } >>>>> >>>>> And then a bunch of downstream code expecting to build stuff up on >>>>> window.Report. I didn't try to find out where or how much this is used, >>>>> but >>>>> it's certainly not promising. >>>>> >>>>> Where is this decision to expose all interface objects by default >>>>> coming from? Are we sure it's the right thing for the web when the global >>>>> namespace is shared between the platform and application code? >>>>> >>>>> Sorry Ian, I know I said I thought this should be a simple one :-). If >>>>> we have data that it looks very rare, then I'm still happy to approve. I'm >>>>> just wondering now about the bigger picture of this class of changes. >>>>> >>>>> Rick >>>>> >>>>> On Wed, Nov 23, 2022 at 9:52 AM Yoav Weiss <[email protected]> >>>>> wrote: >>>>> >>>>>> As this is not actually a feature, but a spec alignment effort, I >>>>>> think it makes sense to skip TAG reviews and vendor positions for this. >>>>>> >>>>>> I'm slightly concerned about `Report` collisions. Would you be able >>>>>> to run a quick HTTPArchive scan for "window.Report" and "self.Report" and >>>>>> see what that gives? ("var Report" would just unconditionally override >>>>>> it, >>>>>> so that's not an issue) >>>>>> >>>>>> On Wed, Nov 23, 2022 at 2:55 PM Ian Clelland <[email protected]> >>>>>> wrote: >>>>>> >>>>>>> Contact [email protected] >>>>>>> >>>>>>> Specificationhttps://w3c.github.io/reporting/#idl-index >>>>>>> >>>>>>> Summary >>>>>>> >>>>>>> Several interface objects used by the Reporting API were originally >>>>>>> not exposed as symbols on the JavaScript global object. This includes >>>>>>> Report, ReportBody, and the various specific report body types such as >>>>>>> CSPViolationReportBody and DeprecationReportBody. This change exposes >>>>>>> those >>>>>>> objects on the window (or worker global), bringing the implementation in >>>>>>> line with the specification. >>>>>>> >>>>>>> >>>>>>> Blink componentBlink>ReportingObserver >>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EReportingObserver> >>>>>>> >>>>>>> TAG review >>>>>>> >>>>>>> TAG review statusNot applicable >>>>>>> >>>>>>> Risks >>>>>>> >>>>>>> >>>>>>> Interoperability and Compatibility >>>>>>> >>>>>>> The main risk here is that the newly-exposed symbols will collide >>>>>>> with symbols defined in existing scripts. Most of the names are quite >>>>>>> specific, but "Report" and "ReportBody" may have some existing use. >>>>>>> However, this is unlikely to be a problem in practice, as most code >>>>>>> which >>>>>>> uses these names for its own variables will also continue to work; >>>>>>> redefining them in a local script should not have any effect on either >>>>>>> the >>>>>>> script or the Reporting API. One possibility for breakage would be a >>>>>>> script >>>>>>> that defines, say, a "Report" object, (unrelated to the Reporting API,) >>>>>>> but >>>>>>> does so only if it did not previously exist. In that case, the >>>>>>> hypothetical >>>>>>> script would *not* define its own Report, and would presumably attempt >>>>>>> to >>>>>>> use the newly-exposed interface object. In order for this to be a real >>>>>>> issue, a script would have to: 1. Use one of these symbol names, down to >>>>>>> capitalization. I suspect that "Report" is the most probable, with the >>>>>>> various ReportBody types being far less likely candidates for collision >>>>>>> 2. >>>>>>> Specifically guard against re-defining the symbol, even though it's not >>>>>>> currently platform-exposed anywhere. This would likely only occur if >>>>>>> some >>>>>>> initialization code was expected to be run multiple times blindly. >>>>>>> (Blink's >>>>>>> web test result page, for instance, uses "Report" as an object name, but >>>>>>> defines it unconditionally, so it will simply shadow the interface >>>>>>> object.) >>>>>>> >>>>>>> >>>>>>> *Gecko*: No signal >>>>>>> >>>>>>> *WebKit*: No signal >>>>>>> >>>>>>> *Web developers*: No signals >>>>>>> >>>>>>> *Other signals*: >>>>>>> >>>>>>> Ergonomics >>>>>>> >>>>>>> None >>>>>>> >>>>>>> >>>>>>> Activation >>>>>>> >>>>>>> None >>>>>>> >>>>>>> >>>>>>> Security >>>>>>> >>>>>>> None >>>>>>> >>>>>>> >>>>>>> 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? >>>>>>> >>>>>>> >>>>>>> >>>>>>> Debuggability >>>>>>> >>>>>>> >>>>>>> >>>>>>> Will this feature be supported on all six Blink platforms (Windows, >>>>>>> Mac, Linux, Chrome OS, Android, and Android WebView)?Yes >>>>>>> >>>>>>> This is a change to the IDL for these interfaces, and is >>>>>>> automatically supported by all Blink platforms. >>>>>>> >>>>>>> >>>>>>> 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 >>>>>>> >>>>>>> Requires code in //chrome?False >>>>>>> >>>>>>> Tracking bug >>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1122656 >>>>>>> >>>>>>> Estimated milestones >>>>>>> >>>>>>> M110 >>>>>>> >>>>>>> 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). >>>>>>> >>>>>>> >>>>>>> Link to entry on the Chrome Platform Status >>>>>>> https://chromestatus.com/feature/5977883141472256 >>>>>>> >>>>>>> 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 [email protected]. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAK_TSXKUf6wAHB-3EdgP04%2Bcmhgis3j3M54B%3DASXGtHVJcenDQ%40mail.gmail.com >>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAK_TSXKUf6wAHB-3EdgP04%2Bcmhgis3j3M54B%3DASXGtHVJcenDQ%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 [email protected]. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfV3b3BnY26B6tuGtFFYRm-K5P0U6b5_giLgB1h6PeK7ag%40mail.gmail.com >>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfV3b3BnY26B6tuGtFFYRm-K5P0U6b5_giLgB1h6PeK7ag%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 [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY8pXY9nUDxRwXTheVUCf0A81cZyfJLE7WDA%2B4%3Dqf-DiJQ%40mail.gmail.com.
