Guard against undefined data in translations where semantics has entries

[matthiasblaesing]

It was found that in version 1.24.2 of "Interactive Video" there are entries in the semantics.json file, where the corresponding entries in the translations are missing. This breaks hard in h5peditor.js, function updateCommonFieldsDefault, where the fields from the semantics entries are iterated and field member of the translations entry is accessed without checking, that is actually exists.

[matthiasblaesing]

Steps to reproduce:

1. Install moodle 3.11.10
2. Ensure you use moodle with german locale (tested in german, other locales might show same problem)
3. Install mod_hvp plugin (https://moodle.org/plugins/mod_hvp)
4. Create a course
5. Add a H5P (using the hvp Plugin!) material
6. Add "Course Presentation" to material
7. Ensure the Console in the Browser is open (Stacktrace below was taken with Firefox, but Chrome differed only slightly)
8. Add "Interactive Video" (Dotsmenu -> third last entry)

Resulting view Lade, bitte warten should be replaced with the editor, which does not happen):

In the console:

Uncaught TypeError: translation[i] is undefined
    updateCommonFieldsDefault http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:280
    updateCommonFieldsDefault http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:278
    updateCommonFieldsDefault http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:278
    success http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:230
    jQuery 6
    loadLibrary http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:215
    loadLibrary http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-library.js?ver=2022012000:356
    librariesLoaded http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-library.js?ver=2022012000:326
    getLibraries http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-library-list-cache.js?ver=2022012000:47
    appendTo http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-library.js?ver=2022012000:189
    processSemanticsChunk http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:488
    generateForm http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5PEditor.CoursePresentation-1.24/scripts/cp-editor.js?ver=1.24.3:1471
    processElement http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5PEditor.CoursePresentation-1.24/scripts/cp-editor.js?ver=1.24.3:1692
    attachElement http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5P.CoursePresentation-1.24/dist/h5p-course-presentation.js?ver=1.24.1:1
    addElement http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5PEditor.CoursePresentation-1.24/scripts/cp-editor.js?ver=1.24.3:235
    createElement http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5PEditor.CoursePresentation-1.24/scripts/cp-editor.js?ver=1.24.3:392
    addButton http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5P.DragNBar-1.5/scripts/drag-n-bar.js?ver=1.5.16:741
    jQuery 9
    addButton http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5P.DragNBar-1.5/scripts/drag-n-bar.js?ver=1.5.16:718
    attach http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5P.DragNBar-1.5/scripts/drag-n-bar.js?ver=1.5.16:636
    initializeDNB http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5PEditor.CoursePresentation-1.24/scripts/cp-editor.js?ver=1.24.3:709
    success http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-library-list-cache.js?ver=2022012000:58
    jQuery 6
    getLibraries http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-library-list-cache.js?ver=2022012000:66
    initializeDNB http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5PEditor.CoursePresentation-1.24/scripts/cp-editor.js?ver=1.24.3:520
    appendTo http://moodledev.dev.itmc.tu-dortmund.de/moodle/pluginfile.php/1/mod_hvp/libraries/H5PEditor.CoursePresentation-1.24/scripts/cp-editor.js?ver=1.24.3:268
    processSemanticsChunk http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:488
    processSemantics http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-form.js?ver=2022012000:445
    loadSemantics http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor-library-selector.js?ver=2022012000:281
    success http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:236
    libraryRequested http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:154
    onload http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:90
    onload http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:90
    loadJs http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:88
    libraryRequested http://moodledev.dev.itmc.tu-dortmund.de/moodle/mod/hvp/editor/scripts/h5peditor.js?ver=2022012000:134

The failing code accesses the field member of translation[i] without checking, that this entry exists.

The problem was observed with the version 1.22.4 of the mod_hvp plugin for moodle, using version 1.24.1 of Course Presenation and 1.24.2 of Interactive Video.

[matthiasblaesing]

I just noticed that this issue and the fix suggestion was already reported by @borovinskiy in #157 and has a suggestion for a fix, that matches this. This fix was created independently though.

[fnoks]

I believe this is trying to fix the symptom of h5p/h5p-interactive-video#258. I believe the real issue was that the translations for IV was invalid, and I suspect trying to accept invalid translation files might turn into worse problems later.

[matthiasblaesing]

I disagree. There is already logic in place, that guards against missing data in the translation file:

h5p-editor-php-library/scripts/h5peditor.js

Line 278 in 53dc7bd

translation[i].fields !== undefined && translation[i].fields.length) {

h5p-editor-php-library/scripts/h5peditor.js

Line 282 in 53dc7bd

if (semantics[i].field !== undefined && translation[i].field !== undefined ) {

I don't see how this change makes it worse. It just enhances the existing sanity checks.

We currently have this change in our productive Moodle instance and it changes the situation from "H5P is utterly broken" to "No problem, all good.".

[borovinskiy]

Localization errors are not critical and should not break the application.
Alternatively, throw a warning into the console, but not break the application.

[matthiasblaesing]

@fnoks is there any chance that you reconsider integrating this? I still think, that a program should not rely on input to be well formed and if it is try to do the "most right" thing, which is to go on with the state that it can.

It is fine to ask the content developer to provide a new version of the content element, but as shown in the issue you referenced this can take weeks or upstream is just a dead-end and does not care.