Problem/Motivation

There are w3c html validation errors for the following: 

Error: The frameborder attribute on the iframe element is obsolete. Use CSS instead.

Error: The allowtransparency attribute on the iframe element is obsolete. Use CSS instead.

These are added in the field formatter: OEmbedFormatter

Proposed resolution

Remove the invalid attributes from the HTML and add CSS to achieve the same result.

CommentFileSizeAuthor
#22 3071446-22-invalid-iframe-attributes.patch2.24 KBguptahemant
#21 3071446-21-invalid-iframe-attributes.patch2.22 KBguptahemant
#19 3071446-nr-bot.txt170 bytesneeds-review-queue-bot
#15 3071446-15-invalid-iframe-attributes.patch2.59 KBmarcvangend
#15 3071446-15-invalid-iframe-attributes-TEST-ONLY-FAIL.patch709 bytesmarcvangend
#14 3071446-13.patch2.55 KBlendude
#14 3071446-13-TEST_ONLY.patch668 byteslendude
#13 3071446-13-invalid-iframe-attributes.patch1.89 KBmarcvangend
#5 remove-iframe-attributes-3071446-4.patch1.14 KBkirst25
#2 remove-iframe-attributes-3071446-2.patch0 byteskirst25

Issue fork drupal-3071446

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

Kirst25 created an issue. See original summary.

kirst25’s picture

kirst25
she/her
commented
StatusFileSize
new remove-iframe-attributes-3071446-2.patch0 bytes
kirst25’s picture

kirst25
she/her
commented
kirst25’s picture

kirst25
she/her
commented
kirst25’s picture

kirst25
she/her
commented
StatusFileSize
new remove-iframe-attributes-3071446-4.patch1.14 KB
idebr’s picture

idebr commented
Status: Active » Needs review
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Version: 8.7.x-dev » 8.8.x-dev
Category: Bug report » Feature request
Issue tags: +Needs tests, +oembed, +Needs frontend framework manager review

This seems like a good idea, but will need front-end framework manager review (due to CSS changes) and test coverage.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented
Issue summary: View changes
StatusFileSize
new 3071446-13-invalid-iframe-attributes.patch1.89 KB

The oembed.formatter.css files in the 'stable' and 'stable9' themes contain copies of core/modules/media/css/oembed.formatter.css, so here is a patch that applies the same change to those copies.

lendude’s picture

lendude
Primary language Dutch
Location Amsterdam
commented
Issue tags: -Needs tests
StatusFileSize
new 3071446-13-TEST_ONLY.patch668 bytes
new 3071446-13.patch2.55 KB

Here is a test for the removed attributes.

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented
StatusFileSize
new 3071446-15-invalid-iframe-attributes-TEST-ONLY-FAIL.patch709 bytes
new 3071446-15-invalid-iframe-attributes.patch2.59 KB

Combining improvement from #13 with test from #14 and fixing code style errors.

The last submitted patch, 15: 3071446-15-invalid-iframe-attributes-TEST-ONLY-FAIL.patch, failed testing. View results

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3071446-nr-bot.txt170 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

guptahemant’s picture

guptahemant commented
StatusFileSize
new 3071446-21-invalid-iframe-attributes.patch2.22 KB

Attaching a patch for drupal 10, In drupal 10 stable theme is no longer present hence removed the changes related to that.

guptahemant’s picture

guptahemant commented
StatusFileSize
new 3071446-22-invalid-iframe-attributes.patch2.24 KB

Attaching a patch for drupal 10.1.x version

dcam made their first commit to this issue’s fork.

dcam opened merge request !7858

dcam’s picture

dcam commented
Status: Needs work » Needs review

I rerolled the patch in #22 for D11 and converted it to an MR.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left some comments on MR.

dcam’s picture

dcam commented

@smustgrave What are we adding an upgrade path for? Unless I'm missing something there are no configuration changes. The values for those attributes were hard-coded.

smustgrave’s picture

smustgrave commented

If they're not being added to configuration then can ignore that.

dcam’s picture

dcam commented
Status: Needs work » Needs review

The comments on the MR have been answered. I don't believe any changes are necessary.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue tags: -Needs frontend framework manager review

works for me, thanks.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Thanks for taking a look @nod_

Tested out by using a stark theme since Olivero disables media modules ombed library.
Tested with and without the patch and can confirm I'm not seeing any visual regression from removing the attributes.

LGTM

  • alexpott committed 04fb2d1d on 10.3.x 
    Issue #3071446 by marcvangend, Kirst25, dcam, guptahemant, Lendude, nod_...

  • alexpott committed 24b4a8ea on 10.4.x 
    Issue #3071446 by marcvangend, Kirst25, dcam, guptahemant, Lendude, nod_...

  • alexpott committed 0bc782b1 on 11.0.x 
    Issue #3071446 by marcvangend, Kirst25, dcam, guptahemant, Lendude, nod_...

  • alexpott committed 655cb760 on 11.x 
    Issue #3071446 by marcvangend, Kirst25, dcam, guptahemant, Lendude, nod_...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 655cb760fc to 11.x and 0bc782b1f8 to 11.0.x and 24b4a8ea8e to 10.4.x and 04fb2d1d1e to 10.3.x. Thanks!

diff --git a/core/modules/media/src/Plugin/Field/FieldFormatter/OEmbedFormatter.php b/core/modules/media/src/Plugin/Field/FieldFormatter/OEmbedFormatter.php
index 132248d8fc..70047c8983 100644
--- a/core/modules/media/src/Plugin/Field/FieldFormatter/OEmbedFormatter.php
+++ b/core/modules/media/src/Plugin/Field/FieldFormatter/OEmbedFormatter.php
@@ -22,7 +22,6 @@
 use Drupal\media\Plugin\media\Source\OEmbedInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
-// cspell:ignore allowtransparency
 /**
  * Plugin implementation of the 'oembed' formatter.
  *

Made the change above on commit because allowtransparency is no longer in the file.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.