Skip to content

Enable lazy-loading of post embeds #6474

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.

Sign up for GitHub

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

Jump to bottom
Closed
westonruter wants to merge 9 commits into from
Closed

Enable lazy-loading of post embeds #6474

westonruter wants to merge 9 commits into from

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented May 2, 2024 •
edited
Loading

  • Use visibility:hidden instead of clip: rect(1px, 1px, 1px, 1px); to hide the iframe while it is loading. When the iframe is lazy-loaded, Chromium does not reliably load the iframe when it is clipped but it does load it when it is made transparent. This also fixes an accessibility problem for keyboard navigation, since links in the clipped iframe could still receive focus even though they could not be seen by sighted users, resulting in confusingly hidden tab stops. Setting the iframe to be visibility:hidden ensures that the iframe is loaded consistently across browsers and that the contents of the iframe are not interactable until it is made visible.
  • Update wp_iframe_tag_add_loading_attr() to no longer short-circuit when the iframe is a post embed.
  • Use the HTML API to remove the style attribute from post embeds in feeds, instead of doing string replacement.

Trac ticket: https://core.trac.wordpress.org/ticket/58773

Drafted Commit Message

Embeds: Enable lazy-loading of post embeds and fix keyboard a11y for hidden iframes.

Chrome unreliably loads a lazy-loaded iframe when it is hidden using clip: rect(1px, 1px, 1px, 1px). Instead of using clip, a lazy-loaded iframe can also be hidden with visibility:hidden which results in it loading not only in Chrome but all other browsers. With this change applied, the hard-coded check to prevent lazy-loading post embeds is now removed. An added benefit to using visibility:hidden is that the entire iframe in this case is not interactable, meaning that users navigating the document with the keyboard will not unexpectedly encounter tab stops inside of the hidden iframe, as can happen now with clip when the JS fails to reveal the loaded iframe. Note also that the clip property is deprecated.

Lastly, when such a post embed iframe is rendered in an RSS feed, the style attribute is now removed using the HTML Tag Processor as opposed to using string replacement.

Fixes #58773.
Props westonruter, joemcgill, swissspidy, joedolson, adamsilverstein.

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions
Copy link

github-actions bot commented May 2, 2024 •
edited
Loading

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props westonruter, swissspidy, joemcgill.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter westonruter mentioned this pull request May 2, 2024
Merged
@github-actions
Copy link

github-actions bot commented May 2, 2024

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@westonruter westonruter requested a review from swissspidy May 6, 2024 18:43
joemcgill
joemcgill approved these changes May 6, 2024
View reviewed changes
Copy link
Member

@joemcgill joemcgill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see a possible a11y improvement here. Thanks for following up, @westonruter. Once comment about a docblock, but pre-approving the changes.

@westonruter
Merge branch 'trunk' into trac-58773
d6bcdf5 
* trunk: (62 commits)
  Twenty Twenty-One: Fixes primary menu disappearing.
  Tests: Add comment to explain when e2e test for gutenberg is skipped
  Docs: Make Settings API documentation link clickable in `wp-admin/options.php`.
  Bootstrap/Load: Place the debugging output for automatic plugin and theme updates behind a debugging flag.
  Bootstrap/Load: Update the domain parsing when initialising the cookie domain on Multisite.
  Users: Add JS confirmation if user profile has unsaved changes.
  Tests: Move `add_settings_section()` tests to a more appropriate place.
  Twenty Seventeen: Resolves Header Image Quality Issue
  Options, Meta APIs: Prime transient options prior to use.
  Toolbar: Accessibility: Remove adminbar skiplink focus fix.
  Docs: Update a HelpHub link on Edit Site screen to avoid unnecessary redirection.
  Docs: Use more specific URLs for a few Network Admin HelpHub links.
  REST API: Ensure attachments are uploaded to the post's year/month folder.
  Posts, Post Types: Use a consistent plural form of “status” in variable names.
  Upgrade/Install: Automatically roll back to the previous version when an automatic plugin update results in a fatal error on the front end of the site.
  Interactivity API: Cannot be used from wp-admin
  Script Modules: Hooks are not registered in wp-admin
  Bootstrap/Load: Take the port number into consideration when determining whether a subdomain installation of Multisite is allowed.
  Coding Standards: Use strict comparison in `wp-includes/nav-menu-template.php`.
  Editor: move global CSS custom properties to `:root` selector.
  ...
@westonruter
Merge branch 'trac-58773' of https://github.com/westonruter/wordpress…
53074a6 
…-develop into trac-58773

* 'trac-58773' of https://github.com/westonruter/wordpress-develop:
  Remove todo
@westonruter
Copy link
Member Author

westonruter commented May 13, 2024

Committed to trunk in r58143 (b4889e4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joemcgill joemcgill joemcgill approved these changes

@swissspidy swissspidy swissspidy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@westonruter @joemcgill @swissspidy