🐛 amp-auto-lightbox: Ignore unlaid out elements (#26573)

Fixes #25764
ampproject · Feb 3, 2020 · 0167a14 · 0167a14
1 parent e7a8e7f
commit 0167a14
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 0 deletions.
diff --git a/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js b/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js
@@ -399,6 +399,11 @@ export function apply(ampdoc, element) {
 export function runCandidates(ampdoc, candidates) {
   return candidates.map(candidate =>
     whenLoaded(candidate).then(() => {
+      // <amp-img> will change the img's src inline data on unlayout and remove
+      // it from DOM, but a LOAD_END event would still be triggered afterwards.
+      if (candidate.signals().get(CommonSignals.UNLOAD)) {
+        return;
+      }
       if (!Criteria.meetsAll(candidate)) {
         return;
       }

diff --git a/extensions/amp-auto-lightbox/0.1/test/test-amp-auto-lightbox.js b/extensions/amp-auto-lightbox/0.1/test/test-amp-auto-lightbox.js
@@ -544,6 +544,21 @@ describes.realWin(
     });
 
     describe('runCandidates', () => {
+      it('ignores amp-img load signal after being unlaid out', async () => {
+        const img = html`
+          <amp-img src="bla.png" layout="flex-item"></amp-img>
+        `;
+
+        const signals = new Signals();
+        img.signals = () => signals;
+
+        signals.signal(CommonSignals.UNLOAD);
+        signals.signal(CommonSignals.LOAD_END);
+
+        const elected = await Promise.all(runCandidates(env.ampdoc, [img]));
+        expect(elected[0]).to.be.undefined;
+      });
+
       it('filters out candidates that fail to load', async () => {
         const shouldNotLoad = mockLoadedSignal(
           html`