Note that in the past I've suggested these not be interfaces at all: https://github.com/w3c/reporting/issues/216 . As far as I can tell that issue is still open, and it would have been better to resolve it by making everything use dictionaries (or even just non-dictionary objects, like several specs do today) instead of creating new interfaces against proposed W3C TAG Design Principles <https://github.com/w3ctag/design-principles/issues/11>.
Also that there is no spec for several of these interfaces (at least CoopAccessViolationReportBody, possibly others). So I think there's considerable interop risk. I'd like the API Owners to reconsider their LGTMs in light of these unresolved discussions. At the very least, I think at TAG review is warranted for this, as there are architectural implications here. But fixing the spec situation would be even better. On Thursday, November 24, 2022 at 3:01:12 PM UTC+9 Yoav Weiss wrote: > LGTM3 > > On Wed, Nov 23, 2022 at 11:28 PM Chris Harrelson <[email protected]> > wrote: > >> LGTM2 >> >> On Wed, Nov 23, 2022, 4:14 PM Rick Byers <[email protected]> wrote: >> >>> 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 >>> >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY8pXY9nUDxRwXTheVUCf0A81cZyfJLE7WDA%2B4%3Dqf-DiJQ%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/128d14ec-3ca9-4274-ad15-1e82a7e9a0afn%40chromium.org.
