-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resize working randomly with embedded H5P content #314
Comments
any progress on this @thomasmars |
Any progress on this - I'm getting the same issue |
I am experiencing a similar issue in Moodle when inserting an H5P resource into a page. On most loads, the resource loads with the incorrect height. After reloading the page a few times the height of the resource loads correctly. |
We are experiencing the same problem on our instance when embedding H5P Content. Sometimes it does resize correctly but sometimes not. We could recreate the problem with several different Browsers (Chrome, Firefox and Safari). H5P Version: 1.21.0 For the case the video itself doesn't resize properly, we found that on the iframe object the style properties width and height are missing. When it is resizing correctly, it seems the resizer code adds the style attribute with width and height.
In addition I have to mention, that we were not able to reproduce this on a local development system, only on a running server over web. We should maybe consider due to inconsistency of the bug, that we're maybe facing a timing problem of the function calls in JS. |
@thomasmars do you have a hint or a clue what could be the problem here? |
We have also seen this happening with several different browsers lately. |
@dnutzz Yes, it seems likely that this is a timing issue. Do you have a public URL where this can be reproduced, the steps to reproduce it and the browsers and browser versions this can be seen in ? Or alternatively, can you share the H5P where you can reproduce this ? |
Unfortunately we have no public courses (in terms of accessing them without an account). But all our courses are free, so I can only invite you to create a free accout (which you can delete afterwards) here and then visit this course. |
@dnutzz Are you by any chance using some custom H5P iframe integration that's similar to the one that oncampus (formerly mooin) is using for their platform? That one had bugs which could lead to two competing resize functions on the iframe which a) could result in flickering and b) could lead to strange resize behavior. |
@thomasmars We experience this issue specifically with H5P built into core Moodle. |
@dnutzz As Otacke is saying, you seem to be using a custom iframe solution, the H5P embed code is wrapped within a "iframeplayer":
That iframe is limiting the size of the H5P embed. You'll have to solve it for that integration, I don't know if this is something theme specific or something special that you have implemented yourselves. Here is a working iframe from your iframeplayer:
Notice the difference in that the style has been applied to this iframe @anrichp Please contact Moodle core if you're using the H5P integration they've made for core, or optionally you can use the H5P plugin as provided here and I'll be able to look further into it if you're experiencing the same problem. For anyone else experiencing this problem. Please check if there can be any problems arising from conflict with other javascript, themes or special embed techniques you are using. You can check this by embedding the H5P embed on a clean page, if this works then it is likely something to do with your setup, and you can find the likely source of the issue by disabling themes/plugins/special scripts one by one until you find what has caused the issue and then contact their support, get rid of it or look into how the issue can be amended. I'll be happy to look into any issues where you the issue is reproducible and you can rule out any interference of third party themes/scripts. Thanks. |
@thomasmars thanks for pointing this out. On those cases it's not working, the style attribute stays empty (I think the js code does not get the hook on the element right > timing). |
Here is another example of a H5P embed on a Page activity, that doesn't require logging in. Usually I can see the resize issue right after loading the activity, but sometimes I need to refresh the page a couple of times. I did some testing in that Moodle installation. At first I couldn't reproduce the issue with H5P Moodle-plugin version There are no third-party themes installed in that Moodle. There are a few additional plugins, but I don't think they are causing any problems. |
Can reproduce it on Firefox 81. It occurs randomly. When it's working, the style attribute is injected on the iframe wrapper (which is copy pasted from the hvp embed code modal). When it's not resized correctly, the style attribute is missing (see screenshots). This hook happens asynchronously. Video example: https://youtu.be/ahStxagJeGE Not resized:Resized: |
Thanks for the example. |
Hello, I would like to see this fixed. |
Hey, This bug is causing a lot of problems with many H5P users. I see that the Jira issue is still unassigned and priority only 'Medium'. Is the issue being worked at? Many Moodle courses are unusable and teachers are trying to find other ways or tools to deliver their content. |
I would like to see an fix to this |
I just ran into this problem as well. |
We've deployed this change to a production site and it hasn't had a noticeable effect on the issue. |
I have a similar problem on a Moodle 3.8 Plattform with Adaptable Design. The problem appears in different Browsers and could not be solved by simple adaption of the HTML-Code of the IFrame. It is described also here in this Moodle Forum Post & Discussion and here. Did anyone find a simple workaround? |
I've found a couple of timing issues that could contribute to this problem.
Proposed fix for 1: If the H5P.instance is not set up, wait for the 'initialized' event and then set up the H5PEmbedCommunicator. |
I expect that people following this issue are still interested in getting this resolved, so I thought I'd post an update. The latest released version of the Moodle H5P plugin (release 1.22.4, version 2022012000) still does not solve the primary resizing bug (see point 1 in previous comment). I hope this helps! |
Thank you for implementing the patch. This sounds promising and I really look forward to giving it a try until H5P team will be able to fix it. Is there a chance you could raise a pull request so it will clearly show the diff between canonical repo and your patched branch? Kind regards, |
Hi, |
Thanks @golenkovm for creating the pull request and @danowar2k for testing. Our site has been running with the patch since last fall and we haven't experienced any resizing issues since. As for getting this approved and into the hvp code base, good luck! |
Meanwhile, here's our local workaround (done in our theme): amd/src/h5pEmbeddedResizeFix.js
classes/output/core_renderer.php
|
Well, it seems that there are two problems here, an easily solvable one and a harder problem. My code above solves the easy problem (setting the width to 100% of embedded h5p iframes). The harder problem is "the necessary height of the h5p iframe is known at a point in time which can be anytime before or after something else happened or not." Trying too early to resize the embed iframe may result in the wrong height read from the h5p iframe (which may even be 1 sometimes). And just waiting for the h5p iframe document to be ready may be too late. Or the h5p iframe document could declare itself ready before the code that does $(h5pIframeDoc).ready() is even run through, so that the ready listener listens for a thing that has already happened. Another variant of the above code did this and failed setting the height, too. Meanwhile, we just now tested the catalyst pull request (catalyst#23) after understanding what it does (just delaying the resize messages until H5P says the h5p iframe document is initialized correctly). Right now it looks like those changes to embed.js do the trick. Funny because h5p-resizer.js isn't even included on the pages. tl;dr The catalyst PR works, please merge. :-) |
We're still using the resize fix above, please merge that fix :-) |
When will this fix be released? |
The above fix in combination with a fix for using embed.php ( Lines 75 to 78 in b1105da
embed.php needs to use the option forceembedtype and set it to div, because otherwise there are multiple levels of iframe + embed.php + h5p-resizer.js adding and that screws with dynamic resizing. view_assets has a bug where the $options variable doesn't use the forceembedtype array index but instead stdClass notation ($option->forceembedtype). |
Here's the PR for the $options bug: #545 |
Here's the PR for forcing the embedtype in embed.php to be a div: #546 |
Huh, I've just randomly decide to remove the embed.js fix for this (catalyst#23). It seems (!) that my two pull requests fix this bug on their own? Maybe someone could confirm that? |
Hi,
Embedded H5P content auto-resizing seems to work very randomly with MS browsers. Is this something that could be fixed in H5P Moodle plugin or PHP library?
Here is a couple of screenshots from my tests using Edge. When opening a Moodle Page activity with embedded H5P Interactive video in a small browser window, at first it doesn't resize the content:
But after refreshing the page a few times it does resize eventually:
The text was updated successfully, but these errors were encountered: