On Fri, Nov 25, 2022 at 9:27 AM Ian Clelland <[email protected]> wrote:
> > > 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. > We definitely shouldn't be exposing interface objects that aren't spec'd. Thanks for catching this Domenic. 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! > Saying this intent is on hold until https://github.com/w3c/reporting/issues/216 is resolved one way or the other makes sense to me. Let's take the design debate there. I have some additional questions / concerns but blink-dev isn't the right forum for API design discussions. > 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/CAFUtAY9QQoXMpH8ex%3DYJW0-3YATGXQj%3DZSuYWNXP5Pi5ciCpMg%40mail.gmail.com.
