Amphtml: 🌸 Cherry-pick request for #27837 into 2004161906340 (Approved)

[Enriqe]

Cherry-pick request

| Issue | PR | Beta / Experimental? | Stable? | LTS? | Release issue |
| :---------------: | :------------: | :------------------: | :----------: | :----------: | ------------------------------------------------------------------------------- |
| #27826 | #27837 | YES | YES | NO | Stable: #27625 \| Beta/Experimental: #27765 |

Why does this issue meet the cherry-pick criteria?

A button present on all pages of all stories is completely unresponsive on all mobile phones, blocking a critical user journey (sharing)

Mini-postmortem

Summary

AMP Stories' sharing button was broken (tapping on it wouldn't do anything).

#27533 moved the assignment of href from being set in the buildCallback() to being set in layoutCallback() in AmpSocialShare.

In amp-story, we append a hidden amp-story-share component here
https://github.com/ampproject/amphtml/blob/7d820920d6c9d890b0f15e6d69a721dfde92ea9e/extensions/amp-story/1.0/amp-story-share-menu.js#L137

Since it's hidden, AmpSocialShare.layoutCallback() never gets called and href was never set.

Where did things go wrong?

• There were no integration / e2e tests in amp-story that tested this functionality. Only unit tests.
• Stories didn't expect AmpSocialShare to perform relevant actions in the layoutCallback, we shouldn't use components under these assumptions anymore.
• The UI team (rightfully) authored and approved the offending PR, and they had no context on how the Stories team used it. Perhaps if the Stories team had been included to the PR they could've caught the issue.

Action items - What could've been done to prevent it?

• #27886: Add integration / e2e tests.
• As much as possible, try to avoid "hacky" ways of using components (skipping important lifecycle events).
• #27844: Require unanimous OWNERs approval for components enabled in multiple formats

Event log

• Release 2004161906340 promoted to Stable channel (Thursday April 16, 2020, 2:38:57 PM PST)
• Issue was reported in slack channel (#27826) Friday April 17, 2020 6:46am PST
• Release was reverted back to 2004030010070 (April 17, 2020, 12:03:45 PM PST)

Impact

• All AMP stories mobile users.
• Sharing a story through the sharing button was broken.

---

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

[danielrozenberg]

I rolled back Stable from 2004161906340 to 2004030010070. As this is a Friday we should postpone actually cherry-picking the Stable release until Monday, but since this is also a clean revert we should cherry-pick it into Beta/Experimental

• @ampproject/cherry-pick-approvers to approve cherry picking on Stable on Monday
• @ampproject/wg-approvers (@newmuis and @choumx) already approved cherry picking on Beta/Experimental

[estherkim]

On-duty here, no cherry-picks or additional approvals are necessary today.

• Stable (2004030010070) does not need a cherry-pick; it does not contain the commit that needed to be reverted.
• Beta (2004172112280) was cherry-picked on Friday. Pending errors and performance monitoring, this will be promoted to Stable tomorrow.

I will leave this issue open for the postmortem write up. Thanks!

[Enriqe]

Finished writing the mini PM. Adding @ampproject/wg-stories and @ampproject/wg-ui-and-a11y in case I missed something.

[caroqliu]

Thanks for finishing this @Enriqe! Added another note to exhaustively require OWNERs review for component changes from WGs concerning formats that a component is enabled in.