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'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/CAL5BFfUM%3DQ6YzqSPiBdKb3ZAVyj%2BktNid95_bzHL_BJ%2BA7Yg5Q%40mail.gmail.com.
