Skip to content

Commit 80274dc

Browse files
kristoferbaxterkristoferbaxter
authored
Revert "Revert "♻️ Support img element (#34028)" (#34589)"
This reverts commit 3e8f1fc.
1 parent 63d8ca9 commit 80274dc

41 files changed

Lines changed: 709 additions & 334 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎build-system/test-configs/forbidden-terms.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,8 @@ const forbiddenTermsSrcInclusive = {
870870
'extensions/amp-analytics/0.1/transport.js',
871871
'extensions/amp-web-push/0.1/iframehost.js',
872872
'extensions/amp-recaptcha-input/0.1/amp-recaptcha-service.js',
873+
'extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js',
874+
'extensions/amp-image-slider/0.1/amp-image-slider.js',
873875
],
874876
},
875877
'\\.getTime\\(\\)': {

‎builtins/amp-img/amp-img.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {Services} from '../../src/services';
2121
import {dev} from '../../src/log';
2222
import {guaranteeSrcForSrcsetUnsupportedBrowsers} from '../../src/utils/img';
2323
import {listen} from '../../src/event-helper';
24+
import {propagateAttributes} from '../../src/core/dom/propagate-attributes';
2425
import {propagateObjectFitStyles, setImportantStyles} from '../../src/style';
2526
import {registerElement} from '../../src/service/custom-element-registry';
2627
import {removeElement, scopedQuerySelector} from '../../src/dom';
@@ -130,8 +131,9 @@ export class AmpImg extends BaseElement {
130131
this.element
131132
);
132133
}
133-
this.propagateAttributes(
134+
propagateAttributes(
134135
attrs,
136+
this.element,
135137
this.img_,
136138
/* opt_removeMissingAttrs */ true
137139
);
@@ -216,7 +218,7 @@ export class AmpImg extends BaseElement {
216218

217219
// It is important to call this before setting `srcset` attribute.
218220
this.maybeGenerateSizes_(/* sync setAttribute */ true);
219-
this.propagateAttributes(ATTRIBUTES_TO_PROPAGATE, this.img_);
221+
propagateAttributes(ATTRIBUTES_TO_PROPAGATE, this.element, this.img_);
220222
this.propagateDataset(this.img_);
221223
if (!IS_ESM) {
222224
guaranteeSrcForSrcsetUnsupportedBrowsers(this.img_);

‎examples/amp-lightbox.amp.html‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ <h2>Scrollable Lightbox</h2>
6363
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.
6464
</p>
6565
<p>
66+
<!-- native image element example, it's not valid yet, but supported. -->
67+
<img src="https://lh3.googleusercontent.com/5rcQ32ml8E5ONp9f9-Rf78IofLb9QjS5_0mqsY1zEFc=w300-h200-no" width=300 height=200 loading="lazy" />
6668
<amp-img src="https://lh3.googleusercontent.com/5rcQ32ml8E5ONp9f9-Rf78IofLb9QjS5_0mqsY1zEFc=w300-h200-no" width=300 height=200></amp-img>
6769
</p>
6870
<p class="lightbox-text">

‎examples/image-lightbox.amp.html‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@
2222
<script async src="https://cdn.ampproject.org/v0.js"></script>
2323
</head>
2424
<body>
25+
<!-- Test for native image element, it's not valid yet, but supported. -->
26+
<img on="tap:ampimagelightbox"
27+
role="button"
28+
src="img/bigbuckbunny.jpg"
29+
width="500"
30+
height="500"
31+
loading="lazy" />
2532
<!--
2633
- Test layout: intrinsic, fill, fixed, fixed-height, responsive
2734
-->

‎extensions/amp-anim/0.1/amp-anim.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
observeWithSharedInOb,
2323
unobserveWithSharedInOb,
2424
} from '../../../src/viewport-observer';
25+
import {propagateAttributes} from '../../../src/core/dom/propagate-attributes';
2526
import {propagateObjectFitStyles} from '../../../src/style';
2627

2728
const TAG = 'amp-anim';
@@ -55,7 +56,7 @@ export class AmpAnim extends AMP.BaseElement {
5556
buildCallback() {
5657
this.img_ = new Image();
5758
this.img_.setAttribute('decoding', 'async');
58-
this.propagateAttributes(BUILD_ATTRIBUTES, this.img_);
59+
propagateAttributes(BUILD_ATTRIBUTES, this.element, this.img_);
5960
this.applyFillContent(this.img_, true);
6061
propagateObjectFitStyles(this.element, this.img_);
6162

@@ -88,8 +89,9 @@ export class AmpAnim extends AMP.BaseElement {
8889
const img = dev().assertElement(this.img_);
8990
// Remove missing attributes to remove the placeholder srcset if none is
9091
// specified on the element.
91-
this.propagateAttributes(
92+
propagateAttributes(
9293
LAYOUT_ATTRIBUTES,
94+
this.element,
9395
img,
9496
/* opt_removeMissingAttrs */ true
9597
);

‎extensions/amp-audio/0.1/amp-audio.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {closestAncestorElementBySelector} from '../../../src/dom';
2828
import {dev, user} from '../../../src/log';
2929
import {getMode} from '../../../src/mode';
3030
import {listen} from '../../../src/event-helper';
31+
import {propagateAttributes} from '../../../src/core/dom/propagate-attributes';
3132
import {setIsMediaComponent} from '../../../src/video-interface';
3233
import {triggerAnalyticsEvent} from '../../../src/analytics';
3334

@@ -85,7 +86,11 @@ export class AmpAudio extends AMP.BaseElement {
8586
if (src !== undefined) {
8687
assertHttpsUrl(src, this.element);
8788
}
88-
this.propagateAttributes(['src', 'loop', 'controlsList'], this.audio_);
89+
propagateAttributes(
90+
['src', 'loop', 'controlsList'],
91+
this.element,
92+
this.audio_
93+
);
8994
}
9095

9196
const artist = mutations['artist'];
@@ -119,7 +124,7 @@ export class AmpAudio extends AMP.BaseElement {
119124
if (src) {
120125
assertHttpsUrl(src, this.element);
121126
}
122-
this.propagateAttributes(
127+
propagateAttributes(
123128
[
124129
'src',
125130
'preload',
@@ -131,6 +136,7 @@ export class AmpAudio extends AMP.BaseElement {
131136
'aria-labelledby',
132137
'controlsList',
133138
],
139+
this.element,
134140
audio
135141
);
136142

‎extensions/amp-auto-lightbox/0.1/amp-auto-lightbox.js‎

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
whenUpgradedToCustomElement,
3333
} from '../../../src/dom';
3434
import {dev} from '../../../src/log';
35+
import {loadPromise} from '../../../src/event-helper';
3536
import {measureIntersectionNoRoot} from '../../../src/utils/intersection-no-root';
3637
import {toArray} from '../../../src/core/types/array';
3738
import {tryParseJson} from '../../../src/core/types/object/json';
@@ -114,25 +115,23 @@ const META_OG_TYPE = 'meta[property="og:type"]';
114115

115116
const NOOP = () => {};
116117

117-
/**
118-
* For better minification.
119-
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
120-
* @return {!Document|!ShadowRoot}
121-
*/
122-
const getRootNode = (ampdoc) => ampdoc.getRootNode();
123-
124118
/** @visibleForTesting */
125119
export class Criteria {
126120
/**
127121
* @param {!Element} element
122+
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
128123
* @param {number} renderWidth
129124
* @param {number} renderHeight
130125
* @return {boolean}
131126
*/
132-
static meetsAll(element, renderWidth, renderHeight) {
127+
static meetsAll(element, ampdoc, renderWidth, renderHeight) {
133128
return (
134-
Criteria.meetsSizingCriteria(element, renderWidth, renderHeight) &&
135-
Criteria.meetsTreeShapeCriteria(element)
129+
Criteria.meetsSizingCriteria(
130+
element,
131+
ampdoc,
132+
renderWidth,
133+
renderHeight
134+
) && Criteria.meetsTreeShapeCriteria(element)
136135
);
137136
}
138137

@@ -155,16 +154,17 @@ export class Criteria {
155154

156155
/**
157156
* @param {!Element} element
157+
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
158158
* @param {number} renderWidth
159159
* @param {number} renderHeight
160160
* @return {boolean}
161161
*/
162-
static meetsSizingCriteria(element, renderWidth, renderHeight) {
162+
static meetsSizingCriteria(element, ampdoc, renderWidth, renderHeight) {
163163
const {naturalHeight, naturalWidth} = getMaxNaturalDimensions(
164-
dev().assertElement(element.querySelector('img'))
164+
dev().assertElement(element.querySelector('img') || element)
165165
);
166166

167-
const viewport = Services.viewportForDoc(element);
167+
const viewport = Services.viewportForDoc(ampdoc);
168168
const {height: vh, width: vw} = viewport.getSize();
169169

170170
return meetsSizingCriteria(
@@ -273,18 +273,26 @@ function markAsVisited(candidate) {
273273
}
274274

275275
/**
276-
* @param {string} tagName
276+
* @param {!Array<string>} tagNames
277277
* @return {string}
278278
*/
279-
function candidateSelector(tagName) {
280-
return `${tagName}:not([${LIGHTBOXABLE_ATTR}]):not([${VISITED_ATTR}])`;
279+
function candidateSelector(tagNames) {
280+
return tagNames
281+
.map(
282+
(tagName) =>
283+
`${tagName}:not([${LIGHTBOXABLE_ATTR}]):not([${VISITED_ATTR}])`
284+
)
285+
.join(',');
281286
}
282287

283288
/**
284289
* @param {!Element} element
285290
* @return {!Promise}
286291
*/
287292
function whenLoaded(element) {
293+
if (element.tagName === 'IMG') {
294+
return loadPromise(element);
295+
}
288296
return whenUpgradedToCustomElement(element).then((element) =>
289297
element.signals().whenSignal(CommonSignals.LOAD_END)
290298
);
@@ -298,7 +306,7 @@ export class Scanner {
298306
* @return {!Array<!Element>}
299307
*/
300308
static getCandidates(root) {
301-
const selector = candidateSelector('amp-img');
309+
const selector = candidateSelector(['amp-img', 'img']);
302310
const candidates = toArray(root.querySelectorAll(selector));
303311
// TODO(alanorozco): DOM mutations should be wrapped in mutate contexts.
304312
// Alternatively, use in-memory "visited" marker instead of attribute.
@@ -318,7 +326,7 @@ export class DocMetaAnnotations {
318326
* @return {string|undefined}
319327
*/
320328
static getOgType(ampdoc) {
321-
const tag = getRootNode(ampdoc).querySelector(META_OG_TYPE);
329+
const tag = ampdoc.getRootNode().querySelector(META_OG_TYPE);
322330
if (tag) {
323331
return tag.getAttribute('content');
324332
}
@@ -339,7 +347,7 @@ export class DocMetaAnnotations {
339347
* @return {!Array<string>}
340348
*/
341349
static getAllLdJsonTypes(ampdoc) {
342-
return toArray(getRootNode(ampdoc).querySelectorAll(SCRIPT_LD_JSON))
350+
return toArray(ampdoc.getRootNode().querySelectorAll(SCRIPT_LD_JSON))
343351
.map((el) => {
344352
const {textContent} = el;
345353
return (tryParseJson(textContent) || {})['@type'];
@@ -369,10 +377,8 @@ export class DocMetaAnnotations {
369377
function usesLightboxExplicitly(ampdoc) {
370378
// TODO(alanorozco): Backport into Extensions service.
371379
const requiredExtensionSelector = `script[custom-element="${REQUIRED_EXTENSION}"]`;
372-
373380
const lightboxedElementsSelector = `[${LIGHTBOXABLE_ATTR}]:not([${VISITED_ATTR}])`;
374-
375-
const exists = (selector) => !!getRootNode(ampdoc).querySelector(selector);
381+
const exists = (selector) => !!ampdoc.getRootNode().querySelector(selector);
376382

377383
return (
378384
exists(requiredExtensionSelector) && exists(lightboxedElementsSelector)
@@ -440,13 +446,17 @@ export function runCandidates(ampdoc, candidates) {
440446
whenLoaded(candidate).then(() => {
441447
return measureIntersectionNoRoot(candidate).then(
442448
({boundingClientRect}) => {
443-
// <amp-img> will change the img's src inline data on unlayout and
444-
// remove it from DOM.
445-
if (!candidate.signals().get(CommonSignals.LOAD_END)) {
449+
if (
450+
!candidate.tagName === 'IMG' &&
451+
!candidate.signals().get(CommonSignals.LOAD_END)
452+
) {
453+
// <amp-img> will change the img's src inline data on unlayout and
454+
// remove it from DOM.
446455
return;
447456
}
457+
448458
const {height, width} = boundingClientRect;
449-
if (!Criteria.meetsAll(candidate, width, height)) {
459+
if (!Criteria.meetsAll(candidate, ampdoc, width, height)) {
450460
return;
451461
}
452462
dev().info(TAG, 'apply', candidate);
@@ -475,7 +485,7 @@ export function scan(ampdoc, opt_root) {
475485
AMP.extension(TAG, '0.1', (AMP) => {
476486
const {ampdoc} = AMP;
477487
ampdoc.whenReady().then(() => {
478-
getRootNode(ampdoc).addEventListener(AmpEvents.DOM_UPDATE, (e) => {
488+
ampdoc.getRootNode().addEventListener(AmpEvents.DOM_UPDATE, (e) => {
479489
const {target} = e;
480490
scan(ampdoc, dev().assertElement(target));
481491
});

‎extensions/amp-auto-lightbox/0.1/test/test-amp-auto-lightbox.js‎

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -140,29 +140,62 @@ describes.realWin(
140140

141141
it(`${accepts ? 'accepts' : 'rejects'} ${accepts || rejects}`, () => {
142142
[
143-
html` <amp-img src="asada.png" layout="flex-item"></amp-img> `,
144-
html`
145-
<div>
146-
<amp-img src="adobada.png" layout="flex-item"></amp-img>
147-
</div>
148-
`,
149-
html`
150-
<div>
143+
{
144+
markup: html`
145+
<amp-img src="asada.png" layout="flex-item"></amp-img>
146+
`,
147+
tagName: 'AMP-IMG',
148+
},
149+
{
150+
markup: html`
151151
<div>
152-
<amp-img src="carnitas.png" layout="flex-item"></amp-img>
152+
<amp-img src="adobada.png" layout="flex-item"></amp-img>
153153
</div>
154-
</div>
155-
`,
154+
`,
155+
tagName: 'AMP-IMG',
156+
},
157+
{
158+
markup: html`
159+
<div>
160+
<div>
161+
<amp-img src="carnitas.png" layout="flex-item"></amp-img>
162+
</div>
163+
</div>
164+
`,
165+
tagName: 'AMP-IMG',
166+
},
167+
{
168+
markup: html` <img src="asada.png" layout="flex-item" /> `,
169+
tagName: 'IMG',
170+
},
171+
{
172+
markup: html`
173+
<div>
174+
<img src="adobada.png" layout="flex-item" />
175+
</div>
176+
`,
177+
tagName: 'IMG',
178+
},
179+
{
180+
markup: html`
181+
<div>
182+
<div>
183+
<img src="carnitas.png" layout="flex-item" />
184+
</div>
185+
</div>
186+
`,
187+
tagName: 'IMG',
188+
},
156189
].forEach((unwrapped) => {
157-
maybeMutate(unwrapped);
190+
maybeMutate(unwrapped.markup);
158191

159-
const scenario = maybeWrap(unwrapped);
192+
const scenario = maybeWrap(unwrapped.markup);
160193
const candidate = firstElementLeaf(scenario);
161194

162195
env.win.document.body.appendChild(scenario);
163196

164197
expect(candidate).to.be.ok;
165-
expect(candidate.tagName).to.equal('AMP-IMG');
198+
expect(candidate.tagName).to.equal(unwrapped.tagName);
166199

167200
expect(
168201
Criteria.meetsTreeShapeCriteria(candidate),

‎extensions/amp-brid-player/0.1/amp-brid-player.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {getData, listen} from '../../../src/event-helper';
4242
import {htmlFor} from '../../../src/static-template';
4343
import {installVideoManagerForDoc} from '../../../src/service/video-manager-impl';
4444
import {isLayoutSizeDefined} from '../../../src/layout';
45+
import {propagateAttributes} from '../../../src/core/dom/propagate-attributes';
4546

4647
const TAG = 'amp-brid-player';
4748

@@ -247,7 +248,7 @@ class AmpBridPlayer extends AMP.BaseElement {
247248
<img placeholder referrerpolicy="origin" loading="lazy" />
248249
`;
249250

250-
this.propagateAttributes(['aria-label'], placeholder);
251+
propagateAttributes(['aria-label'], this.element, placeholder);
251252
this.applyFillContent(placeholder);
252253

253254
const altText = placeholder.hasAttribute('aria-label')

‎extensions/amp-gfycat/0.1/amp-gfycat.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import {getData, listen} from '../../../src/event-helper';
2727
import {installVideoManagerForDoc} from '../../../src/service/video-manager-impl';
2828
import {isLayoutSizeDefined} from '../../../src/layout';
29+
import {propagateAttributes} from '../../../src/core/dom/propagate-attributes';
2930

3031
const TAG = 'amp-gfycat';
3132

@@ -90,7 +91,7 @@ class AmpGfycat extends AMP.BaseElement {
9091
const placeholder = this.win.document.createElement('img');
9192
const videoid = dev().assertString(this.videoid_);
9293
this.applyFillContent(placeholder);
93-
this.propagateAttributes(['alt', 'aria-label'], placeholder);
94+
propagateAttributes(['alt', 'aria-label'], this.element, placeholder);
9495
placeholder.setAttribute('loading', 'lazy');
9596
placeholder.setAttribute('placeholder', '');
9697
placeholder.setAttribute('referrerpolicy', 'origin');

0 commit comments

Comments
 (0)