From d1bd392a682fb506cd7b93238bb29c1e07ca54d6 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Mon, 4 Oct 2021 10:41:50 +0200 Subject: [PATCH] MOBILE-3773 video: Fix placeholder displayed if no poster --- src/core/directives/external-content.ts | 6 +++-- src/core/directives/format-text.ts | 31 ++++++++++++++++++++-- src/core/services/filepool.ts | 34 +++++++++++++++++++------ src/core/services/utils/mimetype.ts | 5 ++++ src/core/services/utils/url.ts | 13 ++++++++++ src/core/singletons/url.ts | 28 ++++++++++++++++++++ 6 files changed, 105 insertions(+), 12 deletions(-) diff --git a/src/core/directives/external-content.ts b/src/core/directives/external-content.ts index 4d84f6845..d3e157dcc 100644 --- a/src/core/directives/external-content.ts +++ b/src/core/directives/external-content.ts @@ -266,9 +266,11 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges { finalUrl = CoreFile.convertFileSrc(finalUrl); } - if (!CoreUrlUtils.isLocalFileUrl(finalUrl)) { + if (!CoreUrlUtils.isLocalFileUrl(finalUrl) && !finalUrl.includes('#')) { /* In iOS, if we use the same URL in embedded file and background download then the download only - downloads a few bytes (cached ones). Add a hash to the URL so both URLs are different. */ + downloads a few bytes (cached ones). Add an anchor to the URL so both URLs are different. + Don't add this anchor if the URL already has an anchor, otherwise other anchors might not work. + The downloaded URL won't have anchors so the URLs will already be different. */ finalUrl = finalUrl + '#moodlemobile-embedded'; } diff --git a/src/core/directives/format-text.ts b/src/core/directives/format-text.ts index 22b5d3e4a..4869c50b8 100644 --- a/src/core/directives/format-text.ts +++ b/src/core/directives/format-text.ts @@ -555,7 +555,7 @@ export class CoreFormatTextDirective implements OnChanges { }); videos.forEach((video) => { - this.treatMedia(video); + this.treatMedia(video, true); }); iframes.forEach((iframe) => { @@ -673,8 +673,9 @@ export class CoreFormatTextDirective implements OnChanges { * Add media adapt class and apply CoreExternalContentDirective to the media element and its sources and tracks. * * @param element Video or audio to treat. + * @param isVideo Whether it's a video. */ - protected treatMedia(element: HTMLElement): void { + protected treatMedia(element: HTMLElement, isVideo: boolean = false): void { this.addMediaAdaptClass(element); this.addExternalContent(element); @@ -692,8 +693,16 @@ export class CoreFormatTextDirective implements OnChanges { const sources = Array.from(element.querySelectorAll('source')); const tracks = Array.from(element.querySelectorAll('track')); + const hasPoster = isVideo && !!element.getAttribute('poster'); + + if (isVideo && !hasPoster) { + this.fixVideoSrcPlaceholder(element); + } sources.forEach((source) => { + if (isVideo && !hasPoster) { + this.fixVideoSrcPlaceholder(source); + } source.setAttribute('target-src', source.getAttribute('src') || ''); source.removeAttribute('src'); this.addExternalContent(source); @@ -709,6 +718,24 @@ export class CoreFormatTextDirective implements OnChanges { }); } + /** + * Try to fix the placeholder displayed when a video doesn't have a poster. + * + * @param element Element to fix. + */ + protected fixVideoSrcPlaceholder(element: HTMLElement): void { + const src = element.getAttribute('src'); + if (!src) { + return; + } + + if (src.match(/#t=\d/)) { + return; + } + + element.setAttribute('src', src + '#t=0.001'); + } + /** * Add media adapt class and treat the iframe source. * diff --git a/src/core/services/filepool.ts b/src/core/services/filepool.ts index eb4cf3fdd..98efbef5f 100644 --- a/src/core/services/filepool.ts +++ b/src/core/services/filepool.ts @@ -47,6 +47,7 @@ import { CoreFilepoolQueueDBEntry, } from '@services/database/filepool'; import { CoreFileHelper } from './file-helper'; +import { CoreUrl } from '@singletons/url'; /* * Factory for handling downloading files and retrieve downloaded files. @@ -672,6 +673,13 @@ export class CoreFilepoolProvider { poolFileObject?: CoreFilepoolFileEntry, ): Promise { const fileId = this.getFileIdByUrl(fileUrl); + + // Extract the anchor from the URL (if any). + const anchor = CoreUrl.getUrlAnchor(fileUrl); + if (anchor) { + fileUrl = fileUrl.replace(anchor, ''); + } + const extension = CoreMimetypeUtils.guessExtensionFromUrl(fileUrl); const addExtension = typeof filePath == 'undefined'; const path = filePath || (await this.getFilePath(siteId, fileId, extension)); @@ -712,7 +720,8 @@ export class CoreFilepoolProvider { extension: fileEntry.extension, }); - return fileEntry.toURL(); + // Add the anchor again to the local URL. + return fileEntry.toURL() + (anchor || ''); }).finally(() => { // Download finished, delete the promise. delete this.filePromises[siteId][downloadId]; @@ -1015,7 +1024,10 @@ export class CoreFilepoolProvider { url = await this.getInternalUrlById(siteId, fileId); } - return finishSuccessfulDownload(url); + // Add the anchor to the local URL if any. + const anchor = CoreUrl.getUrlAnchor(fileUrl); + + return finishSuccessfulDownload(url + (anchor || '')); } catch (error) { // The file is not downloaded or it's outdated. this.notifyFileDownloading(siteId, fileId, links); @@ -1284,6 +1296,9 @@ export class CoreFilepoolProvider { }); } + // Remove the anchor. + url = CoreUrl.removeUrlAnchor(url); + // Try to guess the filename the target file should have. // We want to keep the original file name so people can easily identify the files after the download. const filename = this.guessFilenameFromUrl(url); @@ -1446,7 +1461,7 @@ export class CoreFilepoolProvider { return CoreConstants.NOT_DOWNLOADABLE; } - fileUrl = CoreFileHelper.getFileUrl(file); + fileUrl = CoreUrl.removeUrlAnchor(CoreFileHelper.getFileUrl(file)); timemodified = file.timemodified || timemodified; revision = revision || this.getRevisionFromUrl(fileUrl); const fileId = this.getFileIdByUrl(fileUrl); @@ -1556,11 +1571,14 @@ export class CoreFilepoolProvider { try { // We found the file entry, now look for the file on disk. - if (mode === 'src') { - return await this.getInternalSrcById(siteId, fileId); - } else { - return await this.getInternalUrlById(siteId, fileId); - } + const path = mode === 'src' ? + await this.getInternalSrcById(siteId, fileId) : + await this.getInternalUrlById(siteId, fileId); + + // Add the anchor to the local URL if any. + const anchor = CoreUrl.getUrlAnchor(fileUrl); + + return path + (anchor || ''); } catch (error) { // The file is not on disk. // We could not retrieve the file, delete the entries associated with that ID. diff --git a/src/core/services/utils/mimetype.ts b/src/core/services/utils/mimetype.ts index 78ea7322f..ddf65901d 100644 --- a/src/core/services/utils/mimetype.ts +++ b/src/core/services/utils/mimetype.ts @@ -313,6 +313,11 @@ export class CoreMimetypeUtilsProvider { if (position > -1) { candidate = candidate.substr(0, position); } + // Remove anchor if any. + position = candidate.indexOf('#'); + if (position > -1) { + candidate = candidate.substr(0, position); + } if (EXTENSION_REGEX.test(candidate)) { extension = candidate; diff --git a/src/core/services/utils/url.ts b/src/core/services/utils/url.ts index 2f7cd0736..79aafb51c 100644 --- a/src/core/services/utils/url.ts +++ b/src/core/services/utils/url.ts @@ -58,6 +58,10 @@ export class CoreUrlUtilsProvider { * @return URL with params. */ addParamsToUrl(url: string, params?: Record, anchor?: string, boolToNumber?: boolean): string { + // Remove any existing anchor to add the params before it. + const urlAndAnchor = url.split('#'); + url = urlAndAnchor[0]; + let separator = url.indexOf('?') != -1 ? '&' : '?'; for (const key in params) { @@ -75,6 +79,15 @@ export class CoreUrlUtilsProvider { } } + // Re-add the anchor if any. + if (urlAndAnchor.length > 1) { + // Remove the URL from the array. + urlAndAnchor.shift(); + + // Use a join in case there is more than one #. + url += '#' + urlAndAnchor.join('#'); + } + if (anchor) { url += '#' + anchor; } diff --git a/src/core/singletons/url.ts b/src/core/singletons/url.ts index 5c8b1f10f..097f05155 100644 --- a/src/core/singletons/url.ts +++ b/src/core/singletons/url.ts @@ -203,4 +203,32 @@ export class CoreUrl { && CoreText.removeEndingSlash(partsA?.path) === CoreText.removeEndingSlash(partsB?.path); } + /** + * Get the anchor of a URL. If there's more than one they'll all be returned, separated by #. + * E.g. myurl.com#foo=1#bar=2 will return #foo=1#bar=2. + * + * @param url URL. + * @return Anchor, undefined if no anchor. + */ + static getUrlAnchor(url: string): string | undefined { + const firstAnchorIndex = url.indexOf('#'); + if (firstAnchorIndex === -1) { + return; + } + + return url.substr(firstAnchorIndex); + } + + /** + * Remove the anchor from a URL. + * + * @param url URL. + * @return URL without anchor if any. + */ + static removeUrlAnchor(url: string): string { + const urlAndAnchor = url.split('#'); + + return urlAndAnchor[0]; + } + }