From 43ce384da81bd56fa9a5e4fd313a50c73db66122 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 23 Dec 2021 10:30:34 +0100 Subject: [PATCH 1/2] MOBILE-3897 external-content: Update URL when state changes --- src/core/directives/external-content.ts | 265 ++++++++++++------ .../settings/services/settings-helper.ts | 4 +- src/core/services/filepool.ts | 9 + 3 files changed, 184 insertions(+), 94 deletions(-) diff --git a/src/core/directives/external-content.ts b/src/core/directives/external-content.ts index efe74f020..50bd68e34 100644 --- a/src/core/directives/external-content.ts +++ b/src/core/directives/external-content.ts @@ -12,8 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { Directive, Input, AfterViewInit, ElementRef, OnChanges, SimpleChange, Output, EventEmitter } from '@angular/core'; - +import { + Directive, + Input, + AfterViewInit, + ElementRef, + OnChanges, + SimpleChange, + Output, + EventEmitter, + OnDestroy, +} from '@angular/core'; import { CoreApp } from '@services/app'; import { CoreFile } from '@services/file'; import { CoreFilepool } from '@services/filepool'; @@ -24,6 +33,9 @@ import { CoreUtils } from '@services/utils/utils'; import { Platform } from '@singletons'; import { CoreLogger } from '@singletons/logger'; import { CoreError } from '@classes/errors/error'; +import { CoreSite } from '@classes/site'; +import { CoreEventObserver, CoreEvents } from '@singletons/events'; +import { CoreConstants } from '../constants'; /** * Directive to handle external content. @@ -38,7 +50,7 @@ import { CoreError } from '@classes/errors/error'; @Directive({ selector: '[core-external-content]', }) -export class CoreExternalContentDirective implements AfterViewInit, OnChanges { +export class CoreExternalContentDirective implements AfterViewInit, OnChanges, OnDestroy { @Input() siteId?: string; // Site ID to use. @Input() component?: string; // Component to link the file to. @@ -54,6 +66,7 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges { protected element: Element; protected logger: CoreLogger; protected initialized = false; + protected fileEventObserver?: CoreEventObserver; constructor(element: ElementRef) { @@ -183,34 +196,8 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges { protected async handleExternalContent(targetAttr: string, url: string, siteId?: string): Promise { const tagName = this.element.tagName; - if (tagName == 'VIDEO' && targetAttr != 'poster') { - const video = this.element; - if (video.textTracks) { - // It's a video with subtitles. Fix some issues with subtitles. - video.textTracks.onaddtrack = (event): void => { - const track = event.track; - if (track) { - track.oncuechange = (): void => { - if (!track.cues) { - return; - } - - const line = Platform.is('tablet') || CoreApp.isAndroid() ? 90 : 80; - // Position all subtitles to a percentage of video height. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - Array.from(track.cues).forEach((cue: any) => { - cue.snapToLines = false; - cue.line = line; - cue.size = 100; // This solves some Android issue. - }); - // Delete listener. - track.oncuechange = null; - }; - } - }; - } - + this.handleVideoSubtitles( this.element); } const site = await CoreSites.getSite(siteId); @@ -234,46 +221,7 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges { throw new CoreError('Site doesn\'t allow downloading files.'); } - // Download images, tracks and posters if size is unknown. - const downloadUnknown = tagName == 'IMG' || tagName == 'TRACK' || targetAttr == 'poster'; - let finalUrl: string; - - if (targetAttr === 'src' && tagName !== 'SOURCE' && tagName !== 'TRACK' && tagName !== 'VIDEO' && tagName !== 'AUDIO') { - finalUrl = await CoreFilepool.getSrcByUrl( - site.getId(), - url, - this.component, - this.componentId, - 0, - true, - downloadUnknown, - ); - } else if (tagName === 'TRACK') { - // Download tracks right away. Using an online URL for tracks can give a CORS error in Android. - finalUrl = await CoreFilepool.downloadUrl(site.getId(), url, false, this.component, this.componentId); - - finalUrl = CoreFile.convertFileSrc(finalUrl); - } else { - finalUrl = await CoreFilepool.getUrlByUrl( - site.getId(), - url, - this.component, - this.componentId, - 0, - true, - downloadUnknown, - ); - - finalUrl = CoreFile.convertFileSrc(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 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'; - } + const finalUrl = await this.getUrlToUse(targetAttr, url, site); this.logger.debug('Using URL ' + finalUrl + ' for ' + url); if (tagName === 'SOURCE') { @@ -295,28 +243,7 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges { this.element.setAttribute('data-original-' + targetAttr, url); } - // Set events to download big files (not downloaded automatically). - if (!CoreUrlUtils.isLocalFileUrl(finalUrl) && targetAttr != 'poster' && - (tagName == 'VIDEO' || tagName == 'AUDIO' || tagName == 'A' || tagName == 'SOURCE')) { - const eventName = tagName == 'A' ? 'click' : 'play'; - let clickableEl = this.element; - - if (tagName == 'SOURCE') { - clickableEl = CoreDomUtils.closest(this.element, 'video,audio'); - if (!clickableEl) { - return; - } - } - - clickableEl.addEventListener(eventName, () => { - // User played media or opened a downloadable link. - // Download the file if in wifi and it hasn't been downloaded already (for big files). - if (CoreApp.isWifi()) { - // We aren't using the result, so it doesn't matter which of the 2 functions we call. - CoreFilepool.getUrlByUrl(site.getId(), url, this.component, this.componentId, 0, false); - } - }); - } + this.setListeners(targetAttr, url, site); } /** @@ -359,6 +286,153 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges { } } + /** + * Handle video subtitles if any. + * + * @param video Video element. + */ + protected handleVideoSubtitles(video: HTMLVideoElement): void { + if (!video.textTracks) { + return; + } + + // It's a video with subtitles. Fix some issues with subtitles. + video.textTracks.onaddtrack = (event): void => { + const track = event.track; + if (track) { + track.oncuechange = (): void => { + if (!track.cues) { + return; + } + + const line = Platform.is('tablet') || CoreApp.isAndroid() ? 90 : 80; + // Position all subtitles to a percentage of video height. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Array.from(track.cues).forEach((cue: any) => { + cue.snapToLines = false; + cue.line = line; + cue.size = 100; // This solves some Android issue. + }); + // Delete listener. + track.oncuechange = null; + }; + } + }; + } + + /** + * Get the URL to use in the element. E.g. if the file is already downloaded it will return the local URL. + * + * @param targetAttr Attribute to modify. + * @param url Original URL to treat. + * @param site Site. + * @return Promise resolved with the URL. + */ + protected async getUrlToUse(targetAttr: string, url: string, site: CoreSite): Promise { + const tagName = this.element.tagName; + let finalUrl: string; + + // Download images, tracks and posters if size is unknown. + const downloadUnknown = tagName == 'IMG' || tagName == 'TRACK' || targetAttr == 'poster'; + + if (targetAttr === 'src' && tagName !== 'SOURCE' && tagName !== 'TRACK' && tagName !== 'VIDEO' && tagName !== 'AUDIO') { + finalUrl = await CoreFilepool.getSrcByUrl( + site.getId(), + url, + this.component, + this.componentId, + 0, + true, + downloadUnknown, + ); + } else if (tagName === 'TRACK') { + // Download tracks right away. Using an online URL for tracks can give a CORS error in Android. + finalUrl = await CoreFilepool.downloadUrl(site.getId(), url, false, this.component, this.componentId); + + finalUrl = CoreFile.convertFileSrc(finalUrl); + } else { + finalUrl = await CoreFilepool.getUrlByUrl( + site.getId(), + url, + this.component, + this.componentId, + 0, + true, + downloadUnknown, + ); + + finalUrl = CoreFile.convertFileSrc(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 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'; + } + + return finalUrl; + } + + /** + * Set listeners if needed. + * + * @param targetAttr Attribute to modify. + * @param url Original URL to treat. + * @param site Site. + * @return Promise resolved when done. + */ + protected async setListeners(targetAttr: string, url: string, site: CoreSite): Promise { + if (this.fileEventObserver) { + // Already listening to events. + return; + } + + const tagName = this.element.tagName; + let state = await CoreFilepool.getFileStateByUrl(site.getId(), url); + + // Listen for download changes in the file. + const eventName = await CoreFilepool.getFileEventNameByUrl(site.getId(), url); + + this.fileEventObserver = CoreEvents.on(eventName, async () => { + const newState = await CoreFilepool.getFileStateByUrl(site.getId(), url); + if (newState === state) { + return; + } + + state = newState; + if (state === CoreConstants.DOWNLOADING) { + return; + } + + // The file state has changed. Handle the file again, maybe it's downloaded now or the file has been deleted. + this.checkAndHandleExternalContent(); + }); + + // Set events to download big files (not downloaded automatically). + if (targetAttr !== 'poster' && (tagName === 'VIDEO' || tagName === 'AUDIO' || tagName === 'A' || tagName === 'SOURCE')) { + const eventName = tagName == 'A' ? 'click' : 'play'; + let clickableEl = this.element; + + if (tagName == 'SOURCE') { + clickableEl = CoreDomUtils.closest(this.element, 'video,audio'); + if (!clickableEl) { + return; + } + } + + clickableEl.addEventListener(eventName, () => { + // User played media or opened a downloadable link. + // Download the file if in wifi and it hasn't been downloaded already (for big files). + if (state !== CoreConstants.DOWNLOADED && state !== CoreConstants.DOWNLOADING && CoreApp.isWifi()) { + // We aren't using the result, so it doesn't matter which of the 2 functions we call. + CoreFilepool.getUrlByUrl(site.getId(), url, this.component, this.componentId, 0, false); + } + }); + } + } + /** * Wait for the image to be loaded or error, and emit an event when it happens. */ @@ -374,4 +448,11 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges { this.element.addEventListener('error', listener); } + /** + * @inheritdoc + */ + ngOnDestroy(): void { + this.fileEventObserver?.off(); + } + } diff --git a/src/core/features/settings/services/settings-helper.ts b/src/core/features/settings/services/settings-helper.ts index 3ca96984a..5ae1d8ed8 100644 --- a/src/core/features/settings/services/settings-helper.ts +++ b/src/core/features/settings/services/settings-helper.ts @@ -231,7 +231,7 @@ export class CoreSettingsHelperProvider { * @return Sync promise or null if site is not being syncrhonized. */ getSiteSyncPromise(siteId: string): Promise | void { - if (this.syncPromises[siteId]) { + if (this.syncPromises[siteId] !== undefined) { return this.syncPromises[siteId]; } } @@ -244,7 +244,7 @@ export class CoreSettingsHelperProvider { * @return Promise resolved when synchronized, rejected if failure. */ async synchronizeSite(syncOnlyOnWifi: boolean, siteId: string): Promise { - if (this.syncPromises[siteId]) { + if (this.syncPromises[siteId] !== undefined) { // There's already a sync ongoing for this site, return the promise. return this.syncPromises[siteId]; } diff --git a/src/core/services/filepool.ts b/src/core/services/filepool.ts index d0c00aece..86278c88a 100644 --- a/src/core/services/filepool.ts +++ b/src/core/services/filepool.ts @@ -559,10 +559,19 @@ export class CoreFilepoolProvider { async clearFilepool(siteId: string): Promise { const db = await CoreSites.getSiteDb(siteId); + // Read the data first to be able to notify the deletions. + const filesEntries = await db.getAllRecords(FILES_TABLE_NAME); + const filesLinks = await db.getAllRecords(LINKS_TABLE_NAME); + await Promise.all([ db.deleteRecords(FILES_TABLE_NAME), db.deleteRecords(LINKS_TABLE_NAME), ]); + + // Notify now. + const filesLinksMap = CoreUtils.arrayToObjectMultiple(filesLinks, 'fileId'); + + filesEntries.forEach(entry => this.notifyFileDeleted(siteId, entry.fileId, filesLinksMap[entry.fileId] || [])); } /** From e91e5d1cceb6dd3d6236c510148019037c432a87 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 23 Dec 2021 12:27:57 +0100 Subject: [PATCH 2/2] MOBILE-3897 link: Wait for download to finish before open file --- src/core/directives/external-content.ts | 2 +- src/core/directives/link.ts | 21 +++++++++++++++++++ .../services/module-prefetch-delegate.ts | 7 ++----- src/core/services/filepool.ts | 12 ++++++++--- upgrade.txt | 1 + 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/core/directives/external-content.ts b/src/core/directives/external-content.ts index 50bd68e34..6bbf6d9f4 100644 --- a/src/core/directives/external-content.ts +++ b/src/core/directives/external-content.ts @@ -364,7 +364,7 @@ export class CoreExternalContentDirective implements AfterViewInit, OnChanges, O finalUrl = CoreFile.convertFileSrc(finalUrl); } - if (!CoreUrlUtils.isLocalFileUrl(finalUrl) && !finalUrl.includes('#')) { + if (!CoreUrlUtils.isLocalFileUrl(finalUrl) && !finalUrl.includes('#') && tagName !== 'A') { /* 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 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. diff --git a/src/core/directives/link.ts b/src/core/directives/link.ts index 809934e2b..81670c941 100644 --- a/src/core/directives/link.ts +++ b/src/core/directives/link.ts @@ -26,6 +26,7 @@ import { CoreConstants } from '@/core/constants'; import { CoreContentLinksHelper } from '@features/contentlinks/services/contentlinks-helper'; import { CoreCustomURLSchemes } from '@services/urlschemes'; import { DomSanitizer } from '@singletons'; +import { CoreFilepool } from '@services/filepool'; /** * Directive to open a link in external browser or in the app. @@ -218,6 +219,26 @@ export class CoreLinkDirective implements OnInit { } } + if (currentSite.isSitePluginFileUrl(href)) { + // It's a site file. Check if it's being downloaded right now. + const isDownloading = await CoreFilepool.isFileDownloadingByUrl(currentSite.getId(), href); + + if (isDownloading) { + // Wait for the download to finish before opening the file to prevent downloading it twice. + const modal = await CoreDomUtils.showModalLoading(); + + try { + const path = await CoreFilepool.downloadUrl(currentSite.getId(), href); + + return this.openLocalFile(path); + } catch { + // Error downloading, just open the original URL. + } finally { + modal.dismiss(); + } + } + } + if (this.autoLogin == 'yes') { if (this.inApp) { await currentSite.openInAppWithAutoLogin(href); diff --git a/src/core/features/course/services/module-prefetch-delegate.ts b/src/core/features/course/services/module-prefetch-delegate.ts index 31984758f..a787928db 100644 --- a/src/core/features/course/services/module-prefetch-delegate.ts +++ b/src/core/features/course/services/module-prefetch-delegate.ts @@ -457,13 +457,10 @@ export class CoreCourseModulePrefetchDelegateService extends CoreDelegate { + async isFileDownloadingByUrl(siteId: string, fileUrl: string): Promise { const file = await this.fixPluginfileURL(siteId, fileUrl); const fileId = this.getFileIdByUrl(CoreFileHelper.getFileUrl(file)); - await this.hasFileInQueue(siteId, fileId); + try { + await this.hasFileInQueue(siteId, fileId); + + return true; + } catch { + return false; + } } /** diff --git a/upgrade.txt b/upgrade.txt index 75a4f115f..b373c0db4 100644 --- a/upgrade.txt +++ b/upgrade.txt @@ -8,6 +8,7 @@ information provided here is intended especially for developers. Now you have to pass all items and 3 optional params have been added. - CoreCourseModulePrefetchDelegate.getPrefetchHandlerFor now admits module name instead of full module object. - CoreCourse.getModuleBasicInfoByInstance and CoreCourse.getModuleBasicInfo have been modified to accept an "options" parameter instead of only siteId. +- The function CoreFilepool.isFileDownloadingByUrl now returns Promise instead of relying on resolve/reject. === 3.9.5 ===