ampproject / ampproject/amphtml

amp-lightbox closes without user interaction on Tab switch

Open
#31,941 2 comments 0 reactions 0 assignees View on GitHub
Component: amp-lightbox P3: When Possible Stale Type: Bug WG: components
Dominant language
JavaScript
Stars
14.9k
Forks
4.1k
PR merge metrics
No merged PRs in 30d

Description

We encountered a strange Bug with `amp-lightbox` closes automatically if the user switches Tabs in Chrome.

![amp-lightbox--autoclose](https://user-images.githubusercontent.com/1544760/104499190-ffefbe00-55dc-11eb-8f1f-86b4c9346a1e.gif)

### Reproducer
See [Reproducer in AMP-Playground](https://playground.amp.dev/?format=websites&_gl=1*1x3bd84*_ga*YW1wLUVvY0x0M0JNTndrU1NvZS11S0NfZ2c.&mode=Responsive&preview=true#share=PCEtLQogICMjIEludHJvZHVjdGlvbgoKICBUaGUgW2BhbXAtbGlnaHRib3hgXSgvY29udGVudC9hbXAtZGV2L2RvY3VtZW50YXRpb24vY29tcG9uZW50cy9yZWZlcmVuY2UvYW1wLWxpZ2h0Ym94LXYwLjEubWQpIGNvbXBvbmVudCBhbGxvd3MgZm9yIGEg4oCcbGlnaHRib3jigJ0gb3Igc2ltaWxhciBleHBlcmllbmNlIC0gd2hlcmUgdXBvbiB1c2VyIGludGVyYWN0aW9uIGEgY29tcG9uZW50IGV4cGFuZHMgdG8gZmlsbCB0aGUgdmlld3BvcnQsIHVudGlsIGl0IGlzIGNsb3NlZCBhZ2FpbiBieSB0aGUgdXNlci4KLS0+PCEtLSAtLT4KPCFkb2N0eXBlIGh0bWw+CjxodG1sIOKaoSBsYW5nPSJlbiI+CjxoZWFkPgogICAgPG1ldGEgY2hhcnNldD0idXRmLTgiPgogICAgPHRpdGxlPmFtcC1saWdodGJveDwvdGl0bGU+CiAgICA8c2NyaXB0IGFzeW5jIHNyYz0iaHR0cHM6Ly9jZG4uYW1wcHJvamVjdC5vcmcvdjAuanMiPjwvc2NyaXB0PgogICAgPHNjcmlwdCBhc3luYyBjdXN0b20tZWxlbWVudD0iYW1wLWNhcm91c2VsIiBzcmM9Imh0dHBzOi8vY2RuLmFtcHByb2plY3Qub3JnL3YwL2FtcC1jYXJvdXNlbC0wLjEuanMiPjwvc2NyaXB0PgogICAgPHNjcmlwdCBhc3luYyBjdXN0b20tZWxlbWVudD0iYW1wLWxpZ2h0Ym94IiBzcmM9Imh0dHBzOi8vY2RuLmFtcHByb2plY3Qub3JnL3YwL2FtcC1saWdodGJveC0wLjEuanMiPjwvc2NyaXB0PgogICAgPGxpbmsgcmVsPSJjYW5vbmljYWwiIGhyZWY9Imh0dHBzOi8vYW1wLmRldi9kb2N1bWVudGF0aW9uL2V4YW1wbGVzL2NvbXBvbmVudHMvYW1wLWxpZ2h0Ym94L2luZGV4Lmh0bWwiPgogICAgPG1ldGEgbmFtZT0idmlld3BvcnQiIGNvbnRlbnQ9IndpZHRoPWRldmljZS13aWR0aCI+CiAgICA8c3R5bGUgYW1wLWJvaWxlcnBsYXRlPmJvZHl7LXdlYmtpdC1hbmltYXRpb246LWFtcC1zdGFydCA4cyBzdGVwcygxLGVuZCkgMHMgMSBub3JtYWwgYm90aDstbW96LWFuaW1hdGlvbjotYW1wLXN0YXJ0IDhzIHN0ZXBzKDEsZW5kKSAwcyAxIG5vcm1hbCBib3RoOy1tcy1hbmltYXRpb246LWFtcC1zdGFydCA4cyBzdGVwcygxLGVuZCkgMHMgMSBub3JtYWwgYm90aDthbmltYXRpb246LWFtcC1zdGFydCA4cyBzdGVwcygxLGVuZCkgMHMgMSBub3JtYWwgYm90aH1ALXdlYmtpdC1rZXlmcmFtZXMgLWFtcC1zdGFydHtmcm9te3Zpc2liaWxpdHk6aGlkZGVufXRve3Zpc2liaWxpdHk6dmlzaWJsZX19QC1tb3ota2V5ZnJhbWVzIC1hbXAtc3RhcnR7ZnJvbXt2aXNpYmlsaXR5OmhpZGRlbn10b3t2aXNpYmlsaXR5OnZpc2libGV9fUAtbXMta2V5ZnJhbWVzIC1hbXAtc3RhcnR7ZnJvbXt2aXNpYmlsaXR5OmhpZGRlbn10b3t2aXNpYmlsaXR5OnZpc2libGV9fUAtby1rZXlmcmFtZXMgLWFtcC1zdGFydHtmcm9te3Zpc2liaWxpdHk6aGlkZGVufXRve3Zpc2liaWxpdHk6dmlzaWJsZX19QGtleWZyYW1lcyAtYW1wLXN0YXJ0e2Zyb217dmlzaWJpbGl0eTpoaWRkZW59dG97dmlzaWJpbGl0eTp2aXNpYmxlfX08L3N0eWxlPjxub3NjcmlwdD48c3R5bGUgYW1wLWJvaWxlcnBsYXRlPmJvZHl7LXdlYmtpdC1hbmltYXRpb246bm9uZTstbW96LWFuaW1hdGlvbjpub25lOy1tcy1hbmltYXRpb246bm9uZTthbmltYXRpb246bm9uZX08L3N0eWxlPjwvbm9zY3JpcHQ+CiAgICA8c3R5bGUgYW1wLWN1c3RvbT4KICAgICAgICAubGlnaHRib3ggewogICAgICAgICAgICBiYWNrZ3JvdW5kOiByZ2JhKDAsMCwwLDAuOCk7CiAgICAgICAgICAgIHdpZHRoOiAxMDAlOwogICAgICAgICAgICBoZWlnaHQ6IDEwMCU7CiAgICAgICAgICAgIHBvc2l0aW9uOiBhYnNvbHV0ZTsKICAgICAgICAgICAgZGlzcGxheTogZmxleDsKICAgICAgICAgICAgYWxpZ24taXRlbXM6IGNlbnRlcjsKICAgICAgICAgICAganVzdGlmeS1jb250ZW50OiBjZW50ZXI7CiAgICAgICAgfQogICAgICAgIC5saWdodGJveCBoMSB7CiAgICAgICAgICAgIGNvbG9yOiB3aGl0ZTsKICAgICAgICB9CiAgICA8L3N0eWxlPgo8L2hlYWQ+Cjxib2R5PgoKPGRpdj4KICAgIDxhbXAtbGlnaHRib3ggaWQ9Im15LWxpZ2h0Ym94IiBsYXlvdXQ9Im5vZGlzcGxheSI+CiAgICAgICAgPGRpdj4KICAgICAgICAgICAgPGFtcC1jYXJvdXNlbCBpZD0ibGlnaHRib3gtY2Fyb3VzZWwiCiAgICAgICAgICAgICAgICAgICAgICAgICAgaGVpZ2h0PSIzIgogICAgICAgICAgICAgICAgICAgICAgICAgIHdpZHRoPSI0IgogICAgICAgICAgICAgICAgICAgICAgICAgIGxheW91dD0icmVzcG9uc2l2ZSIKICAgICAgICAgICAgICAgICAgICAgICAgICBsb29wCiAgICAgICAgICAgICAgICAgICAgICAgICAgdHlwZT0ic2xpZGVzIj4KICAgICAgICAgICAgICAgIDxhbXAtaW1nIHNyYz0iaHR0cHM6Ly9wbGFjZWhvbGQuaXQvNDAweDMwMD8xIgogICAgICAgICAgICAgICAgICAgICAgICAgd2lkdGg9IjQwMCIKICAgICAgICAgICAgICAgICAgICAgICAgIGhlaWdodD0iMzAwIgogICAgICAgICAgICAgICAgICAgICAgICAgYWx0PSJhIHNhbXBsZSBpbWFnZSI+PC9hbXAtaW1nPgogICAgICAgICAgICAgICAgPGFtcC1pbWcgc3JjPSJodHRwczovL3BsYWNlaG9sZC5pdC80MDB4MzAwPzIiCiAgICAgICAgICAgICAgICAgICAgICAgICB3aWR0aD0iNDAwIgogICAgICAgICAgICAgICAgICAgICAgICAgaGVpZ2h0PSIzMDAiCiAgICAgICAgICAgICAgICAgICAgICAgICBhbHQ9ImFub3RoZXIgc2FtcGxlIGltYWdlIj48L2FtcC1pbWc+CiAgICAgICAgICAgICAgICA8YW1wLWltZyBzcmM9Imh0dHBzOi8vcGxhY2Vob2xkLml0LzQwMHgzMDA/MyIKICAgICAgICAgICAgICAgICAgICAgICAgIHdpZHRoPSI0MDAiCiAgICAgICAgICAgICAgICAgICAgICAgICBoZWlnaHQ9IjMwMCIKICAgICAgICAgICAgICAgICAgICAgICAgIGFsdD0iYW5kIGFub3RoZXIgc2FtcGxlIGltYWdlIj48L2FtcC1pbWc+CiAgICAgICAgICAgIDwvYW1wLWNhcm91c2VsPgogICAgICAgICAgICA8YSBvbj0idGFwOm15LWxpZ2h0Ym94LmNsb3NlIj4KICAgICAgICAgICAgICAgIENsb3NlIExpZ2h0Ym94CiAgICAgICAgICAgIDwvYT4KICAgICAgICA8L2Rpdj4KICAgIDwvYW1wLWxpZ2h0Ym94PgoKICAgIDxhbXAtY2Fyb3VzZWwgaGVpZ2h0PSIxMDAiCiAgICAgICAgICAgICAgICAgIGxheW91dD0iZml4ZWQtaGVpZ2h0IgogICAgICAgICAgICAgICAgICB0eXBlPSJzbGlkZXMiCiAgICA+CiAgICAgICAgPGEgb249InRhcDpteS1saWdodGJveC5vcGVuIj4KICAgICAgICAgICAgPGFtcC1pbWcgc3JjPSJodHRwczovL3BsYWNlaG9sZC5pdC80MDB4MzAwPzEiCiAgICAgICAgICAgICAgICAgICAgIHdpZHRoPSIxNTAiCiAgICAgICAgICAgICAgICAgICAgIGhlaWdodD0iMTAwIgogICAgICAgICAgICAgICAgICAgICBhbHQ9ImEgc2FtcGxlIGltYWdlIj48L2FtcC1pbWc+CiAgICAgICAgPC9hPgogICAgICAgIDxhIG9uPSJ0YXA6bXktbGlnaHRib3gub3BlbiI+CiAgICAgICAgICAgIDxhbXAtaW1nIHNyYz0iaHR0cHM6Ly9wbGFjZWhvbGQuaXQvNDAweDMwMD8yIgogICAgICAgICAgICAgICAgICAgICB3aWR0aD0iMTUwIgogICAgICAgICAgICAgICAgICAgICBoZWlnaHQ9IjEwMCIKICAgICAgICAgICAgICAgICAgICAgYWx0PSJhbm90aGVyIHNhbXBsZSBpbWFnZSI+PC9hbXAtaW1nPgogICAgICAgIDwvYT4KICAgICAgICA8YSBvbj0idGFwOm15LWxpZ2h0Ym94Lm9wZW4iPgogICAgICAgICAgICA8YW1wLWltZyBzcmM9Imh0dHBzOi8vcGxhY2Vob2xkLml0LzQwMHgzMDA/MyIKICAgICAgICAgICAgICAgICAgICAgd2lkdGg9IjE1MCIKICAgICAgICAgICAgICAgICAgICAgaGVpZ2h0PSIxMDAiCiAgICAgICAgICAgICAgICAgICAgIGFsdD0iYW5kIGFub3RoZXIgc2FtcGxlIGltYWdlIj48L2FtcC1pbWc+CiAgICAgICAgPC9hPgogICAgPC9hbXAtY2Fyb3VzZWw+CjwvZGl2Pgo8L2JvZHk+PC9odG1sPgo=)

If you click on the first slide, the lightbox opens. If you now go to another Tab in Chrome, and come back, the lightbox closes without user interaction. I traced it down to `history-impl.js` trying to pop the history in
```js
if (toPop.length > 0) {
for (let i = 0; i < toPop.length; i++) {
// With the same delay timeouts must observe the order, although
// there's no hard requirement in this case to follow the pop order.
this.timer_.delay(() => toPop[i](historyState), 1);
}
}
```
and eventually calling this code in `amp-lightbox-0.1.js`:
```js
F(a.getAmpDoc(), "history").push(function() {
return a.close(c)
}).then(function(U) {
a.ca = U
});
```

### Checked mutations of test code
This does NOT happen, if I:
- remove the `amp-carousel`s
- change the "close lightbox" link to a `button`
- remove the "close lightbox" link
- remove the `on="tap:my-lightbox.close"` attribute on the link

### Wild Guess
As this only happens if there is a link inside the lightbox that closes it, I think that is the culprit.

### Workaround
For anyone who is affected by this bug right now: Change the "Close Lightbox" Link to a `button`..

### Related Bugs
Probably #17442 is related.

### Affected AMP Version
At least `2012301722001`, but I've seen it [earlier](https://github.com/ampproject/amphtml/issues/17442#issuecomment-601559711)..

### Affected Browsers
Tested in Chrome 87.0.4280.141, but probably more Browsers are affected by this.

// cc @choumx (sorry) because of his investigation in #17442

Contributor guide

Open the contributing guide

Research direction

Start with the AMP Playground reproducer and inspect the history-impl.js timeout path that eventually invokes amp-lightbox-0.1.js. Compare behavior with and without the lightbox's closing link, carousel, and tap action, then verify that switching away from and back to the Chrome tab leaves the lightbox open unless the user closes it.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend, web-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.