Problem/Motivation

We have been using the Iframe module for years to embed lazy-loading <iframe> static HTML 'Explainers' on our educational website to great effect.

Our site students can view our Explainers embedded within the tutorial pages like this and opt to snap the …

iframe_module_in_action

The underlaying markup for the <iframe> rendered as…

<iframe class=" lazyloaded" width="100%" height="701" name="iframe-field_iframe-870" id="iframe-field_iframe-870"
  title=""
  allow="accelerometer;autoplay;camera;encrypted-media;geolocation;gyroscope;microphone;payment;picture-in-picture"
  allowfullscreen="allowfullscreen" loading="lazy"
  data-src="https://sirclickalot.co.uk/assets/books/gcse-computer-science-plus/chapters/data-representation/topics/binary-numbers/explainers/what-is-binary">
  Your browser does not support iframes, but you can visit <a
    href="https://sirclickalot.co.uk/assets/books/gcse-computer-science-plus/chapters/data-representation/topics/binary-numbers/explainers/what-is-binary"></a>
</iframe>

It all worked beautifully ;-?)

However, since updating to 3.01 with it subsequent change the way that the 'Allow full screen' option is handled, the underlaying markup is now…

<iframe class=" lazyloaded" width="100%" height="701" name="iframe-field_iframe-870" id="iframe-field_iframe-870"
  title=""
  allow="accelerometer;autoplay;camera;encrypted-media;geolocation;gyroscope;microphone;payment;picture-in-picture;fullscreen"
  loading="lazy"
  data-src="https://sirclickalot.co.uk/assets/books/gcse-computer-science-plus/chapters/data-representation/topics/binary-numbers/explainers/what-is-binary">
  Your browser does not support iframes, but you can visit <a
    href="https://sirclickalot.co.uk/assets/books/gcse-computer-science-plus/chapters/data-representation/topics/binary-numbers/explainers/what-is-binary"></a>
</iframe>

…and our JS-powered snap to full screen links no longer work reporting (JS Console)…

iframe_module_console_report

We immediately went back to the field settings to check that Boolean value for the 'allow full screen' toggle but it was TRUE as expected…

/iframe_module_allow_full_screen_field_setting

We have left 3.01 deployed here as an publicly-accessible example of the breakage...

https://bit-by-bit.org/node/251

Downgrading back to 3.00 and we all good again.

CommentFileSizeAuthor
iframe_module_in_action.png68.73 KBsirclickalot
iframe_module_console_report.png14.56 KBsirclickalot
iframe_module_allow_full_screen_field_setting.png10.8 KBsirclickalot

Issue fork iframe-3603949

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

sirclickalot created an issue. See original summary.

smustgrave’s picture

smustgrave commented
Category: Support request » Bug report
Priority: Normal » Major

Seems like a regression.

sirclickalot’s picture

sirclickalot
Primary language English
Location Somerset
commented

While I agree that the "allowfullscreen" attribute is considered a legacy attribute and redefined as allow="fullscreen"
It does appear that actually removing the allowfullscreen="allowfullscreen" might be jumping the gun a little.

Perhaps the nest middle ground is to add to the allow as below…

allow="accelerometer;autoplay;camera;encrypted-media;geolocation;gyroscope;microphone;payment;picture-in-picture;fullscreen"

But also keep: allowfullscreen="allowfullscreen" for now?

I have downgraded the live example now (to module v3.00) at: https://bit-by-bit.org/node/251

smustgrave opened merge request !30

smustgrave’s picture

smustgrave commented
Status: Active » Needs review
sirclickalot’s picture

sirclickalot
Primary language English
Location Somerset
commented

Thanks @smustgrave.

I have applied the patch (30.patch) from the MR and it seems to have fixed the issue of allowing the full screen links to work again.

You can see it in action with multiple <iframe> elements on a page here: https://bit-by-bit.org/node/6897

With the 3.00 version of the module, I had overridden the iframe.html.twig template to allow for lazy loading (we use the Lazy module (https://www.drupal.org/project/lazy) for this.

The only edits are to the <iframe> element of template and they looked like this...

  <iframe class="lazyload" {{ attributes }} loading="lazy">
    {{ 'Your browser does not support iframes, but you can visit <a href=":url">@text</a>'|t({ ':url': src, '@text': text }) }}
  </iframe>

HOWEVER, as demonstrated on https://bit-by-bit.org/node/6897 the <iframe> no longer lazy loads and the whole page has to load in full.

On inspecting the resultant markup, the 'laziness' is all there but I notice that despite there being no space in <iframe> class attribute in my custom template, that one is inserted when the <iframe> is rendered...

<iframe class=" lazyloaded" width="100%" height="701" name="iframe-field_iframe-10663" id="iframe-field_iframe-10663"
  title=""
  allow="accelerometer;autoplay;camera;encrypted-media;geolocation;gyroscope;microphone;payment;picture-in-picture;fullscreen"
  src="https://sirclickalot.co.uk/assets/books/gcse-computer-science-plus/chapters/data-representation/topics/binary-addition/explainers/adding-binary-numbers"
  loading="lazy">
  Your browser does not support iframes, but you can visit <a
    href="https://sirclickalot.co.uk/assets/books/gcse-computer-science-plus/chapters/data-representation/topics/binary-addition/explainers/adding-binary-numbers"></a>
</iframe>

I am not sure if that is the likely cause?

As you might imagine, with pages such is this containing multiple <iframe> elements, the lazy-loading was an important asset so it would be nice to have it functioning again.

Thanks

angel_devoeted’s picture

angel_devoeted commented

Tested MR !30 via DrupalPod. It fixes the fullscreen regression introduced in 3.0.1.

Regarding #6, this MR does not cause the lazy loading issue or the malformed class attribute.
That regression seems to come from the 3.0.1 release itself due to how the src attribute is handled in the attributes object, which prevents the Lazy module from altering it.

@sirclickalot, maybe it would be a good idea to open a separate issue for lazy loading?

Since the lazy loading issue is unrelated, the MR looks good.

sirclickalot’s picture

sirclickalot
Primary language English
Location Somerset
commented

Thank you @angel_devoeted, that's a very useful insight!

I have detailed it here: https://www.drupal.org/project/iframe/issues/3607132

smustgrave’s picture

smustgrave commented

So this fixes the issue?

angel_devoeted’s picture

angel_devoeted commented

Yes, this MR fixes the fullscreen regression.

smustgrave’s picture

smustgrave commented
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.