Revert "⏪ Revert "♻️ Support img element (#34028)"" (#34630)

* Revert "Revert "♻️ Support img element (#34028)" (#34589)" This reverts commit 3e8f1fc. * Support image viewer and lightbox * Fix amp-img>img detection * Checks * switch to use the internal attributes instead of spying on propagate attributes helper * Add test for not selecting amp-img>img, but amp-img instead * Path changes from merge * Relative path instead of absolute * Correct usage of closest by selector * Accidental left over merge * More merge conflicts * merge conflicts * Linting fixes Co-authored-by: Ryan Cebulko <ryan@cebulko.com>
ampproject · Jun 16, 2021 · fa14e5c · fa14e5c
1 parent 2022f75
commit fa14e5c
Show file tree

Hide file tree

Showing 41 changed files with 777 additions and 374 deletions.
diff --git a/build-system/test-configs/forbidden-terms.js b/build-system/test-configs/forbidden-terms.js
@@ -875,10 +875,13 @@ const forbiddenTermsSrcInclusive = {
       'src/service/resources-impl.js',
       'src/service/variable-source.js',
       'src/validator-integration.js',
-      'extensions/amp-image-lightbox/0.1/amp-image-lightbox.js',
       'extensions/amp-analytics/0.1/transport.js',
-      'extensions/amp-web-push/0.1/iframehost.js',
+      'extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js',
+      'extensions/amp-image-lightbox/0.1/amp-image-lightbox.js',
+      'extensions/amp-image-slider/0.1/amp-image-slider.js',
+      'extensions/amp-image-viewer/0.1/amp-image-viewer.js',
       'extensions/amp-recaptcha-input/0.1/amp-recaptcha-service.js',
+      'extensions/amp-web-push/0.1/iframehost.js',
     ],
   },
   '\\.getTime\\(\\)': {

diff --git a/builtins/amp-img/amp-img.js b/builtins/amp-img/amp-img.js
@@ -21,6 +21,7 @@ import {Services} from '#service';
 import {dev} from '../../src/log';
 import {guaranteeSrcForSrcsetUnsupportedBrowsers} from '#core/dom/img';
 import {listen} from '../../src/event-helper';
+import {propagateAttributes} from '#core/dom/propagate-attributes';
 import {propagateObjectFitStyles, setImportantStyles} from '#core/dom/style';
 import {registerElement} from '#service/custom-element-registry';
 import {removeElement} from '#core/dom';
@@ -33,7 +34,7 @@ const TAG = 'amp-img';
  * Attributes to propagate to internal image when changed externally.
  * @type {!Array<string>}
  */
-const ATTRIBUTES_TO_PROPAGATE = [
+export const ATTRIBUTES_TO_PROPAGATE = [
   'alt',
   'aria-describedby',
   'aria-label',
@@ -131,8 +132,9 @@ export class AmpImg extends BaseElement {
           this.element
         );
       }
-      this.propagateAttributes(
+      propagateAttributes(
         attrs,
+        this.element,
         this.img_,
         /* opt_removeMissingAttrs */ true
       );
@@ -217,7 +219,7 @@ export class AmpImg extends BaseElement {
 
     // It is important to call this before setting `srcset` attribute.
     this.maybeGenerateSizes_(/* sync setAttribute */ true);
-    this.propagateAttributes(ATTRIBUTES_TO_PROPAGATE, this.img_);
+    propagateAttributes(ATTRIBUTES_TO_PROPAGATE, this.element, this.img_);
     this.propagateDataset(this.img_);
     if (!IS_ESM) {
       guaranteeSrcForSrcsetUnsupportedBrowsers(this.img_);

diff --git a/examples/amp-lightbox.amp.html b/examples/amp-lightbox.amp.html
@@ -63,6 +63,8 @@ <h2>Scrollable Lightbox</h2>
       Lorem ipsum dolor sit amet, has nisl nihil convenire et, vim at aeque inermis reprehendunt. Propriae tincidunt id nec, elit nusquam te mea, ius noster platonem in. Mea an idque minim, sit sale deleniti apeirian et. Omnium legendos tractatos cu mea. Vix in stet dolorem accusamus. Iisque rationibus consetetur in cum, quo unum nulla legere ut. Simul numquam saperet no sit.
     </p>
     <p>
+      <!-- native image element example, it's not valid yet, but supported. -->
+      <img src="https://lh3.googleusercontent.com/5rcQ32ml8E5ONp9f9-Rf78IofLb9QjS5_0mqsY1zEFc=w300-h200-no" width=300 height=200 loading="lazy" />
       <amp-img src="https://lh3.googleusercontent.com/5rcQ32ml8E5ONp9f9-Rf78IofLb9QjS5_0mqsY1zEFc=w300-h200-no" width=300 height=200></amp-img>
     </p>
     <p class="lightbox-text">

diff --git a/examples/image-lightbox.amp.html b/examples/image-lightbox.amp.html
@@ -22,6 +22,13 @@
 <script async src="https://cdn.ampproject.org/v0.js"></script>
 </head>
 <body>
+  <!-- Test for native image element, it's not valid yet, but supported. -->
+  <img on="tap:ampimagelightbox"
+    role="button"
+    src="img/bigbuckbunny.jpg"
+    width="500"
+    height="500"
+    loading="lazy" />
   <!--
     - Test layout: intrinsic, fill, fixed, fixed-height, responsive
   -->

diff --git a/extensions/amp-anim/0.1/amp-anim.js b/extensions/amp-anim/0.1/amp-anim.js
@@ -22,6 +22,7 @@ import {
   observeWithSharedInOb,
   unobserveWithSharedInOb,
 } from '../../../src/viewport-observer';
+import {propagateAttributes} from '#core/dom/propagate-attributes';
 import {propagateObjectFitStyles} from '#core/dom/style';
 
 const TAG = 'amp-anim';
@@ -55,7 +56,7 @@ export class AmpAnim extends AMP.BaseElement {
   buildCallback() {
     this.img_ = new Image();
     this.img_.setAttribute('decoding', 'async');
-    this.propagateAttributes(BUILD_ATTRIBUTES, this.img_);
+    propagateAttributes(BUILD_ATTRIBUTES, this.element, this.img_);
     applyFillContent(this.img_, true);
     propagateObjectFitStyles(this.element, this.img_);
 
@@ -88,8 +89,9 @@ export class AmpAnim extends AMP.BaseElement {
     const img = dev().assertElement(this.img_);
     // Remove missing attributes to remove the placeholder srcset if none is
     // specified on the element.
-    this.propagateAttributes(
+    propagateAttributes(
       LAYOUT_ATTRIBUTES,
+      this.element,
       img,
       /* opt_removeMissingAttrs */ true
     );

diff --git a/extensions/amp-audio/0.1/amp-audio.js b/extensions/amp-audio/0.1/amp-audio.js
@@ -31,6 +31,7 @@ import {
 import {dev, user} from '../../../src/log';
 import {getMode} from '../../../src/mode';
 import {listen} from '../../../src/event-helper';
+import {propagateAttributes} from '#core/dom/propagate-attributes';
 import {setIsMediaComponent} from '../../../src/video-interface';
 import {triggerAnalyticsEvent} from '../../../src/analytics';
 
@@ -88,7 +89,11 @@ export class AmpAudio extends AMP.BaseElement {
       if (src !== undefined) {
         assertHttpsUrl(src, this.element);
       }
-      this.propagateAttributes(['src', 'loop', 'controlsList'], this.audio_);
+      propagateAttributes(
+        ['src', 'loop', 'controlsList'],
+        this.element,
+        this.audio_
+      );
     }
 
     const artist = mutations['artist'];
@@ -122,7 +127,7 @@ export class AmpAudio extends AMP.BaseElement {
     if (src) {
       assertHttpsUrl(src, this.element);
     }
-    this.propagateAttributes(
+    propagateAttributes(
       [
         'src',
         'preload',
@@ -134,6 +139,7 @@ export class AmpAudio extends AMP.BaseElement {
         'aria-labelledby',
         'controlsList',
       ],
+      this.element,
       audio
     );
 

diff --git a/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js b/extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js
@@ -29,6 +29,7 @@ import {Services} from '#service';
 import {closestAncestorElementBySelector} from '#core/dom/query';
 import {dev} from '../../../src/log';
 import {dispatchCustomEvent} from '#core/dom';
+import {loadPromise} from '../../../src/event-helper';
 import {measureIntersectionNoRoot} from '../../../src/utils/intersection-no-root';
 import {toArray} from '#core/types/array';
 import {tryParseJson} from '#core/types/object/json';
@@ -112,33 +113,40 @@ const META_OG_TYPE = 'meta[property="og:type"]';
 
 const NOOP = () => {};
 
-/**
- * For better minification.
- * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
- * @return {!Document|!ShadowRoot}
- */
-const getRootNode = (ampdoc) => ampdoc.getRootNode();
-
 /** @visibleForTesting */
 export class Criteria {
   /**
    * @param {!Element} element
+   * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
    * @param {number} renderWidth
    * @param {number} renderHeight
    * @return {boolean}
    */
-  static meetsAll(element, renderWidth, renderHeight) {
+  static meetsAll(element, ampdoc, renderWidth, renderHeight) {
     return (
-      Criteria.meetsSizingCriteria(element, renderWidth, renderHeight) &&
-      Criteria.meetsTreeShapeCriteria(element)
+      Criteria.meetsSizingCriteria(
+        element,
+        ampdoc,
+        renderWidth,
+        renderHeight
+      ) && Criteria.meetsTreeShapeCriteria(element, ampdoc)
     );
   }
 
   /**
    * @param {!Element} element
+   * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
    * @return {boolean}
    */
-  static meetsTreeShapeCriteria(element) {
+  static meetsTreeShapeCriteria(element, ampdoc) {
+    if (
+      element.tagName === 'IMG' &&
+      closestAncestorElementBySelector(element, 'amp-img')
+    ) {
+      // Images that are a child of an AMP-IMG do not need additional treatment.
+      return false;
+    }
+
     const disabledSelector = `${DISABLED_ANCESTORS},${DISABLED_BY_ATTR}`;
     const disabledAncestor = closestAncestorElementBySelector(
       element,
@@ -147,22 +155,23 @@ export class Criteria {
     if (disabledAncestor) {
       return false;
     }
-    const actions = Services.actionServiceForDoc(element);
+    const actions = Services.actionServiceForDoc(ampdoc || element);
     return !actions.hasResolvableAction(element, 'tap');
   }
 
   /**
    * @param {!Element} element
+   * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
    * @param {number} renderWidth
    * @param {number} renderHeight
    * @return {boolean}
    */
-  static meetsSizingCriteria(element, renderWidth, renderHeight) {
+  static meetsSizingCriteria(element, ampdoc, renderWidth, renderHeight) {
     const {naturalHeight, naturalWidth} = getMaxNaturalDimensions(
-      dev().assertElement(element.querySelector('img'))
+      dev().assertElement(element.querySelector('img') || element)
     );
 
-    const viewport = Services.viewportForDoc(element);
+    const viewport = Services.viewportForDoc(ampdoc);
     const {height: vh, width: vw} = viewport.getSize();
 
     return meetsSizingCriteria(
@@ -271,18 +280,26 @@ function markAsVisited(candidate) {
 }
 
 /**
- * @param {string} tagName
+ * @param {!Array<string>} tagNames
  * @return {string}
  */
-function candidateSelector(tagName) {
-  return `${tagName}:not([${LIGHTBOXABLE_ATTR}]):not([${VISITED_ATTR}])`;
+function candidateSelector(tagNames) {
+  return tagNames
+    .map(
+      (tagName) =>
+        `${tagName}:not([${LIGHTBOXABLE_ATTR}]):not([${VISITED_ATTR}])`
+    )
+    .join(',');
 }
 
 /**
  * @param {!Element} element
  * @return {!Promise}
  */
 function whenLoaded(element) {
+  if (element.tagName === 'IMG') {
+    return loadPromise(element);
+  }
   return whenUpgradedToCustomElement(element).then((element) =>
     element.signals().whenSignal(CommonSignals.LOAD_END)
   );
@@ -296,7 +313,7 @@ export class Scanner {
    * @return {!Array<!Element>}
    */
   static getCandidates(root) {
-    const selector = candidateSelector('amp-img');
+    const selector = candidateSelector(['amp-img', 'img']);
     const candidates = toArray(root.querySelectorAll(selector));
     // TODO(alanorozco): DOM mutations should be wrapped in mutate contexts.
     // Alternatively, use in-memory "visited" marker instead of attribute.
@@ -316,7 +333,7 @@ export class DocMetaAnnotations {
    * @return {string|undefined}
    */
   static getOgType(ampdoc) {
-    const tag = getRootNode(ampdoc).querySelector(META_OG_TYPE);
+    const tag = ampdoc.getRootNode().querySelector(META_OG_TYPE);
     if (tag) {
       return tag.getAttribute('content');
     }
@@ -337,7 +354,7 @@ export class DocMetaAnnotations {
    * @return {!Array<string>}
    */
   static getAllLdJsonTypes(ampdoc) {
-    return toArray(getRootNode(ampdoc).querySelectorAll(SCRIPT_LD_JSON))
+    return toArray(ampdoc.getRootNode().querySelectorAll(SCRIPT_LD_JSON))
       .map((el) => {
         const {textContent} = el;
         return (tryParseJson(textContent) || {})['@type'];
@@ -367,10 +384,8 @@ export class DocMetaAnnotations {
 function usesLightboxExplicitly(ampdoc) {
   // TODO(alanorozco): Backport into Extensions service.
   const requiredExtensionSelector = `script[custom-element="${REQUIRED_EXTENSION}"]`;
-
   const lightboxedElementsSelector = `[${LIGHTBOXABLE_ATTR}]:not([${VISITED_ATTR}])`;
-
-  const exists = (selector) => !!getRootNode(ampdoc).querySelector(selector);
+  const exists = (selector) => !!ampdoc.getRootNode().querySelector(selector);
 
   return (
     exists(requiredExtensionSelector) && exists(lightboxedElementsSelector)
@@ -438,13 +453,17 @@ export function runCandidates(ampdoc, candidates) {
     whenLoaded(candidate).then(() => {
       return measureIntersectionNoRoot(candidate).then(
         ({boundingClientRect}) => {
-          // <amp-img> will change the img's src inline data on unlayout and
-          // remove it from DOM.
-          if (!candidate.signals().get(CommonSignals.LOAD_END)) {
+          if (
+            candidate.tagName !== 'IMG' &&
+            !candidate.signals().get(CommonSignals.LOAD_END)
+          ) {
+            // <amp-img> will change the img's src inline data on unlayout and
+            // remove it from DOM.
             return;
           }
+
           const {height, width} = boundingClientRect;
-          if (!Criteria.meetsAll(candidate, width, height)) {
+          if (!Criteria.meetsAll(candidate, ampdoc, width, height)) {
             return;
           }
           dev().info(TAG, 'apply', candidate);
@@ -473,7 +492,7 @@ export function scan(ampdoc, opt_root) {
 AMP.extension(TAG, '0.1', (AMP) => {
   const {ampdoc} = AMP;
   ampdoc.whenReady().then(() => {
-    getRootNode(ampdoc).addEventListener(AmpEvents.DOM_UPDATE, (e) => {
+    ampdoc.getRootNode().addEventListener(AmpEvents.DOM_UPDATE, (e) => {
       const {target} = e;
       scan(ampdoc, dev().assertElement(target));
     });