EVOgeek
/
Vmeda.Online
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

MOBILE-2235 h5p: Fix issues identified during PeerReview

main
Dani Palou 2019-12-04 13:37:00 +01:00
parent 1a2ea9485f
commit 5dae06ff81
9 changed files with 141 additions and 165 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
scripts/langindex.json
View File

@ -1620,6 +1620,7 @@
"core.h5p.originator": "h5p",
"core.h5p.pd": "h5p",
"core.h5p.pddl": "h5p",
"core.h5p.play": "local_moodlemobileapp",
"core.h5p.pdm": "h5p",
"core.h5p.resizescript": "h5p",
"core.h5p.resubmitScores": "h5p",

5
src/addon/mod/scorm/providers/prefetch-handler.ts
View File

@ -172,11 +172,6 @@ export class AddonModScormPrefetchHandler extends CoreCourseActivityPrefetchHand
return this.filepoolProvider.downloadUrl(siteId, packageUrl, true, this.component, scorm.coursemodule,
undefined, this.downloadProgress.bind(this, true, onProgress));
}
}).then(() => {
// Remove the destination folder to prevent having old unused files.
return this.fileProvider.removeDir(dirPath).catch(() => {
// Ignore errors, it might have failed because the folder doesn't exist.
});
}).then(() => {
// Get the ZIP file path.
return this.filepoolProvider.getFilePathByUrl(siteId, packageUrl);

16
src/core/h5p/components/h5p-player/h5p-player.ts
View File

@ -148,7 +148,7 @@ export class CoreH5PPlayerComponent implements OnInit, OnChanges, OnDestroy {
if (this.canDownload && (this.state == CoreConstants.OUTDATED || this.state == CoreConstants.NOT_DOWNLOADED)) {
// Download the package in background if the size is low.
this.downloadInBg().catch((error) => {
this.attemptDownloadInBg().catch((error) => {
this.logger.error('Error downloading H5P in background', error);
});
}
@ -188,7 +188,7 @@ export class CoreH5PPlayerComponent implements OnInit, OnChanges, OnDestroy {
*
* @return Promise resolved when done.
*/
protected downloadInBg(): Promise<any> {
protected attemptDownloadInBg(): Promise<any> {
if (this.urlParams && this.src && this.siteCanDownload && this.h5pProvider.canGetTrustedH5PFileInSite() &&
this.appProvider.isOnline()) {
@ -225,8 +225,6 @@ export class CoreH5PPlayerComponent implements OnInit, OnChanges, OnDestroy {
if (this.src && this.siteCanDownload && this.h5pProvider.canGetTrustedH5PFileInSite() && this.site.containsUrl(this.src)) {
this.calculating = true;
this.calculateState();
// Listen for changes in the state.
@ -238,19 +236,21 @@ export class CoreH5PPlayerComponent implements OnInit, OnChanges, OnDestroy {
// An error probably means the file cannot be downloaded or we cannot check it (offline).
});
return;
} else {
this.calculating = false;
this.canDownload = false;
}
this.calculating = false;
this.canDownload = false;
}
/**
* Calcuñate state of the file.
* Calculate state of the file.
*
* @param fileUrl The H5P file URL.
*/
protected calculateState(): void {
this.calculating = true;
// Get the status of the file.
this.filepoolProvider.getFileStateByUrl(this.siteId, this.urlParams.url).then((state) => {
this.canDownload = true;

137
src/core/h5p/providers/h5p.ts
View File

@ -13,7 +13,6 @@
// limitations under the License.
import { Injectable } from '@angular/core';
import { CoreEventsProvider } from '@providers/events';
import { CoreFileProvider } from '@providers/file';
import { CoreFilepoolProvider } from '@providers/filepool';
import { CoreLoggerProvider } from '@providers/logger';
@ -81,6 +80,10 @@ export class CoreH5PProvider {
protected siteSchema: CoreSiteSchema = {
name: 'CoreH5PProvider',
version: 1,
canBeCleared: [
this.CONTENT_TABLE, this.LIBRARIES_TABLE, this.LIBRARY_DEPENDENCIES_TABLE, this.CONTENTS_LIBRARIES_TABLE,
this.LIBRARIES_CACHEDASSETS_TABLE
],
tables: [
{
name: this.CONTENT_TABLE,
@ -101,10 +104,6 @@ export class CoreH5PProvider {
type: 'INTEGER',
notNull: true
},
{
name: 'displayoptions', // Not used right now, but we keep the field to be consistent with Moodle web.
type: 'INTEGER'
},
{
name: 'foldername',
type: 'TEXT',
@ -293,12 +292,11 @@ export class CoreH5PProvider {
]
};
protected ROOT_CACHE_KEY = 'mmH5P:';
protected ROOT_CACHE_KEY = 'CoreH5P:';
protected logger;
constructor(logger: CoreLoggerProvider,
eventsProvider: CoreEventsProvider,
private sitesProvider: CoreSitesProvider,
private textUtils: CoreTextUtilsProvider,
private fileProvider: CoreFileProvider,
@ -312,12 +310,6 @@ export class CoreH5PProvider {
this.logger = logger.getInstance('CoreH5PProvider');
this.sitesProvider.registerSiteSchema(this.siteSchema);
eventsProvider.on(CoreEventsProvider.SITE_STORAGE_DELETED, (data) => {
this.deleteAllData(data.siteId).catch((error) => {
this.logger.error('Error deleting all H5P data from site.', error);
});
});
}
/**
@ -568,24 +560,6 @@ export class CoreH5PProvider {
});
}
/**
* Delete all the H5P data from the DB of a certain site.
*
* @param siteId Site ID.
* @return Promise resolved when done.
*/
protected deleteAllData(siteId: string): Promise<any> {
return this.sitesProvider.getSiteDb(siteId).then((db) => {
return Promise.all([
db.deleteRecords(this.CONTENT_TABLE),
db.deleteRecords(this.LIBRARIES_TABLE),
db.deleteRecords(this.LIBRARY_DEPENDENCIES_TABLE),
db.deleteRecords(this.CONTENTS_LIBRARIES_TABLE),
db.deleteRecords(this.LIBRARIES_CACHEDASSETS_TABLE)
]);
});
}
/**
* Delete cached assets from DB and filesystem.
*
@ -784,14 +758,8 @@ export class CoreH5PProvider {
const folderName = this.mimeUtils.removeExtension(file.name),
destFolder = this.textUtils.concatenatePaths(CoreFileProvider.TMPFOLDER, 'h5p/' + folderName);
// Make sure the dest dir doesn't exist already.
return this.fileProvider.removeDir(destFolder).catch(() => {
// Ignore errors.
}).then(() => {
return this.fileProvider.createDir(destFolder);
}).then(() => {
return this.fileProvider.unzipFile(file.toURL(), destFolder);
}).then(() => {
// Unzip the file.
return this.fileProvider.unzipFile(file.toURL(), destFolder).then(() => {
// Read the contents of the unzipped dir.
return this.fileProvider.getDirectoryContents(destFolder);
}).then((contents) => {
@ -830,11 +798,6 @@ export class CoreH5PProvider {
return Promise.reject(error);
});
});
}).then(() => {
// Remove tmp folder.
return this.fileProvider.removeDir(destFolder).catch(() => {
// Ignore errors, it will be deleted eventually.
});
}).then(() => {
// Create the content player.
@ -843,6 +806,11 @@ export class CoreH5PProvider {
return this.createContentIndex(content.id, fileUrl, contentData, embedType, siteId);
});
}).finally(() => {
// Remove tmp folder.
return this.fileProvider.removeDir(destFolder).catch(() => {
// Ignore errors, it will be deleted eventually.
});
});
});
});
@ -1002,6 +970,40 @@ export class CoreH5PProvider {
});
}
/**
* Validate and fix display options, updating them if needed.
*
* @param displayOptions The display options to validate.
* @param id Package ID.
*/
fixDisplayOptions(displayOptions: CoreH5PDisplayOptions, id: number): CoreH5PDisplayOptions {
// Never allow downloading in the app.
displayOptions[CoreH5PProvider.DISPLAY_OPTION_DOWNLOAD] = false;
// Embed - force setting it if always on or always off. In web, this is done when storing in DB.
const embed = this.getOption(CoreH5PProvider.DISPLAY_OPTION_EMBED, CoreH5PDisplayOptionBehaviour.ALWAYS_SHOW);
if (embed == CoreH5PDisplayOptionBehaviour.ALWAYS_SHOW || embed == CoreH5PDisplayOptionBehaviour.NEVER_SHOW) {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_EMBED] = (embed == CoreH5PDisplayOptionBehaviour.ALWAYS_SHOW);
}
if (!this.getOption(CoreH5PProvider.DISPLAY_OPTION_FRAME, true)) {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_FRAME] = false;
} else {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_EMBED] = this.setDisplayOptionOverrides(
CoreH5PProvider.DISPLAY_OPTION_EMBED, CoreH5PPermission.EMBED_H5P, id,
displayOptions[CoreH5PProvider.DISPLAY_OPTION_EMBED]);
if (this.getOption(CoreH5PProvider.DISPLAY_OPTION_COPYRIGHT, true) == false) {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_COPYRIGHT] = false;
}
}
displayOptions[CoreH5PProvider.DISPLAY_OPTION_COPY] = this.hasPermission(CoreH5PPermission.COPY_H5P, id);
return displayOptions;
}
/**
* Get the assets of a package.
*
@ -1215,7 +1217,7 @@ export class CoreH5PProvider {
}).then((url) => {
// Add display options to the URL.
return this.getContentDataByUrl(fileUrl, siteId).then((data) => {
const options = this.validateDisplayOptions(this.getDisplayOptionsFromUrlParams(urlParams), data.id);
const options = this.fixDisplayOptions(this.getDisplayOptionsFromUrlParams(urlParams), data.id);
return this.urlUtils.addParamsToUrl(url, options, undefined, true);
});
@ -1478,7 +1480,7 @@ export class CoreH5PProvider {
* @return Display options as object.
*/
getDisplayOptionsForView(disable: number, id: number): CoreH5PDisplayOptions {
return this.validateDisplayOptions(this.getDisplayOptionsAsObject(disable), id);
return this.fixDisplayOptions(this.getDisplayOptionsAsObject(disable), id);
}
/**
@ -1733,8 +1735,8 @@ export class CoreH5PProvider {
* @return Return the value for this display option.
*/
getOption(name: string, defaultValue: any = false): any {
// For now, all them are disabled by default, so only will be rendered when defined in the displayoptions DB field.
return CoreH5PDisplayOptionBehaviour.CONTROLLED_BY_AUTHOR_DEFAULT_OFF; // CONTROLLED_BY_AUTHOR_DEFAULT_OFF.
// For now, all them are disabled by default, so only will be rendered when defined in the display options.
return CoreH5PDisplayOptionBehaviour.CONTROLLED_BY_AUTHOR_DEFAULT_OFF;
}
/**
@ -1776,7 +1778,7 @@ export class CoreH5PProvider {
*
* @param url The file URL.
* @param options Options.
* @param ignoreCache Whether to ignore cache..
* @param ignoreCache Whether to ignore cache.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file data.
*/
@ -2135,7 +2137,6 @@ export class CoreH5PProvider {
*
* @param fileUrl The file URL used to download the file.
* @param file The file entry of the downloaded file.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved when done.
*/
protected processH5PFiles(destFolder: string, entries: (DirectoryEntry | FileEntry)[])
@ -2227,7 +2228,6 @@ export class CoreH5PProvider {
const data: any = {
jsoncontent: content.params,
displayoptions: null,
mainlibraryid: content.library.libraryId,
timemodified: Date.now(),
filtered: null,
@ -2597,40 +2597,6 @@ export class CoreH5PProvider {
return db.updateRecords(this.CONTENT_TABLE, data, {id: id});
});
}
/**
* Validate display options, updating them if needed.
*
* @param displayOptions The display options to validate.
* @param id Package ID.
*/
validateDisplayOptions(displayOptions: CoreH5PDisplayOptions, id: number): CoreH5PDisplayOptions {
// Never allow downloading in the app.
displayOptions[CoreH5PProvider.DISPLAY_OPTION_DOWNLOAD] = false;
// Embed - force setting it if always on or always off. In web, this is done when storing in DB.
const embed = this.getOption(CoreH5PProvider.DISPLAY_OPTION_EMBED, CoreH5PDisplayOptionBehaviour.ALWAYS_SHOW);
if (embed == CoreH5PDisplayOptionBehaviour.ALWAYS_SHOW || embed == CoreH5PDisplayOptionBehaviour.NEVER_SHOW) {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_EMBED] = (embed == CoreH5PDisplayOptionBehaviour.ALWAYS_SHOW);
}
if (this.getOption(CoreH5PProvider.DISPLAY_OPTION_FRAME, true) == false) {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_FRAME] = false;
} else {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_EMBED] = this.setDisplayOptionOverrides(
CoreH5PProvider.DISPLAY_OPTION_EMBED, CoreH5PPermission.EMBED_H5P, id,
displayOptions[CoreH5PProvider.DISPLAY_OPTION_EMBED]);
if (this.getOption(CoreH5PProvider.DISPLAY_OPTION_COPYRIGHT, true) == false) {
displayOptions[CoreH5PProvider.DISPLAY_OPTION_COPYRIGHT] = false;
}
}
displayOptions[CoreH5PProvider.DISPLAY_OPTION_COPY] = this.hasPermission(CoreH5PPermission.COPY_H5P, id);
return displayOptions;
}
}
/**
@ -2701,7 +2667,6 @@ export type CoreH5PContentDBData = {
id: number; // The id of the content.
jsoncontent: string; // The content in json format.
mainlibraryid: number; // The library we first instantiate for this node.
displayoptions: number; // H5P Button display options. Not used right now.
foldername: string; // Name of the folder that contains the contents.
fileurl: string; // The online URL of the H5P package.
filtered: string; // Filtered version of json_content.

24
src/core/h5p/providers/pluginfile-handler.ts
View File

@ -37,17 +37,6 @@ export class CoreH5PPluginFileHandler implements CorePluginFileHandler {
protected fileProvider: CoreFileProvider,
protected h5pProvider: CoreH5PProvider) { }
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
canDownloadFile(file: CoreWSExternalFile, siteId?: string): Promise<CoreWSExternalFile> {
return this.h5pProvider.getTrustedH5PFile(file.fileurl, {}, false, siteId);
}
/**
* React to a file being deleted.
*
@ -61,6 +50,17 @@ export class CoreH5PPluginFileHandler implements CorePluginFileHandler {
return this.h5pProvider.deleteContentByUrl(fileUrl, siteId);
}
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
getDownloadableFile(file: CoreWSExternalFile, siteId?: string): Promise<CoreWSExternalFile> {
return this.h5pProvider.getTrustedH5PFile(file.fileurl, {}, false, siteId);
}
/**
* Given an HTML element, get the URLs of the files that should be downloaded and weren't treated by
* CoreDomUtilsProvider.extractDownloadableFilesFromHtml.
@ -68,7 +68,7 @@ export class CoreH5PPluginFileHandler implements CorePluginFileHandler {
* @param container Container where to get the URLs from.
* @return {string[]} List of URLs.
*/
getDownloadableFiles(container: HTMLElement): string[] {
getDownloadableFilesFromHTML(container: HTMLElement): string[] {
const iframes = <HTMLIFrameElement[]> Array.from(container.querySelectorAll('iframe.h5p-iframe'));
const urls = [];

23
src/providers/file.ts
View File

@ -762,7 +762,7 @@ export class CoreFileProvider {
* @return Promise resolved when the entry is moved.
*/
moveDir(originalPath: string, newPath: string, destDirExists?: boolean): Promise<any> {
return this.moveFileOrDir(originalPath, newPath, true);
return this.moveFileOrDir(originalPath, newPath, true, destDirExists);
}
/**
@ -775,7 +775,7 @@ export class CoreFileProvider {
* @return Promise resolved when the entry is moved.
*/
moveFile(originalPath: string, newPath: string, destDirExists?: boolean): Promise<any> {
return this.moveFileOrDir(originalPath, newPath, false);
return this.moveFileOrDir(originalPath, newPath, false, destDirExists);
}
/**
@ -991,11 +991,26 @@ export class CoreFileProvider {
* @param destFolder Path to the destination folder. If not defined, a new folder will be created with the
* same location and name as the ZIP file (without extension).
* @param onProgress Function to call on progress.
* @param recreateDir Delete the dest directory before unzipping. Defaults to true.
* @return Promise resolved when the file is unzipped.
*/
unzipFile(path: string, destFolder?: string, onProgress?: Function): Promise<any> {
unzipFile(path: string, destFolder?: string, onProgress?: Function, recreateDir: boolean = true): Promise<any> {
// Get the source file.
return this.getFile(path).then((fileEntry) => {
let fileEntry: FileEntry;
return this.getFile(path).then((fe) => {
fileEntry = fe;
if (destFolder && recreateDir) {
// Make sure the dest dir doesn't exist already.
return this.removeDir(destFolder).catch(() => {
// Ignore errors.
}).then(() => {
// Now create the dir, otherwise if any of the ancestor dirs doesn't exist the unzip would fail.
return this.createDir(destFolder);
});
}
}).then(() => {
// If destFolder is not set, use same location as ZIP file. We need to use absolute paths (including basePath).
destFolder = this.addBasePathIfNeeded(destFolder || this.mimeUtils.removeExtension(path));

2
src/providers/filepool.ts
View File

@ -1339,7 +1339,7 @@ export class CoreFilepoolProvider {
*/
protected fixPluginfileURL(siteId: string, fileUrl: string, timemodified: number = 0): Promise<CoreWSExternalFile> {
return this.pluginFileDelegate.canDownloadFile({fileurl: fileUrl, timemodified: timemodified}).then((file) => {
return this.pluginFileDelegate.getDownloadableFile({fileurl: fileUrl, timemodified: timemodified}).then((file) => {
return this.sitesProvider.getSite(siteId).then((site) => {
return site.checkAndFixPluginfileURL(file.fileurl);

96
src/providers/plugin-file-delegate.ts
View File

@ -48,15 +48,6 @@ export interface CorePluginFileHandler {
*/
getComponentRevisionReplace?(args: string[]): string;
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
canDownloadFile?(file: CoreWSExternalFile, siteId?: string): Promise<CoreWSExternalFile>;
/**
* React to a file being deleted.
*
@ -67,6 +58,15 @@ export interface CorePluginFileHandler {
*/
fileDeleted?(fileUrl: string, path: string, siteId?: string): Promise<any>;
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
getDownloadableFile?(file: CoreWSExternalFile, siteId?: string): Promise<CoreWSExternalFile>;
/**
* Given an HTML element, get the URLs of the files that should be downloaded and weren't treated by
* CoreDomUtilsProvider.extractDownloadableFilesFromHtml.
@ -74,7 +74,7 @@ export interface CorePluginFileHandler {
* @param container Container where to get the URLs from.
* @return {string[]} List of URLs.
*/
getDownloadableFiles?(container: HTMLElement): string[];
getDownloadableFilesFromHTML?(container: HTMLElement): string[];
/**
* Get a file size.
@ -116,39 +116,6 @@ export class CorePluginFileDelegate {
this.logger = logger.getInstance('CorePluginFileDelegate');
}
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
canDownloadFile(file: CoreWSExternalFile, siteId?: string): Promise<CoreWSExternalFile> {
const handler = this.getHandlerForFile(file);
return this.canHandlerDownloadFile(file, handler, siteId);
}
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param handler The handler to use.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
protected canHandlerDownloadFile(file: CoreWSExternalFile, handler: CorePluginFileHandler, siteId?: string)
: Promise<CoreWSExternalFile> {
if (handler && handler.canDownloadFile) {
return handler.canDownloadFile(file, siteId).then((newFile) => {
return newFile || file;
});
}
return Promise.resolve(file);
}
/**
* React to a file being deleted.
*
@ -167,6 +134,39 @@ export class CorePluginFileDelegate {
return Promise.resolve();
}
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
getDownloadableFile(file: CoreWSExternalFile, siteId?: string): Promise<CoreWSExternalFile> {
const handler = this.getHandlerForFile(file);
return this.getHandlerDownloadableFile(file, handler, siteId);
}
/**
* Check whether a file can be downloaded. If so, return the file to download.
*
* @param file The file data.
* @param handler The handler to use.
* @param siteId Site ID. If not defined, current site.
* @return Promise resolved with the file to use. Rejected if cannot download.
*/
protected getHandlerDownloadableFile(file: CoreWSExternalFile, handler: CorePluginFileHandler, siteId?: string)
: Promise<CoreWSExternalFile> {
if (handler && handler.getDownloadableFile) {
return handler.getDownloadableFile(file, siteId).then((newFile) => {
return newFile || file;
});
}
return Promise.resolve(file);
}
/**
* Get the handler for a certain pluginfile url.
*
@ -201,14 +201,14 @@ export class CorePluginFileDelegate {
* @param container Container where to get the URLs from.
* @return List of URLs.
*/
getDownloadableFiles(container: HTMLElement): string[] {
getDownloadableFilesFromHTML(container: HTMLElement): string[] {
let files = [];
for (const component in this.handlers) {
const handler = this.handlers[component];
if (handler && handler.getDownloadableFiles) {
files = files.concat(handler.getDownloadableFiles(container));
if (handler && handler.getDownloadableFilesFromHTML) {
files = files.concat(handler.getDownloadableFilesFromHTML(container));
}
}
@ -256,8 +256,8 @@ export class CorePluginFileDelegate {
const handler = this.getHandlerForFile(file);
// First of all check if file can be downloaded.
return this.canHandlerDownloadFile(file, handler, siteId).then((canDownload) => {
if (!canDownload) {
return this.getHandlerDownloadableFile(file, handler, siteId).then((file) => {
if (!file) {
return 0;
}

2
src/providers/utils/dom.ts
View File

@ -286,7 +286,7 @@ export class CoreDomUtilsProvider {
}
// Now get other files from plugin file handlers.
urls = urls.concat(this.pluginFileDelegate.getDownloadableFiles(element));
urls = urls.concat(this.pluginFileDelegate.getDownloadableFilesFromHTML(element));
return urls;
}