Skip to content

Commit 861d0c2

Browse files
committed
MOBILE-5072 external-content: Support picture element and srcset attr
1 parent 835b59d commit 861d0c2

2 files changed

Lines changed: 153 additions & 23 deletions

File tree

src/core/directives/external-content.ts

Lines changed: 112 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -157,10 +157,24 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
157157
targetAttr = 'src';
158158
url = this.url ?? this.src ?? ''; // eslint-disable-line @typescript-eslint/no-deprecated
159159

160+
await this.handleSrcset(this.siteId);
161+
160162
} else if (tagName === 'AUDIO' || tagName === 'VIDEO' || tagName === 'SOURCE' || tagName === 'TRACK') {
161163
targetAttr = 'src';
162164
url = this.url ?? this.src ?? ''; // eslint-disable-line @typescript-eslint/no-deprecated
163165

166+
if (tagName === 'SOURCE') {
167+
await this.handleSrcset(this.siteId);
168+
169+
// For elements that only use srcset, url can be empty.
170+
// Resolve early when tagName === 'SOURCE' and there is no src URL.
171+
if (!url) {
172+
this.onReadyPromise.resolve();
173+
174+
return;
175+
}
176+
}
177+
164178
if (tagName === 'VIDEO' && this.posterUrl) {
165179
// Handle poster.
166180

@@ -201,28 +215,19 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
201215
}
202216

203217
const site = await CorePromiseUtils.ignoreErrors(CoreSites.getSite(this.siteId));
204-
const isSiteFile = site?.isSitePluginFileUrl(url);
205-
206-
// Try to convert the URL to absolute. This will only work for URLs relative to the site URL, it won't work for
207-
// URLs relative to a subpath (e.g. relative to the course page URL).
208-
url = site && url ? CoreUrl.toAbsoluteURL(site.getURL(), url) : url;
209-
210-
if (!url || !url.match(/^https?:\/\//i) || CoreUrl.isLocalFileUrl(url) ||
211-
(tagName === 'A' && !(isSiteFile || site?.isSiteThemeImageUrl(url) || CoreUrl.isGravatarUrl(url)))) {
212-
213-
this.logger.debug(`Ignoring non-downloadable URL: ${url}`);
214-
215-
throw new CoreError('Non-downloadable URL');
216-
}
217218

218-
if (site && !site.canDownloadFiles() && isSiteFile) {
219-
this.element.parentElement?.removeChild(this.element); // Remove element since it'll be broken.
219+
let finalUrl: string;
220+
try {
221+
finalUrl = await this.getFinalUrl(url, tagName, targetAttr, site);
222+
} catch (error) {
223+
if (error instanceof CannotDownloadError) {
224+
// Remove element since it'll be broken.
225+
this.element.parentElement?.removeChild(this.element);
226+
}
220227

221-
throw new CoreError(Translate.instant('core.cannotdownloadfiles'));
228+
throw error;
222229
}
223230

224-
const finalUrl = await this.getUrlToUse(targetAttr, url, site);
225-
226231
this.logger.debug(`Using URL ${finalUrl} for ${url}`);
227232

228233
this.setElementUrl(targetAttr, finalUrl);
@@ -276,6 +281,93 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
276281
}
277282
}
278283

284+
/**
285+
* Handle srcset attribute, replacing each URL with the local/downloaded version.
286+
*
287+
* @param siteId Site ID.
288+
* @returns Promise resolved when done.
289+
*/
290+
protected async handleSrcset(siteId?: string): Promise<void> {
291+
const srcset = this.element.getAttribute('data-original-srcset') ?? this.element.getAttribute('srcset');
292+
if (!srcset) {
293+
return;
294+
}
295+
296+
if (!this.element.hasAttribute('data-original-srcset')) {
297+
this.element.setAttribute('data-original-srcset', srcset);
298+
}
299+
300+
const site = await CorePromiseUtils.ignoreErrors(CoreSites.getSite(siteId));
301+
302+
// Parse srcset: comma-separated list of "url [descriptor]" pairs.
303+
const parts = srcset.split(',').map(part => part.trim()).filter(part => part.length > 0);
304+
305+
const promises = parts.map(async (part) => {
306+
const tokens = part.split(/\s+/);
307+
const url = tokens[0];
308+
const descriptor = tokens.slice(1).join(' ');
309+
310+
if (!url) {
311+
return part;
312+
}
313+
314+
let finalUrl: string;
315+
try {
316+
finalUrl = await this.getFinalUrl(url, this.element.tagName, 'srcset', site);
317+
318+
this.logger.debug(`Using URL ${finalUrl} for ${url} in srcset`);
319+
320+
return descriptor ? `${finalUrl} ${descriptor}` : finalUrl;
321+
} catch (error) {
322+
if (error instanceof CannotDownloadError) {
323+
// URL might be broken.
324+
return '';
325+
}
326+
327+
return part;
328+
}
329+
});
330+
331+
try {
332+
const newParts = await Promise.all(promises);
333+
this.element.setAttribute('srcset', newParts.filter(part => part.length > 0).join(', '));
334+
} catch {
335+
this.logger.error('Error treating srcset.', this.element);
336+
}
337+
}
338+
339+
/**
340+
* Get the final URL to use in the element, checking if it's valid and can be downloaded.
341+
*
342+
* @param url URL to treat.
343+
* @param tagName Name of the tag using the URL.
344+
* @param targetAttr Attribute using the URL.
345+
* @param site Site.
346+
* @returns Promise resolved with the URL to use in the element.
347+
* If the URL is not valid or can't be downloaded, the promise will be rejected.
348+
*/
349+
protected async getFinalUrl(url: string, tagName: string, targetAttr: string, site?: CoreSite): Promise<string> {
350+
const isSiteFile = site?.isSitePluginFileUrl(url);
351+
352+
// Try to convert the URL to absolute. This will only work for URLs relative to the site URL, it won't work for
353+
// URLs relative to a subpath (e.g. relative to the course page URL).
354+
url = site && url ? CoreUrl.toAbsoluteURL(site.getURL(), url) : url;
355+
356+
if (!url || !url.match(/^https?:\/\//i) || CoreUrl.isLocalFileUrl(url) ||
357+
(tagName === 'A' && !(isSiteFile || site?.isSiteThemeImageUrl(url) || CoreUrl.isGravatarUrl(url)))) {
358+
359+
this.logger.debug(`Ignoring non-downloadable URL: ${url}`);
360+
361+
throw new CoreError('Non-downloadable URL');
362+
}
363+
364+
if (site && !site.canDownloadFiles() && isSiteFile) {
365+
throw new CannotDownloadError(Translate.instant('core.cannotdownloadfiles'));
366+
}
367+
368+
return await this.getUrlToUse(targetAttr, url, site);
369+
}
370+
279371
/**
280372
* Handle inline styles, trying to download referenced files.
281373
*
@@ -587,3 +679,5 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O
587679
}
588680

589681
}
682+
683+
class CannotDownloadError extends CoreError {};

src/core/directives/format-text.ts

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,7 @@ export class CoreFormatTextDirective implements OnDestroy, AsyncDirective {
364364

365365
/**
366366
* Wrap an image with a container to adapt its width.
367+
* If the image is inside a picture element, the picture element is wrapped instead.
367368
*
368369
* @param img Image to adapt.
369370
*/
@@ -372,6 +373,11 @@ export class CoreFormatTextDirective implements OnDestroy, AsyncDirective {
372373
return;
373374
}
374375

376+
// If the image is inside a picture element, wrap the picture element instead.
377+
const elementToWrap = img.parentElement?.tagName === 'PICTURE'
378+
? img.parentElement
379+
: img;
380+
375381
// Element to wrap the image.
376382
const container = document.createElement('span');
377383
const originalWidth = img.attributes.getNamedItem('width');
@@ -399,22 +405,25 @@ export class CoreFormatTextDirective implements OnDestroy, AsyncDirective {
399405
container.classList.add('atto_image_button_text-bottom');
400406
}
401407

402-
CoreDom.wrapElement(img, container);
408+
CoreDom.wrapElement(elementToWrap, container);
403409
}
404410

405411
/**
406412
* Add image viewer button to view adapted images at full size.
407413
*/
408414
protected async addImageViewerButton(): Promise<void> {
409-
const imgs = Array.from(this.element.querySelectorAll('.core-adapted-img-container > img'));
415+
const imgs = Array.from(
416+
this.element.querySelectorAll<HTMLImageElement>(
417+
'.core-adapted-img-container > img, .core-adapted-img-container > picture > img',
418+
),
419+
);
410420
if (!imgs.length) {
411421
return;
412422
}
413423

414424
// If cannot calculate element's width, use viewport width to avoid false adapt image icons appearing.
415425
const elWidth = await this.getElementWidth();
416-
417-
imgs.forEach((img: HTMLImageElement) => {
426+
imgs.forEach((img) => {
418427
// Skip image if it's inside a link.
419428
if (img.closest('a')) {
420429
return;
@@ -701,6 +710,7 @@ export class CoreFormatTextDirective implements OnDestroy, AsyncDirective {
701710
const site = await this.getSite();
702711

703712
const images = Array.from(div.querySelectorAll('img'));
713+
const pictures = Array.from(div.querySelectorAll('picture'));
704714
const anchors = Array.from(div.querySelectorAll('a'));
705715
const audios = Array.from(div.querySelectorAll('audio'));
706716
const videos = Array.from(div.querySelectorAll('video'));
@@ -735,12 +745,22 @@ export class CoreFormatTextDirective implements OnDestroy, AsyncDirective {
735745
this.addExternalContent(anchor);
736746
});
737747

748+
const siteId = this.getSiteId();
738749
const externalImages: CoreExternalContentDirective[] = [];
739750
if (images && images.length > 0) {
740751
// Walk through the content to find images, and add our directive.
741-
images.forEach((img: HTMLElement) => {
752+
images.forEach((img) => {
742753
this.addMediaAdaptClass(img);
743754

755+
if (siteId) {
756+
// Avoid WebView starting network requests before external-content can rewrite URLs.
757+
const srcset = img.getAttribute('srcset');
758+
if (srcset) {
759+
img.setAttribute('data-original-srcset', srcset);
760+
img.removeAttribute('srcset');
761+
}
762+
}
763+
744764
const externalImage = this.addExternalContent(img);
745765
if (externalImage && !externalImage.invalid) {
746766
externalImages.push(externalImage);
@@ -752,6 +772,22 @@ export class CoreFormatTextDirective implements OnDestroy, AsyncDirective {
752772
});
753773
}
754774

775+
pictures.forEach(picture => {
776+
const sources = Array.from(picture.querySelectorAll('source'));
777+
778+
sources.forEach((source) => {
779+
if (siteId) {
780+
// Avoid WebView starting network requests before external-content can rewrite URLs.
781+
const srcset = source.getAttribute('srcset');
782+
if (srcset) {
783+
source.setAttribute('data-original-srcset', srcset);
784+
source.removeAttribute('srcset');
785+
}
786+
}
787+
this.addExternalContent(source);
788+
});
789+
});
790+
755791
const audioControllers = audios.map(audio => {
756792
this.treatMedia(audio);
757793

0 commit comments

Comments
 (0)