From 06cd333096d02407854bf6a8edb84e4883a64146 Mon Sep 17 00:00:00 2001 From: Deepak Lalwani Date: Thu, 10 Mar 2022 20:07:35 +0530 Subject: [PATCH 1/6] Prevent loading auto lightbox when disabled using attribute --- extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js | 3 --- src/auto-lightbox.js | 6 +++++- 2 files changed, 5 insertions(+), 4 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 index a5e3425575e9..b9c98e081ab9 100644 --- a/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js +++ b/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js @@ -64,9 +64,6 @@ const DISABLED_BY_ATTR = [ // Runtime-specific. '[placeholder]', - // Explicitly opted out. - '[data-amp-auto-lightbox-disable]', - // Considered "actionable", i.e. that are bound to a default // onclick action(e.g. `button`) or where it cannot be determined whether // they're actionable or not (e.g. `amp-script`). diff --git a/src/auto-lightbox.js b/src/auto-lightbox.js index 80b51fe3f6d8..bca7bb291cf5 100644 --- a/src/auto-lightbox.js +++ b/src/auto-lightbox.js @@ -27,7 +27,11 @@ export const AutoLightboxEvents_Enum = { export function installAutoLightboxExtension(ampdoc) { const {win} = ampdoc; // Only enabled on single documents tagged as or . - if (!isAmphtml(win.document) || !ampdoc.isSingleDoc()) { + if ( + !isAmphtml(win.document) || + !ampdoc.isSingleDoc() || + win.document.body.hasAttribute('data-amp-auto-lightbox-disable') + ) { return; } chunk( From a103fa070bea47006a540f02e6148559ab472905 Mon Sep 17 00:00:00 2001 From: Deepak Lalwani Date: Thu, 10 Mar 2022 22:22:01 +0530 Subject: [PATCH 2/6] Fix failing test case --- .../amp-auto-lightbox/0.1/test/test-amp-auto-lightbox.js | 4 ---- 1 file changed, 4 deletions(-) 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 index ef243e4ea933..895216d41e75 100644 --- 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 @@ -241,10 +241,6 @@ describes.realWin( accepts: 'elements inside non-clickable anchor', wrapWith: () => html` `, }, - { - rejects: 'explicitly opted-out subnodes', - mutate: (el) => el.setAttribute('data-amp-auto-lightbox-disable', ''), - }, { rejects: 'amp-subscriptions subnodes', mutate: (el) => el.setAttribute('subscriptions-action', ''), From 64dd31f9532c291e97ea5c20862a1df1d125dc0b Mon Sep 17 00:00:00 2001 From: Anurag Vasanwala <75766877+AnuragVasanwala@users.noreply.github.com> Date: Thu, 14 Apr 2022 23:27:45 +0530 Subject: [PATCH 3/6] =?UTF-8?q?=E2=9C=A8=20Add=20`data-amp-auto-lightbox-d?= =?UTF-8?q?isable`=20to=20`DISABLED=5FBY=5FATTR`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Marking `data-amp-auto-lightbox-disable` as `` will disable `amp-auto-lightbox` from auto-load on page load. --- extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js b/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js index b9c98e081ab9..a5e3425575e9 100644 --- a/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js +++ b/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js @@ -64,6 +64,9 @@ const DISABLED_BY_ATTR = [ // Runtime-specific. '[placeholder]', + // Explicitly opted out. + '[data-amp-auto-lightbox-disable]', + // Considered "actionable", i.e. that are bound to a default // onclick action(e.g. `button`) or where it cannot be determined whether // they're actionable or not (e.g. `amp-script`). From 825c1c2e730215683532577cd98062ac41cb489e Mon Sep 17 00:00:00 2001 From: Anurag Vasanwala <75766877+AnuragVasanwala@users.noreply.github.com> Date: Thu, 14 Apr 2022 23:36:26 +0530 Subject: [PATCH 4/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Move=20`data-amp-auto-?= =?UTF-8?q?lightbox-disable`=20attribute=20checking=20from=20`body`=20to?= =?UTF-8?q?=20`html`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will check if HTML Tag has `data-amp-auto-lightbox-disable` attribute like: ``. If it has, it will disable auto-loading of `amp-auto-lightbox` on page load. --- src/auto-lightbox.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/auto-lightbox.js b/src/auto-lightbox.js index bca7bb291cf5..82220fcbe786 100644 --- a/src/auto-lightbox.js +++ b/src/auto-lightbox.js @@ -30,7 +30,9 @@ export function installAutoLightboxExtension(ampdoc) { if ( !isAmphtml(win.document) || !ampdoc.isSingleDoc() || - win.document.body.hasAttribute('data-amp-auto-lightbox-disable') + // Prevent loading auto lightbox when disabled using 'data-amp-auto-lightbox-disable' attribute (#37854) + // Check if HTML Tag has 'data-amp-auto-lightbox-disable' attribute + win.document.documentElement.hasAttribute('data-amp-auto-lightbox-disable') ) { return; } From 02ff8d1337704b0ea3cb2c7046ff90368684ce46 Mon Sep 17 00:00:00 2001 From: Anurag Vasanwala <75766877+AnuragVasanwala@users.noreply.github.com> Date: Fri, 15 Apr 2022 00:02:02 +0530 Subject: [PATCH 5/6] =?UTF-8?q?=F0=9F=93=96=20Update=20markdown=20`auto-li?= =?UTF-8?q?ghtbox.md`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/spec/auto-lightbox.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/spec/auto-lightbox.md b/docs/spec/auto-lightbox.md index 79c8e548f742..b4fcf5fbb86e 100644 --- a/docs/spec/auto-lightbox.md +++ b/docs/spec/auto-lightbox.md @@ -53,7 +53,7 @@ To disable it on a particular document section: ``` -Or to disable it for your entire document altogether: +To disable it for your entire document altogether: ```html @@ -61,6 +61,14 @@ Or to disable it for your entire document altogether: ``` +Or to prevent automatically loading of `amp-auto-lightbox` script on page load: + +```html + + + +``` + If you'd like manual tuning of disabled/enabled images and/or grouping, please use [`amp-lightbox-gallery`](https://amp.dev/documentation/components/amp-lightbox-gallery) directly. From 9fdcd6a49563838f56e58d2937dec3c58fd5504d Mon Sep 17 00:00:00 2001 From: Anurag Vasanwala <75766877+AnuragVasanwala@users.noreply.github.com> Date: Fri, 15 Apr 2022 00:03:24 +0530 Subject: [PATCH 6/6] =?UTF-8?q?=E2=8F=AA=20Revert=20unit-test=20changes=20?= =?UTF-8?q?to=20original?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../amp-auto-lightbox/0.1/test/test-amp-auto-lightbox.js | 4 ++++ 1 file changed, 4 insertions(+) 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 index 895216d41e75..ef243e4ea933 100644 --- 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 @@ -241,6 +241,10 @@ describes.realWin( accepts: 'elements inside non-clickable anchor', wrapWith: () => html` `, }, + { + rejects: 'explicitly opted-out subnodes', + mutate: (el) => el.setAttribute('data-amp-auto-lightbox-disable', ''), + }, { rejects: 'amp-subscriptions subnodes', mutate: (el) => el.setAttribute('subscriptions-action', ''),