From edf576380982821a95142d87607ca34e6b23f0b6 Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Wed, 9 Jun 2021 11:38:05 +0200 Subject: [PATCH] MOBILE-3320 resource: Fix 'Open with' with streaming and external repos --- scripts/langindex.json | 1 + .../index/addon-mod-resource-index.html | 13 +++-- .../mod/resource/components/index/index.ts | 52 +++++++++++++++---- .../features/course/services/course-helper.ts | 34 ++++++++++-- src/core/lang.json | 1 + src/core/services/file-helper.ts | 16 +++++- src/core/services/filepool.ts | 14 +++-- src/core/services/utils/mimetype.ts | 10 ++++ src/core/services/utils/utils.ts | 15 +++++- 9 files changed, 129 insertions(+), 27 deletions(-) diff --git a/scripts/langindex.json b/scripts/langindex.json index 79e24efe1..0816e51a6 100644 --- a/scripts/langindex.json +++ b/scripts/langindex.json @@ -1993,6 +1993,7 @@ "core.percentagenumber": "local_moodlemobileapp", "core.phone": "moodle", "core.pictureof": "moodle", + "core.play": "local_moodlemobileapp", "core.previous": "moodle", "core.proceed": "moodle", "core.pulltorefresh": "local_moodlemobileapp", diff --git a/src/addons/mod/resource/components/index/addon-mod-resource-index.html b/src/addons/mod/resource/components/index/addon-mod-resource-index.html index 1d66affb9..68d143323 100644 --- a/src/addons/mod/resource/components/index/addon-mod-resource-index.html +++ b/src/addons/mod/resource/components/index/addon-mod-resource-index.html @@ -47,11 +47,18 @@ - - {{ 'addon.mod_resource.openthefile' | translate }} + + + {{ 'core.play' | translate }} + + + + {{ 'addon.mod_resource.openthefile' | translate }} + - + {{ 'core.openwith' | translate }} diff --git a/src/addons/mod/resource/components/index/index.ts b/src/addons/mod/resource/components/index/index.ts index 1245be191..aca96abae 100644 --- a/src/addons/mod/resource/components/index/index.ts +++ b/src/addons/mod/resource/components/index/index.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { Component, OnInit, Optional } from '@angular/core'; +import { Component, OnDestroy, OnInit, Optional } from '@angular/core'; import { CoreError } from '@classes/errors/error'; import { CoreCourseModuleMainResourceComponent, @@ -21,10 +21,13 @@ import { CoreCourseContentsPage } from '@features/course/pages/contents/contents import { CoreCourse, CoreCourseWSModule } from '@features/course/services/course'; import { CoreCourseModulePrefetchDelegate } from '@features/course/services/module-prefetch-delegate'; import { CoreApp } from '@services/app'; +import { CoreFileHelper } from '@services/file-helper'; import { CoreSites } from '@services/sites'; +import { CoreMimetypeUtils } from '@services/utils/mimetype'; import { CoreTextUtils } from '@services/utils/text'; import { CoreUtils, OpenFileAction } from '@services/utils/utils'; -import { Translate } from '@singletons'; +import { Network, NgZone, Translate } from '@singletons'; +import { Subscription } from 'rxjs'; import { AddonModResource, AddonModResourceCustomData, @@ -40,7 +43,7 @@ import { AddonModResourceHelper } from '../../services/resource-helper'; selector: 'addon-mod-resource-index', templateUrl: 'addon-mod-resource-index.html', }) -export class AddonModResourceIndexComponent extends CoreCourseModuleMainResourceComponent implements OnInit { +export class AddonModResourceIndexComponent extends CoreCourseModuleMainResourceComponent implements OnInit, OnDestroy { component = AddonModResourceProvider.COMPONENT; @@ -52,19 +55,35 @@ export class AddonModResourceIndexComponent extends CoreCourseModuleMainResource warning = ''; isIOS = false; openFileAction = OpenFileAction; + isOnline = false; + isStreamedFile = false; + shouldOpenInBrowser = false; + + protected onlineObserver?: Subscription; constructor(@Optional() courseContentsPage?: CoreCourseContentsPage) { super('AddonModResourceIndexComponent', courseContentsPage); } /** - * Component being initialized. + * @inheritdoc */ async ngOnInit(): Promise { super.ngOnInit(); this.canGetResource = AddonModResource.isGetResourceWSAvailable(); this.isIOS = CoreApp.isIOS(); + this.isOnline = CoreApp.isOnline(); + + if (this.isIOS) { + // Refresh online status when changes. + this.onlineObserver = Network.onChange().subscribe(() => { + // Execute the callback in the Angular zone, so change detection doesn't stop working. + NgZone.run(() => { + this.isOnline = CoreApp.isOnline(); + }); + }); + } await this.loadContent(); try { @@ -76,19 +95,14 @@ export class AddonModResourceIndexComponent extends CoreCourseModuleMainResource } /** - * Perform the invalidate content function. - * - * @return Resolved when done. + * @inheritdoc */ protected async invalidateContent(): Promise { return AddonModResource.invalidateContent(this.module.id, this.courseId); } /** - * Download resource contents. - * - * @param refresh Whether we're refreshing data. - * @return Promise resolved when done. + * @inheritdoc */ protected async fetchContent(refresh?: boolean): Promise { // Load module contents if needed. Passing refresh is needed to force reloading contents. @@ -150,6 +164,14 @@ export class AddonModResourceIndexComponent extends CoreCourseModuleMainResource } else { this.mode = 'external'; this.warning = ''; + + if (this.isIOS) { + this.shouldOpenInBrowser = CoreFileHelper.shouldOpenInBrowser(this.module.contents[0]); + } + + const mimetype = await CoreUtils.getMimeTypeFromUrl(CoreFileHelper.getFileUrl(this.module.contents[0])); + + this.isStreamedFile = CoreMimetypeUtils.isStreamedMimetype(mimetype); } } finally { this.fillContextMenu(refresh); @@ -179,4 +201,12 @@ export class AddonModResourceIndexComponent extends CoreCourseModuleMainResource await CoreSites.getCurrentSite()?.openInBrowserWithAutoLoginIfSameSite(this.module.url!); } + /** + * @inheritdoc + */ + ngOnDestroy(): void { + super.ngOnDestroy(); + this.onlineObserver?.unsubscribe(); + } + } diff --git a/src/core/features/course/services/course-helper.ts b/src/core/features/course/services/course-helper.ts index 1f2976402..64d1d832d 100644 --- a/src/core/features/course/services/course-helper.ts +++ b/src/core/features/course/services/course-helper.ts @@ -691,11 +691,19 @@ export class CoreCourseHelperProvider { // Check if the file should be opened in browser. if (CoreFileHelper.shouldOpenInBrowser(mainFile)) { - return this.openModuleFileInBrowser(mainFile.fileurl, site, module, courseId, component, componentId, files); + return this.openModuleFileInBrowser(mainFile.fileurl, site, module, courseId, component, componentId, files, options); } // File shouldn't be opened in browser. Download the module if it needs to be downloaded. - const result = await this.downloadModuleWithMainFileIfNeeded(module, courseId, component || '', componentId, files, siteId); + const result = await this.downloadModuleWithMainFileIfNeeded( + module, + courseId, + component || '', + componentId, + files, + siteId, + options, + ); if (CoreUrlUtils.isLocalFileUrl(result.path)) { return CoreUtils.openFile(result.path, options); @@ -740,6 +748,7 @@ export class CoreCourseHelperProvider { * @param component The component to link the files to. * @param componentId An ID to use in conjunction with the component. * @param files List of files of the module. If not provided, use module.contents. + * @param options Options to open the file. Only used if not opened in browser. * @return Resolved on success. */ protected async openModuleFileInBrowser( @@ -750,6 +759,7 @@ export class CoreCourseHelperProvider { component?: string, componentId?: string | number, files?: CoreCourseModuleContentFile[], + options: CoreUtilsOpenFileOptions = {}, ): Promise { if (!CoreApp.isOnline()) { // Not online, get the offline file. It will fail if not found. @@ -760,7 +770,7 @@ export class CoreCourseHelperProvider { throw new CoreNetworkError(); } - return CoreUtils.openFile(path); + return CoreUtils.openFile(path, options); } // Open in browser. @@ -791,6 +801,7 @@ export class CoreCourseHelperProvider { * @param componentId An ID to use in conjunction with the component. * @param files List of files of the module. If not provided, use module.contents. * @param siteId The site ID. If not defined, current site. + * @param options Options to open the file. * @return Promise resolved when done. */ async downloadModuleWithMainFileIfNeeded( @@ -800,6 +811,7 @@ export class CoreCourseHelperProvider { componentId?: string | number, files?: CoreCourseModuleContentFile[], siteId?: string, + options: CoreUtilsOpenFileOptions = {}, ): Promise<{ fixedUrl: string; path: string; status?: string }> { siteId = siteId || CoreSites.getCurrentSiteId(); @@ -840,7 +852,17 @@ export class CoreCourseHelperProvider { } if (!path) { - path = await this.downloadModuleWithMainFile(module, courseId, fixedUrl, files, status, component, componentId, siteId); + path = await this.downloadModuleWithMainFile( + module, + courseId, + fixedUrl, + files, + status, + component, + componentId, + siteId, + options, + ); } return { @@ -862,6 +884,7 @@ export class CoreCourseHelperProvider { * @param component The component to link the files to. * @param componentId An ID to use in conjunction with the component. * @param siteId The site ID. If not defined, current site. + * @param options Options to open the file. * @return Promise resolved when done. */ protected async downloadModuleWithMainFile( @@ -873,6 +896,7 @@ export class CoreCourseHelperProvider { component?: string, componentId?: string | number, siteId?: string, + options: CoreUtilsOpenFileOptions = {}, ): Promise { siteId = siteId || CoreSites.getCurrentSiteId(); @@ -885,7 +909,7 @@ export class CoreCourseHelperProvider { throw new CoreNetworkError(); } - const shouldDownloadFirst = await CoreFilepool.shouldDownloadFileBeforeOpen(fixedUrl, mainFile.filesize); + const shouldDownloadFirst = await CoreFilepool.shouldDownloadFileBeforeOpen(fixedUrl, mainFile.filesize, options); if (shouldDownloadFirst) { // Download and then return the local URL. diff --git a/src/core/lang.json b/src/core/lang.json index 057501d38..b4d4d57f0 100644 --- a/src/core/lang.json +++ b/src/core/lang.json @@ -225,6 +225,7 @@ "percentagenumber": "{{$a}}%", "phone": "Phone", "pictureof": "Picture of {{$a}}", + "play": "Play", "previous": "Previous", "proceed": "Proceed", "pulltorefresh": "Pull to refresh", diff --git a/src/core/services/file-helper.ts b/src/core/services/file-helper.ts index 4df8e5e71..17b5f0d21 100644 --- a/src/core/services/file-helper.ts +++ b/src/core/services/file-helper.ts @@ -73,7 +73,17 @@ export class CoreFileHelperProvider { await this.showConfirmOpenUnsupportedFile(); } - let url = await this.downloadFileIfNeeded(file, fileUrl, component, componentId, timemodified, state, onProgress, siteId); + let url = await this.downloadFileIfNeeded( + file, + fileUrl, + component, + componentId, + timemodified, + state, + onProgress, + siteId, + options, + ); if (!url) { return; @@ -127,6 +137,7 @@ export class CoreFileHelperProvider { * @param state The file's state. If not provided, it will be calculated. * @param onProgress Function to call on progress. * @param siteId The site ID. If not defined, current site. + * @param options Options to open the file. * @return Resolved with the URL to use on success. */ protected async downloadFileIfNeeded( @@ -138,6 +149,7 @@ export class CoreFileHelperProvider { state?: string, onProgress?: CoreFileHelperOnProgress, siteId?: string, + options: CoreUtilsOpenFileOptions = {}, ): Promise { siteId = siteId || CoreSites.getCurrentSiteId(); @@ -172,7 +184,7 @@ export class CoreFileHelperProvider { onProgress({ calculating: true }); } - const shouldDownloadFirst = await CoreFilepool.shouldDownloadFileBeforeOpen(fixedUrl, file.filesize || 0); + const shouldDownloadFirst = await CoreFilepool.shouldDownloadFileBeforeOpen(fixedUrl, file.filesize || 0, options); if (shouldDownloadFirst) { // Download the file first. if (state == CoreConstants.DOWNLOADING) { diff --git a/src/core/services/filepool.ts b/src/core/services/filepool.ts index a819c6963..3cc4c2854 100644 --- a/src/core/services/filepool.ts +++ b/src/core/services/filepool.ts @@ -26,7 +26,7 @@ import { CoreMimetypeUtils } from '@services/utils/mimetype'; import { CoreTextUtils } from '@services/utils/text'; import { CoreTimeUtils } from '@services/utils/time'; import { CoreUrlUtils } from '@services/utils/url'; -import { CoreUtils, PromiseDefer } from '@services/utils/utils'; +import { CoreUtils, CoreUtilsOpenFileOptions, PromiseDefer } from '@services/utils/utils'; import { SQLiteDB } from '@classes/sqlitedb'; import { CoreError } from '@classes/errors/error'; import { CoreConstants } from '@/core/constants'; @@ -2781,7 +2781,7 @@ export class CoreFilepoolProvider { const mimetype = await CoreUtils.getMimeTypeFromUrl(url); // If the file is streaming (audio or video) we reject. - if (mimetype.indexOf('video') != -1 || mimetype.indexOf('audio') != -1) { + if (CoreMimetypeUtils.isStreamedMimetype(mimetype)) { throw new CoreError('File is audio or video.'); } } @@ -2791,6 +2791,7 @@ export class CoreFilepoolProvider { * * @param url File online URL. * @param size File size. + * @param options Options. * @return Promise resolved with boolean: whether file should be downloaded before opening it. * @description * Convenience function to check if a file should be downloaded before opening it. @@ -2800,16 +2801,21 @@ export class CoreFilepoolProvider { * - The file cannot be streamed. * If the file is big and can be streamed, the promise returned by this function will be rejected. */ - async shouldDownloadFileBeforeOpen(url: string, size: number): Promise { + async shouldDownloadFileBeforeOpen(url: string, size: number, options: CoreUtilsOpenFileOptions = {}): Promise { if (size >= 0 && size <= CoreFilepoolProvider.DOWNLOAD_THRESHOLD) { // The file is small, download it. return true; } + if (CoreUtils.shouldOpenWithDialog(options)) { + // Open with dialog needs a local file. + return true; + } + const mimetype = await CoreUtils.getMimeTypeFromUrl(url); // If the file is streaming (audio or video), return false. - return mimetype.indexOf('video') == -1 && mimetype.indexOf('audio') == -1; + return !CoreMimetypeUtils.isStreamedMimetype(mimetype); } /** diff --git a/src/core/services/utils/mimetype.ts b/src/core/services/utils/mimetype.ts index 92ec8ab48..9f5526ef3 100644 --- a/src/core/services/utils/mimetype.ts +++ b/src/core/services/utils/mimetype.ts @@ -562,6 +562,16 @@ export class CoreMimetypeUtilsProvider { return false; } + /** + * Check if a mimetype belongs to a file that can be streamed (audio, video). + * + * @param mimetype Mimetype. + * @return Boolean. + */ + isStreamedMimetype(mimetype: string): boolean { + return mimetype.indexOf('video') != -1 || mimetype.indexOf('audio') != -1; + } + /** * Remove the extension from a path (if any). * diff --git a/src/core/services/utils/utils.ts b/src/core/services/utils/utils.ts index 9849c55ce..7777cd262 100644 --- a/src/core/services/utils/utils.ts +++ b/src/core/services/utils/utils.ts @@ -933,8 +933,7 @@ export class CoreUtilsProvider { } try { - const openFileAction = options.iOSOpenFileAction ?? CoreConstants.CONFIG.iOSDefaultOpenFileAction; - if (CoreApp.isIOS() && openFileAction == OpenFileAction.OPEN_WITH) { + if (this.shouldOpenWithDialog(options)) { await FileOpener.showOpenWithDialog(path, mimetype || ''); } else { await FileOpener.open(path, mimetype || ''); @@ -1652,6 +1651,18 @@ export class CoreUtilsProvider { return this.wait(0); } + /** + * Given some options, check if a file should be opened with showOpenWithDialog. + * + * @param options Options. + * @return Boolean. + */ + shouldOpenWithDialog(options: CoreUtilsOpenFileOptions = {}): boolean { + const openFileAction = options.iOSOpenFileAction ?? CoreConstants.CONFIG.iOSDefaultOpenFileAction; + + return CoreApp.isIOS() && openFileAction == OpenFileAction.OPEN_WITH; + } + } export const CoreUtils = makeSingleton(CoreUtilsProvider);