Revert "Revert "♻️ Support img element (#34028)" (#34589)"

This reverts commit 3e8f1fc.
ampproject · Jun 1, 2021 · 80274dc · 80274dc
1 parent 63d8ca9
commit 80274dc
Show file tree

Hide file tree

Showing 41 changed files with 709 additions and 334 deletions.
diff --git a/build-system/test-configs/forbidden-terms.js b/build-system/test-configs/forbidden-terms.js
@@ -870,6 +870,8 @@ const forbiddenTermsSrcInclusive = {
  'extensions/amp-analytics/0.1/transport.js',
  'extensions/amp-web-push/0.1/iframehost.js',
  'extensions/amp-recaptcha-input/0.1/amp-recaptcha-service.js',
+ 'extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js',
+ 'extensions/amp-image-slider/0.1/amp-image-slider.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 '../../src/services';
 import {dev} from '../../src/log';
 import {guaranteeSrcForSrcsetUnsupportedBrowsers} from '../../src/utils/img';
 import {listen} from '../../src/event-helper';
+import {propagateAttributes} from '../../src/core/dom/propagate-attributes';
 import {propagateObjectFitStyles, setImportantStyles} from '../../src/style';
 import {registerElement} from '../../src/service/custom-element-registry';
 import {removeElement, scopedQuerySelector} from '../../src/dom';
@@ -130,8 +131,9 @@ export class AmpImg extends BaseElement {
  this.element
  );
  }
- this.propagateAttributes(
+ propagateAttributes(
  attrs,
+ this.element,
  this.img_,
  /* opt_removeMissingAttrs */ true
  );
@@ -216,7 +218,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 '../../../src/core/dom/propagate-attributes';
 import {propagateObjectFitStyles} from '../../../src/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_);
  this.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
@@ -28,6 +28,7 @@ import {closestAncestorElementBySelector} from '../../../src/dom';
 import {dev, user} from '../../../src/log';
 import {getMode} from '../../../src/mode';
 import {listen} from '../../../src/event-helper';
+import {propagateAttributes} from '../../../src/core/dom/propagate-attributes';
 import {setIsMediaComponent} from '../../../src/video-interface';
 import {triggerAnalyticsEvent} from '../../../src/analytics';
 
@@ -85,7 +86,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'];
@@ -119,7 +124,7 @@ export class AmpAudio extends AMP.BaseElement {
  if (src) {
  assertHttpsUrl(src, this.element);
  }
- this.propagateAttributes(
+ propagateAttributes(
  [
  'src',
  'preload',
@@ -131,6 +136,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
@@ -32,6 +32,7 @@ import {
  whenUpgradedToCustomElement,
 } from '../../../src/dom';
 import {dev} from '../../../src/log';
+import {loadPromise} from '../../../src/event-helper';
 import {measureIntersectionNoRoot} from '../../../src/utils/intersection-no-root';
 import {toArray} from '../../../src/core/types/array';
 import {tryParseJson} from '../../../src/core/types/object/json';
@@ -114,25 +115,23 @@ 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)
  );
  }
 
@@ -155,16 +154,17 @@ export class Criteria {
 
  /**
  * @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(
@@ -273,18 +273,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)
  );
@@ -298,7 +306,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.
@@ -318,7 +326,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');
  }
@@ -339,7 +347,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'];
@@ -369,10 +377,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)
@@ -440,13 +446,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);
@@ -475,7 +485,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));
  });

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
@@ -140,29 +140,62 @@ describes.realWin(
 
  it(`${accepts ? 'accepts' : 'rejects'} ${accepts || rejects}`, () => {
  [
- html` <amp-img src="asada.png" layout="flex-item"></amp-img> `,
- html`
- <div>
-  <amp-img src="adobada.png" layout="flex-item"></amp-img>
- </div>
- `,
- html`
- <div>
+ {
+  markup: html`
+  <amp-img src="asada.png" layout="flex-item"></amp-img>
+ `,
+ tagName: 'AMP-IMG',
+ },
+ {
+ markup: html`
  <div>
- <amp-img src="carnitas.png" layout="flex-item"></amp-img>
+ <amp-img src="adobada.png" layout="flex-item"></amp-img>
  </div>
- </div>
- `,
+ `,
+ tagName: 'AMP-IMG',
+ },
+ {
+ markup: html`
+ <div>
+ <div>
+ <amp-img src="carnitas.png" layout="flex-item"></amp-img>
+ </div>
+ </div>
+ `,
+ tagName: 'AMP-IMG',
+ },
+ {
+ markup: html` <img src="asada.png" layout="flex-item" /> `,
+ tagName: 'IMG',
+ },
+ {
+ markup: html`
+ <div>
+ <img src="adobada.png" layout="flex-item" />
+ </div>
+ `,
+ tagName: 'IMG',
+ },
+ {
+ markup: html`
+ <div>
+ <div>
+ <img src="carnitas.png" layout="flex-item" />
+ </div>
+ </div>
+ `,
+ tagName: 'IMG',
+ },
  ].forEach((unwrapped) => {
- maybeMutate(unwrapped);
+ maybeMutate(unwrapped.markup);
 
- const scenario = maybeWrap(unwrapped);
+ const scenario = maybeWrap(unwrapped.markup);
  const candidate = firstElementLeaf(scenario);
 
  env.win.document.body.appendChild(scenario);
 
  expect(candidate).to.be.ok;
- expect(candidate.tagName).to.equal('AMP-IMG');
+ expect(candidate.tagName).to.equal(unwrapped.tagName);
 
  expect(
  Criteria.meetsTreeShapeCriteria(candidate),

diff --git a/extensions/amp-brid-player/0.1/amp-brid-player.js b/extensions/amp-brid-player/0.1/amp-brid-player.js
@@ -42,6 +42,7 @@ import {getData, listen} from '../../../src/event-helper';
 import {htmlFor} from '../../../src/static-template';
 import {installVideoManagerForDoc} from '../../../src/service/video-manager-impl';
 import {isLayoutSizeDefined} from '../../../src/layout';
+import {propagateAttributes} from '../../../src/core/dom/propagate-attributes';
 
 const TAG = 'amp-brid-player';
 
@@ -247,7 +248,7 @@ class AmpBridPlayer extends AMP.BaseElement {
  <img placeholder referrerpolicy="origin" loading="lazy" />
  `;
 
- this.propagateAttributes(['aria-label'], placeholder);
+ propagateAttributes(['aria-label'], this.element, placeholder);
  this.applyFillContent(placeholder);
 
  const altText = placeholder.hasAttribute('aria-label')

diff --git a/extensions/amp-gfycat/0.1/amp-gfycat.js b/extensions/amp-gfycat/0.1/amp-gfycat.js
@@ -26,6 +26,7 @@ import {
 import {getData, listen} from '../../../src/event-helper';
 import {installVideoManagerForDoc} from '../../../src/service/video-manager-impl';
 import {isLayoutSizeDefined} from '../../../src/layout';
+import {propagateAttributes} from '../../../src/core/dom/propagate-attributes';
 
 const TAG = 'amp-gfycat';
 
@@ -90,7 +91,7 @@ class AmpGfycat extends AMP.BaseElement {
  const placeholder = this.win.document.createElement('img');
  const videoid = dev().assertString(this.videoid_);
  this.applyFillContent(placeholder);
- this.propagateAttributes(['alt', 'aria-label'], placeholder);
+ propagateAttributes(['alt', 'aria-label'], this.element, placeholder);
  placeholder.setAttribute('loading', 'lazy');
  placeholder.setAttribute('placeholder', '');
  placeholder.setAttribute('referrerpolicy', 'origin');