🌸 Cherry pick request for #27783 into #27625/#27765 (Approved)

[danielrozenberg]

Cherry-pick request

Issue	PR	Beta / Experimental?	Stable?	LTS?	Release issue
#27775	#27783	YES	YES	NO	Stable: #27625 | Beta/Experimental: #27765

Why does this issue meet the cherry-pick criteria?

Issue #27775 was a serious breaking change that caused us to roll back Stable to a previous version.

Mini-postmortem

Summary

The Custom Elements v1 specification requires that all CEs extend from HTMLElement. This is expressed in code as class MyCustomElement extends HTMLElement {}. In browsers that don't support CEv1 natively, we provide a polyfill. Part of this polyfill is installing a new constructor HTMLElementPolyfill that extends from native HTMLElement, then replacing window.HTMLElement = HTMLElementPolyfill.

In order to do proper subclassing, the polyfill sets HTMLElementPolyfill's prototype chain to HTMLElement (this is how native subclassing always works). This means Object.getPrototypeOf(HTMLElementPolyfill) === HTMLElement and Object.getPrototypeOf(HTMLElementPolyfill.prototype) === HTMLElement.prototype.

In IE11, the HTMLElement constructor is a "host object", and not a regular function. Importantly, it does not inherit the usual Function methods .call, .apply, etc.

When we transpile our class syntax, we emit code like superClass.call(this) (this is how you instantiate class instances in ES5), where superClass === HTMLElementPolyfill. Remember that .call is usually inherited from Function. But because HTMLElementPolyfill extends from HTMLElement, and HTMLElement doesn't have a .call, that would cause a "method not defined" error. To fix that, we manually copied HTMLElementPolyfill.call = Function.call in #25245.

We could always depend on the code transpiling to superClass.call(this, this) do to our subclass's constructor super(this) call. But I eliminated this subclass constructor code in https://github.com/ampproject/amphtml/pull/27470/files#diff-8e1cdba946c07a7e631ef74645a5d6f0L100-L102.

When a subclass does not have its own constructor, one is provided by default. This default constructor is equivalent to super(...arguments) and transpiles down to superClass.apply(this, arguments). The is because now the superclass's constructor will receive all arguments, instead of receiving only a single argument as in our old code.

Unfortunately, HTMLElementPolyfill does not have a .apply method, just like it didn't have a .call method. Thus, the new default subclass constructor will throw an error.

Impact

• All AMP elements didn't load on IE11, most noticeably <amp-img>
• Non AMP elements continue to function

Action Items

• ✅ Enable simple integration test in IE11 #27785: Added integration test
• Release infra: fail on reverted commits #27771: Prevent publishing releases with commits that are not on master or were cherry-picked from master
• I2I: Maintaining AMP version numbers for cherry-picked releases #27793: @danielrozenberg add I2I for changing cherry-pick version numbers, to ease debugging

---

/cc @ampproject/wg-approvers @ampproject/cherry-pick-approvers

[dvoytenko]

Approved

[jridgewell]

Post mortem is added.

[dreamofabear]

@jridgewell You also pointed out was that our error reporter actually caught this. However it was already a hectic release week so it went unnoticed. A more automated alerting mechanism for new errors would help prevent these from slipping under the radar.