otacke / h5p-sharing

Make sharing H5P content even easier
MIT License
3 stars 1 forks source link

Error in Height Attribute in Embed Code #1

Closed mrjarbenne closed 3 years ago

mrjarbenne commented 3 years ago

Thanks for sharing this plugin, which should prove quite useful with the new oEmbed functionality coming to OneNote. Although I'm using the plugin to enable easier access to the direct link, I did notice that in the embed code rendered in the H5P Sharing field, the height is set to 1 despite the embed code rendered from the footer of the interactive having a different/larger amount. The image shows the code pulled from the embed modal from the interactive in the yellow text box.

2021-01-19_14-44-00
otacke commented 3 years ago

@mrjarbenne Interesting, thanks for pointing that out. I fetch the complete embed link from H5P and don't build it myself, but maybe too early and it gets updated later on. I'll check. Luckily it shouldn't be serious, because H5P will rescale the iframe anyway unless the resize script cannot run.

otacke commented 3 years ago

@mrjarbenne I couldn't reproduce the issue, but I assume that in your case the H5P iframe had not been initialized yet, so it had a height of 1 pixel when setting the size in the snippet. If that assumption is correct, the latest version should work properly. Would be cool if you verified that on your platform.

mrjarbenne commented 3 years ago

Thanks @otacke for the quick response. I uploaded the latest changes and I'm still seeing the issue. You are correct that the resizer.js script fixes it so it's not a big deal. I didn't adjust the defaults for this activity type. Note the width attribute is also different than what is in the embed modal.

2021-01-20_11-09-52

otacke commented 3 years ago

@mrjarbenne A height of 1 should only be retrieved if H5P's iframe is present, but not initialized yet. That's also why the width differs. The new code explicitly waits until the iframe got initialized, and I was under the impression that it would definitely have been resized by then. Seems I was wrong. Bummer I cannot reproduce it myself, sorry. I'll try to locate the cause of the issue in H5P core.

otacke commented 3 years ago

@mrjarbenne Alright, next one. If this doesn't work, then there's something weird going on - extremely slow server or extremely large H5P content that takes quite a while for processing until ready, some lazy loading that messes with when what is loaded, etc.

mrjarbenne commented 3 years ago

I think we are sorted now. I can't recreate the issue consistently anymore. Thanks for tinkering away with me here.

otacke commented 3 years ago

@mrjarbenne You're saying you cannot reproduce it consistently anymore. So you can still reproduce it occasionally?

mrjarbenne commented 3 years ago

I can, but not on my test server which is smaller, and seems to be working perfectly now with your plugin. In Production we have a Multisite with over 13 000 sites, with H5P network activated, so I'm chalking it up to lag in that space. I'm playing around with some other workarounds now to see if it's a cache issue on my end. I don't want to take any more of your time up now that it's working for me in a smaller setting.

otacke commented 3 years ago

@mrjarbenne Hmm, I'd still like to find the cause end fix it. Does the height really still say "1"? That should not be possible anymore (https://github.com/otacke/h5p-sharing/blob/master/js/inject-sharing-info.js#L199), and if it is, this might be a caching issue still loading the old version of the script. The height should be > 1 or :h if the script timed out after 30 seconds.

mrjarbenne commented 3 years ago

Very strange. Still getting 1 just on one site. Took the content, exported and imported into another subdomain on the network and it works. Cleared the cache on the server. Reistalled the plugin. Deactivated/Reactivated both H5P and Sharing. Before I was consistently getting 1, now it's just on this one subsite, with this one interactive.

2021-01-22_14-42-11 (1)

otacke commented 3 years ago

@mrjarbenne Hmmmmmm. In theory, if the timing is right, the iframe could have been resized > 1 while the content inside has not and is still 1. I just changed the code a little.

mrjarbenne commented 3 years ago

That did it! Amazing!!!

otacke commented 3 years ago

@mrjarbenne Phew! I'll check if there's a better way to do this when I find some time. Not sure if there is. You could detect when H5P content starts resizing, but you wouldn't know how long that will take/when it's done, and that's when you would want to grab the size. Well, this works then, but it's kind of ugly :-D

Thanks a lot for finding the issue for helping me to debug!

mrjarbenne commented 3 years ago

No, THANK YOU for taking the time out of your day to fix it, and for all your contributions to the H5P community.