inline iframes require title attributes - a11y

ampproject / amphtml

The AMP web component framework.

https://amp.dev

Apache License 2.0

14.89k stars 3.89k forks source link

inline iframes require title attributes - a11y #29874

Closed jwinkworth closed 2 years ago

jwinkworth commented 4 years ago

What's the issue?

AMP components that create inline iframes do not have a title attribute. This is against WCAG guidelines and fails Lighthouse audits.

To be clear, this is not an issue with the <amp-iframe> component, but with other components that under the hood create an inline iframe.

Similar bugs appears to have been logged multiple times in the past here, with some reports closed/merged claiming a fix.

12617 - appears to be the PR for this very issue, but I am unclear of it's status (last updated 2 years ago)
17308 - this issue is specific to amp-iframe, but the issue is larger than just that component

How do we reproduce the issue?

Visit http://content-solutions.s3.ca-central-1.amazonaws.com/amp-inline-iframe-bug/index.html
Inspect either inline iframe (youtube/brightcove). Both amp tags have a title attribute defined

Expected result

The inline iframe created by AMP should have the title attribute attached.

Actual result

The inline iframe created by AMP does not have a title attribute attached.

What browsers are affected?

All browsers, all OSes

Which AMP version is affected?

2007302351001

caroqliu commented 3 years ago

@realPrimoh Will be working on this - for some reason you don't come up in the assignees UI. 😅

westonruter commented 3 years ago

Just to note that amp-iframe does propagate the title attribute to the iframe (as of #27037), as this attribute is among the ATTRIBUTES_TO_PROPAGATE:

https://github.com/ampproject/amphtml/blob/7785e940fbac13bb0314d78a2a270d6219c84b66/extensions/amp-iframe/0.1/amp-iframe.js#L51-L62

But amp-youtube (and others probably) does not. I made this example to show the difference between amp-youtube and amp-iframe, where the former does not copy the title attribute but the latter does: https://amp-iframe-title-attribute-iframe-copying.glitch.me/

For the amp-youtube, there is a Lighthouse accessibility audit failure:

realPrimoh commented 3 years ago

@westonruter Thanks Weston, this is good to know. It seems to be mostly an internal iframe issue where the iframe is created within the AMP element to display external sites, for things like amp-youtube, amp-instagram, amp-facebook, etc.

westonruter commented 3 years ago

Yes, I think the main issue is that the same ATTRIBUTES_TO_PROPAGATE logic needs to be adapted from amp-iframe into the other AMP components that also create iframe.

realPrimoh commented 3 years ago

This has been fixed and merged as of this week: https://github.com/ampproject/amphtml/pull/30834.

jwinkworth commented 3 years ago

I've noticed that while this general iframe fix has gone live, the Brightcove video was missed in the MR that fixed this.

Brightcove https://github.com/ampproject/amphtml/blob/6cba3aa84378c86d1f8ed325231ff68ce22b1297/extensions/amp-brightcove/0.1/amp-brightcove.js#L127-L130

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.