On Fri, Nov 25, 2022 at 1:54 AM Domenic Denicola <[email protected]> wrote:
> > > On Fri, Nov 25, 2022, 14:53 Yoav Weiss <[email protected]> wrote: > >> >> On Fri, Nov 25, 2022 at 5:50 AM Domenic Denicola <[email protected]> >> wrote: >> >>> 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>. >> >> >> Thanks Domenic. Turning those into dictionaries does sound appealing. >> Let's wait to hear what Ian says. >> >> >>> >>> Also that there is no spec for several of these interfaces (at >>> least CoopAccessViolationReportBody, possibly others). So I think there's >>> considerable interop risk. >>> >> >> That interop risk is orthogonal to whether we expose those interfaces or >> not, right? Not saying it shouldn't be fixed, just that the risk exists >> already. >> > > I don't think it's orthogonal. Once the interfaces are exposed, they're > much easier to depend on the existence of, and given that there is no spec > for their shape or behavior, such dependence seems like an Interop risk. > Most of these are spec'd: Report: https://w3c.github.io/reporting/#ref-for-dom-report ReportBody: https://w3c.github.io/reporting/#reportbody CSPViolationReportBody: https://www.w3.org/TR/CSP3/#cspviolationreportbody DeprecationReportBody: https://wicg.github.io/deprecation-reporting/#deprecationreportbody InterventionReportBody: https://wicg.github.io/intervention-reporting/#interventionreportbody CoopAccessViolationReportBody and DocumentPolicyViolationReportBody are not. CrashReportBody *is*, though not usefully, as it's not observable. I'm definitely not against turning these into WebIDL dictionaries, as they don't actually have any behaviour - they're just a collection of named data. They were originally defined as interfaces, I believe, in order to spec the constraints on the names and types of their data members. Using plain objects at the time would have made that more difficult. As I understand it, WebIDL dictionaries do not expose any symbols on the global namespace, so that would remove the compatibility concern. 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. >>> >> I'll revert the change for now, and take a pass at changing the specs involved. Thanks for weighing in, Domenic! > >>> 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/CAK_TSX%2BAT3%2BOrL36LSbOPVHfM9sH8rHYsGjxu5YYpbzX%3DrepkA%40mail.gmail.com.
