Re: [blink-dev] Intent to Ship: Expose Reporting API interfaces to JavaScript

Ian Clelland

unread,

Feb 13, 2023, 11:12:45 AM2/13/23

to blink-api-owners-discuss

I've been working on the transition to dict, but failed to revert the initial exposure, which we believed was unlikely to have any real compat issues.

That seems to be wrong, as https://bugs.chromium.org/p/chromium/issues/detail?id=1414463 was filed recently saying that it breaks an enterprise use case. I'm going to revert the original change immediately, but unfortunately we did allow this to ship in M110.

I'd appreciate any guidance here from API owners for the best next steps to take to mitigate this; I can merge back to M111, and can propose a merge back to M110 for the next point release if there's sufficient justification there.

Thanks,

Ian

On Fri, Nov 25, 2022 at 1:51 PM Ian Clelland <icle...@chromium.org> wrote:

On Fri, Nov 25, 2022 at 12:03 PM Manuel Rego Casasnovas <re...@igalia.com> wrote:
WebKit has been doing some work implementing the Reporting API:
https://bugs.webkit.org/show_bug.cgi?id=189365

Do we know their plans regarding this topic? Should we ask for signals?

Thanks Rego --

The last time I heard, Chris Dumez had been working on an implementation, but it didn't include ReportingObserver. I'll reach out to see if that's changed, and I suppose it can't hurt to file for an official signal, now that they've implemented that as a repository.

Ian

Cheers,
Rego

On 25/11/2022 15:39, Rick Byers wrote:
> On Fri, Nov 25, 2022 at 9:27 AM Ian Clelland <icle...@chromium.org
> <mailto:icle...@chromium.org>> wrote:
>
>
>
> On Fri, Nov 25, 2022 at 1:54 AM Domenic Denicola
> <dom...@chromium.org <mailto:dom...@chromium.org>> wrote:
>
>
>
> On Fri, Nov 25, 2022, 14:53 Yoav Weiss <yoav...@chromium.org
> <mailto:yoav...@chromium.org>> wrote:
>
>
> On Fri, Nov 25, 2022 at 5:50 AM Domenic Denicola
> <dom...@chromium.org <mailto:dom...@chromium.org>> wrote:
>
> Note that in the past I've suggested these not be
> interfaces at
> all: https://github.com/w3c/reporting/issues/216
> <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
> <https://w3c.github.io/reporting/#ref-for-dom-report>
> ReportBody: https://w3c.github.io/reporting/#reportbody
> <https://w3c.github.io/reporting/#reportbody>
> CSPViolationReportBody: https://www.w3.org/TR/CSP3/#cspviolationreportbody <https://www.w3.org/TR/CSP3/#cspviolationreportbody>
> DeprecationReportBody: https://wicg.github.io/deprecation-reporting/#deprecationreportbody <https://wicg.github.io/deprecation-reporting/#deprecationreportbody>
> InterventionReportBody: https://wicg.github.io/intervention-reporting/#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
> <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
> <chri...@chromium.org
> <mailto:chri...@chromium.org>> wrote:
>
> LGTM2
>
> On Wed, Nov 23, 2022, 4:14 PM Rick Byers
> <rby...@chromium.org
> <mailto:rby...@chromium.org>> 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
> <icle...@chromium.org
> <mailto:icle...@chromium.org>> wrote:
>
> So I've run through HTTPArchive with
> these queries (shown for desktop, mobile
> is analogous):
>
> SELECTAPPROX_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
> <http://kindful.com>, and
> several named simply '/js/common.js':
>
> https://cookerevivals.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js <https://cookerevivals.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js>
> https://atlast.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js <https://atlast.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js>
> https://indushospitalca.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js <https://indushospitalca.kindful.com/assets/application-b1789711faae06cb6b1679d08f18c4ee8bfde712fa6375641c5e38fca8231494.js>
> https://stridereducationfoundation-bloom.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 <https://tesotunes.com/js/common.js?version=2.1.5.8-beta-22>
> http://arabgramz.com/js/common.js?x=37
> <http://arabgramz.com/js/common.js?x=37>
> https://www.yavedo.com/js/common.js?x=55
> <https://www.yavedo.com/js/common.js?x=55>
> https://shakulive.com/js/common.js?x44
> <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 <rby...@chromium.org
> <mailto:rby...@chromium.org>> wrote:
>
>
> On Wed, Nov 23, 2022 at 12:22 PM
> Yoav Weiss <yoav...@chromium.org
> <mailto:yoav...@chromium.org>> wrote:
>
>
> On Wed, Nov 23, 2022 at 6:13 PM
> Ian Clelland
> <icle...@chromium.org
> <mailto:icle...@chromium.org>>
> 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
> <mailto:dom...@google.com> 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 <https://github.com/whatwg/webidl/issues/350> renames "NoInterfaceObject" to "LegacyNoInterfaceObject"
> https://github.com/whatwg/webidl/issues/365 <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 <https://github.com/w3c/reporting/pull/173>, after https://github.com/whatwg/webidl/pull/423 <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 <https://github.com/whatwg/webidl/issues/1236> to hopefully have that captured somewhere.
>
> On Wed, Nov 23, 2022 at
> 11:18 AM Rick Byers
> <rby...@chromium.org
> <mailto:rby...@chromium.org>> 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
> <yoav...@chromium.org
> <mailto:yoav...@chromium.org>> 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
> <icle...@chromium.org <mailto:icle...@chromium.org>> wrote:
>
>
> Contact
> emails
>
> icle...@chromium.org <mailto:icle...@chromium.org>
>
>
> Specification
>
> https://w3c.github.io/reporting/#idl-index <https://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
> component
>
> Blink>ReportingObserver <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EReportingObserver>
>
>
> TAG review
>
>
>
> TAG
> review
> status
>
> Not 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 <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 <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
> blink-dev+...@chromium.org <mailto:blink-dev+...@chromium.org>.
> 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
> blink-dev+...@chromium.org <mailto:blink-dev+...@chromium.org>.
> 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
> blink-dev+...@chromium.org
> <mailto:blink-dev+...@chromium.org>.
> 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 blink-dev+...@chromium.org
> <mailto:blink-dev+...@chromium.org>.
> 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 <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFUtAY9QQoXMpH8ex%3DYJW0-3YATGXQj%3DZSuYWNXP5Pi5ciCpMg%40mail.gmail.com?utm_medium=email&utm_source=footer>.

Rick Byers

unread,

Feb 13, 2023, 12:39:55 PM2/13/23

to Ian Clelland, Brandon Heenan, Paul Kinlan, blink-api-owners-discuss

Thanks Ian.

Ian and I chatted briefly in the hallway and I suggested requesting merge to 110 since the revert is very low risk. I don't think there's sufficient justification yet to ask for a respin, but hopefully there will be some other reason to do a respin and we can get the revert out to partners sooner rather than later.

Once again it's sad to see how much breakage can get reported the day something lands in stable without hearing a peep from beta users. In general this is a case of shipping a new API to the web breaking some very brittle code which depends on such an API not existing, sadly not something I think we can measure or defend thoroughly against in any reasonable way. +Brandon Heenan from the enterprise team as FYI and in case he has thoughts.

In this specific case of course we did have some reason to be concerned given the generic nature of the API's name ("Report") and the one example I found on GitHub. Perhaps this should raise our concern for shipping any new generic-sounding API name? Should we perhaps have a test which tracks a list of common global names which we've learned we can never ship ourselves? And/or perhaps we should do more evangelism for avoiding coding patterns which depend on the non-existence of any property on Window? With DevTools moving to a ChatGPT interface (*) perhaps we can learn how to detect and warn against the most common such patterns? +Paul Kinlan

Rick

(*) Just kidding... For now....

You received this message because you are subscribed to the Google Groups "blink-api-owners-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-api-owners-d...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-api-owners-discuss/CAK_TSXKUNQ30O0x%2BeGTdOUAueoG%3D33mkhw%2BiF7mEfHcK_xkXsQ%40mail.gmail.com.

Rick Byers

unread,

Feb 13, 2023, 5:21:37 PM2/13/23

to Ian Clelland, Brandon Heenan, Paul Kinlan, Domenic Denicola, blink-api-owners-discuss

Chatting more with Ian I realized I had made assumptions about the coding pattern that was impacted. In this case it was "<iframe name=Report>" and using window.Report as an alias for window.frames["Report"]. I admit that I hadn't even appreciated the risk of this pattern myself in the original I2R thread for the breaking change. Can we even point to any guidance anywhere that says to prefer getElementById/getElementsByName over relying on legacy window properties due to the inherent brittleness of the latter? I just did a quick google search and couldn't find anything.

We could debate whether perhaps new APIs could be tagged in a way that id/name takes precedence? At least this is something we could have, in theory, measured an upper bound of risk for (UseCounter for DOM element having an ID or name of "Report"). We might even be able to use UKM to anonymously collect a hashed set of id/name values that we check all new API names against.

Thoughts on the meta-problem for the inevitable post-mortem? +Domenic Denicola who I imagine has a perspective.

It looks like there will be a respin with the revert, so the immediate question of what to do is answered. But I'd like to brainstorm more here about how to reduce the risk of a repeat in the future.

Rick

Ian Clelland

unread,

Feb 13, 2023, 5:55:09 PM2/13/23

to Rick Byers, Brandon Heenan, Paul Kinlan, Domenic Denicola, blink-api-owners-discuss

On Mon, Feb 13, 2023 at 5:21 PM Rick Byers <rby...@chromium.org> wrote:

Chatting more with Ian I realized I had made assumptions about the coding pattern that was impacted. In this case it was "<iframe name=Report>" and using window.Report as an alias for window.frames["Report"]. I admit that I hadn't even appreciated the risk of this pattern myself in the original I2R thread for the breaking change. Can we even point to any guidance anywhere that says to prefer getElementById/getElementsByName over relying on legacy window properties due to the inherent brittleness of the latter? I just did a quick google search and couldn't find anything.

I was surprised by that as well; I was expecting any usage to be in user-defined JS symbols, which would automatically and harmlessly shadow any IDL interface objects. In this case, though, the named property lookup is lower-precedence than any other namespace, so a collision with any other defined symbol causes the code to fail.

There is a warning in the HTML spec about this: under https://html.spec.whatwg.org/#named-access-on-the-window-object, this note appears:

"As a general rule, relying on this will lead to brittle code. Which IDs end up mapping to this API can vary over time, as new features are added to the web platform, for example. Instead of this, use document.getElementById() or document.querySelector()."

That's pretty deep in the spec, though, and I don't expect that anyone using this particular feature (which I believe predates any DOM apis) will be looking in the HTML spec for guidance.

Brandon Heenan

unread,

Feb 14, 2023, 2:17:06 PM2/14/23

to Ian Clelland, cbe-launch-approvers, M K, Rick Byers, Paul Kinlan, Domenic Denicola, blink-api-owners-discuss

Thanks for looping me in for the enterprise perspective (+cbe-launch-approvers fyi). Here's some information/thoughts:

Can we avoid these in the first place?

Like others here, my first inclination is to think through what processes or best practices we might have in place within Chromium to stop this situation from happening again. Other than not shipping any new APIs, I don't see any obvious answers. We already use the enterprise release notes to advertise risky or breaking changes, and recommend enterprise policies to roll them back. However, we don't do those things for new, additive APIs in the web platform, so it's no surprise that that didn't happen here. It also doesn't feel like a good tradeoff to have enterprise policies to disable every new API as it's introduced just to handle a case like this.

This type of problem may only be possible to address with larger efforts, like targeting API versions, or something like <meta name=compatible-with-browsers-before value=2023-02-01>. That's a big conversation, but planting seeds here.

Can we get bug reports before stable ships?

I can think of 3 possible answers here:

We ourselves may be able to lower the risk by running tests with common enterprise tools like SAP's various solutions. The problem: we already do this to some extent (e.g. Microsoft Active Directory and Group Policy), but it doesn't stop these issues from popping up from new products and new configurations of products that are part of tests. We'll never realistically get anything close to great coverage of everything enterprises are doing. But, we may still be able to do something to somewhat lower the risk. @M K maybe you have thoughts here.
Customers running beta. We actually have had some historical success convincing customers about the value of doing this. In fact, crbug/1413028 may be an example of a bug that came in from a customer before stable fully shipped. @Rick Byers or someone else on the thread, does that look like it's the same root cause?
SAP itself running beta regularly. This strikes me as the most reliable way of finding these issues before stable. Years ago, we had a relationship with SAP through ChromeOS, but that hasn't existed for years now. @Paul Kinlan, is there anyone on your team who has a relationship with SAP? I have had difficulties finding workable solutions with SAP in the past, but this is probably worth trying.

Ian Clelland

unread,

Feb 14, 2023, 2:31:02 PM2/14/23

to Brandon Heenan, Ian Clelland, cbe-launch-approvers, M K, Rick Byers, Paul Kinlan, Domenic Denicola, blink-api-owners-discuss

On Tue, Feb 14, 2023 at 2:17 PM Brandon Heenan <bhe...@google.com> wrote:

Thanks for looping me in for the enterprise perspective (+cbe-launch-approvers fyi). Here's some information/thoughts:

Can we avoid these in the first place?
Like others here, my first inclination is to think through what processes or best practices we might have in place within Chromium to stop this situation from happening again. Other than not shipping any new APIs, I don't see any obvious answers. We already use the enterprise release notes to advertise risky or breaking changes, and recommend enterprise policies to roll them back. However, we don't do those things for new, additive APIs in the web platform, so it's no surprise that that didn't happen here. It also doesn't feel like a good tradeoff to have enterprise policies to disable every new API as it's introduced just to handle a case like this.

This type of problem may only be possible to address with larger efforts, like targeting API versions, or something like <meta name=compatible-with-browsers-before value=2023-02-01>. That's a big conversation, but planting seeds here.

Can we get bug reports before stable ships?
I can think of 3 possible answers here:
We ourselves may be able to lower the risk by running tests with common enterprise tools like SAP's various solutions. The problem: we already do this to some extent (e.g. Microsoft Active Directory and Group Policy), but it doesn't stop these issues from popping up from new products and new configurations of products that are part of tests. We'll never realistically get anything close to great coverage of everything enterprises are doing. But, we may still be able to do something to somewhat lower the risk. @M K maybe you have thoughts here.
Customers running beta. We actually have had some historical success convincing customers about the value of doing this. In fact, crbug/1413028 may be an example of a bug that came in from a customer before stable fully shipped. @Rick Byers or someone else on the thread, does that look like it's the same root cause?

The last couple of comments on that bug suggest that it might be, but there is no evidence in the screen shots or the initial report to back that up; hopefully the reporter can try the fix on Canary or on the recently-pushed 110.0.5481.100 and we'll know for sure. If so, it's unfortunate that that issue wasn't investigated in time to catch the bug before it hit stable.

Paul Kinlan

unread,

Feb 14, 2023, 3:55:58 PM2/14/23

to Ian Clelland, Brandon Heenan, cbe-launch-approvers, M K, Rick Byers, Domenic Denicola, blink-api-owners-discuss

I'll reach out to people I know at SAP.

Rick Byers

unread,

Feb 14, 2023, 4:18:49 PM2/14/23

to Paul Kinlan, Ian Clelland, Brandon Heenan, cbe-launch-approvers, M K, Domenic Denicola, blink-api-owners-discuss

A few things. First clearly I screwed up here by saying I thought the HTTP Archive data was enough to try shipping, and being prepared to revert if we saw any data in beta. That may have been true if we had the change guarded by a finch killswitch. I'm sorry for giving you poor advice on this Ian. I've made some policy doc updates and started a discussion here.

Ian, any thoughts on how hard wiring up RuntimeEnabledFeatures would have been in this case? Eg. would you have had to modify the bindings generator to connect it to LegacyNoInterfaceObject? If we have a bunch more APIs to remove LegacyNoInterfaceObject on then we could consider making it a lot easier to do this.

Brandon, it's great to hear that SAP tests on beta and that bug is intriguing. If it does turn out to be the same issue then I think the failure was perhaps folks speculating on possible causes when a simple bisect would have pointed directly at the cause and likely gotten it reverted (though not sure if that would have made the stable cut based on information available at that time). Any idea what we can do to ensure we reliably get bisects in issues like that? Obviously that relies on someone having access to a repro (which wasn't present on the bug) but anyone can do a coarse grained bisect. IIRC the per-revision build archive is still google internal only, so getting it down to a single CL probably requires a googler who can repro.

Since it's such a footgun, I wonder if we should try to start going down the path of deprecating the practice of putting DOM IDs and names onto the window object? Obviously that could be a scary breaking change, but perhaps we could at least define an opt-in document policy that's best practice for sites to ensure they're not relying on it?

Rick

Brandon Heenan

unread,

Feb 14, 2023, 6:56:57 PM2/14/23

to Rick Byers, Paul Kinlan, Ian Clelland, cbe-launch-approvers, M K, Domenic Denicola, blink-api-owners-discuss

> it's great to hear that SAP tests on beta

Well, to be clear, that bug is a customer using SAP's products, not SAP themselves. I'm not aware of SAP doing any testing on the beta channel (though hopefully Paul's inquiry will find something like that).

> the failure was perhaps folks speculating on possible causes when a simple bisect would have pointed directly at the cause ... that relies on someone having access to a repro.

This is perhaps a good lesson for the enterprise team. Proposed postmortem item: Improve the playbook for the customer team supporting enterprises to run bisects. I suspect this is easier said than done, because customers may be resistant to running scripts in their environment, and it may be really hard to get a simpler repro than "Use [SAP Product] configured to our specific settings". But, it's still worth looking at, I'm sure we can do better than what we have today.

Btw, is there already a postmortem for this?

Domenic Denicola

unread,

Feb 14, 2023, 8:46:24 PM2/14/23

to Rick Byers, Paul Kinlan, Ian Clelland, Brandon Heenan, cbe-launch-approvers, M K, Domenic Denicola, blink-api-owners-discuss

On Wed, Feb 15, 2023 at 6:18 AM Rick Byers <rby...@chromium.org> wrote:

A few things. First clearly I screwed up here by saying I thought the HTTP Archive data was enough to try shipping, and being prepared to revert if we saw any data in beta. That may have been true if we had the change guarded by a finch killswitch. I'm sorry for giving you poor advice on this Ian. I've made some policy doc updates and started a discussion here.

Ian, any thoughts on how hard wiring up RuntimeEnabledFeatures would have been in this case? Eg. would you have had to modify the bindings generator to connect it to LegacyNoInterfaceObject? If we have a bunch more APIs to remove LegacyNoInterfaceObject on then we could consider making it a lot easier to do this.

Brandon, it's great to hear that SAP tests on beta and that bug is intriguing. If it does turn out to be the same issue then I think the failure was perhaps folks speculating on possible causes when a simple bisect would have pointed directly at the cause and likely gotten it reverted (though not sure if that would have made the stable cut based on information available at that time). Any idea what we can do to ensure we reliably get bisects in issues like that? Obviously that relies on someone having access to a repro (which wasn't present on the bug) but anyone can do a coarse grained bisect. IIRC the per-revision build archive is still google internal only, so getting it down to a single CL probably requires a googler who can repro.

Since it's such a footgun, I wonder if we should try to start going down the path of deprecating the practice of putting DOM IDs and names onto the window object? Obviously that could be a scary breaking change, but perhaps we could at least define an opt-in document policy that's best practice for sites to ensure they're not relying on it?

Very related issues have been discussed before in https://github.com/w3c/webappsec-permissions-policy/issues/349 , https://github.com/WICG/document-policy/issues/16 , and https://github.com/whatwg/html/issues/2212 , usually in the context of XSS protection. I think it's worth exploring, and would be a good fit for document policy.

Ian Clelland

unread,

Feb 15, 2023, 1:59:12 PM2/15/23

to Brandon Heenan, Rick Byers, Paul Kinlan, Ian Clelland, cbe-launch-approvers, M K, Domenic Denicola, blink-api-owners-discuss

On Tue, Feb 14, 2023 at 6:57 PM Brandon Heenan <bhe...@google.com> wrote:

> it's great to hear that SAP tests on beta
Well, to be clear, that bug is a customer using SAP's products, not SAP themselves. I'm not aware of SAP doing any testing on the beta channel (though hopefully Paul's inquiry will find something like that).

> the failure was perhaps folks speculating on possible causes when a simple bisect would have pointed directly at the cause ... that relies on someone having access to a repro.
This is perhaps a good lesson for the enterprise team. Proposed postmortem item: Improve the playbook for the customer team supporting enterprises to run bisects. I suspect this is easier said than done, because customers may be resistant to running scripts in their environment, and it may be really hard to get a simpler repro than "Use [SAP Product] configured to our specific settings". But, it's still worth looking at, I'm sure we can do better than what we have today.

Btw, is there already a postmortem for this?

Not yet; postmortem is underway though.

On Tue, Feb 14, 2023 at 1:18 PM Rick Byers <rby...@chromium.org> wrote:
A few things. First clearly I screwed up here by saying I thought the HTTP Archive data was enough to try shipping, and being prepared to revert if we saw any data in beta. That may have been true if we had the change guarded by a finch killswitch. I'm sorry for giving you poor advice on this Ian. I've made some policy doc updates and started a discussion here.

Ian, any thoughts on how hard wiring up RuntimeEnabledFeatures would have been in this case? Eg. would you have had to modify the bindings generator to connect it to LegacyNoInterfaceObject? If we have a bunch more APIs to remove LegacyNoInterfaceObject on then we could consider making it a lot easier to do this.

Yes, I believe that we would have needed a way to make NoInterfaceObject conditional on a runtime flag, and had that flag connected to a finchable feature so that it could have been rolled back remotely.

There are ~100 of these still left in the codebase; I expect that the goal is to eventually remove them all, so it might be worth adding some infrastructure to ensure that we can do that carefully. Most of them are likely much less risky than "Report" as a name, but I wouldn't want to get hit by this again.

Christoph Gnodtke

unread,

Feb 16, 2023, 4:46:21 AM2/16/23

to blink-api-owners-discuss, Ian Clelland, Rick Byers, Paul Kinlan, cbe-launch-approvers, M K, Domenic Denicola, blink-api-owners-discuss, Brandon Heenan

Hi Ian,

SAP BI customer here. I've been in the SAP BI space for almost 20 years now, both as an SAP employee supporting the product but much longer on customer side doing implementations.

I was fortunate that my current employer is using some Chrome Enterprise release stream (?) and is only on a 108 version. We are also using Chrome Beta for verifications and have a 110 version there. So, we didn't get hit by this issue at all. If we had been, I would have been one of the shouting and screaming customers of SAP. :-)

Saying that, although the problem came from Chrome here, I would have shouted equally at SAP.

First for using there old style code base and adressing the iframe in this old style (I think it was common around 2003 to do it this way?)

Secondly, for not implementing proper testing on Beta versions of browsers.

The workflow that failed here is a basic workflow in the product. It's not something hidden somewhere deep, it's a basic functionality. Not having such testcase as a vendor is terrible.

I saw ideas around re-enabling partnership with SAP for testing. I do have some contacts in the SAP BI area, I also know their product manager for the specific SAP BI product that caused the noise here. This will not cover all SAP of course but maybe it could be a starting point.

Last but not least, big thanks for your work and the quick turnaround. I know it doesn't help much, but I think the problem here was more on SAPs old codebase and having no testcases. You've saved the day and the month for many SAP BI engineers and implementers where actually SAP should have stepped in. SAP is also planning to release a fix but it's not anytime soon and it will take people a lot of time to have that installed.

It's very impressive to me how you managed to roll back your change, merge, build, test, release in such a short time. This might all seem normal to you but to me it's exceptional. Thanks for that!

Christoph

Reply all

Reply to author

Forward