ampproject / ampproject/amphtml
amp-lightbox closes without user interaction on Tab switch
- 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.

### 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
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