#58773 closed defect (bug) (fixed)
Improve oEmbed lazy loading
Reported by: | adamsilverstein | Owned by: | adamsilverstein |
---|---|---|---|
Milestone: | 6.6 | Priority: | normal |
Severity: | normal | Version: | 4.5 |
Component: | Embeds | Keywords: | has-patch has-unit-tests |
Focuses: | accessibility, performance, sustainability | Cc: |
Description (last modified by )
Why lazy load oEmbed iframes?
- Lazy loading prevents wasted resources from loading if the user never scrolls down to view the oEmbed
- Iframe lazy loading is handled directly by the browser, making the change simple (add a single attriute) and ensuring users get a great experience
Lazy loaded oEmbeds history
WordPress has lazy loaded iframes since version 5.7 and the original proposal included lazy loading for oEmbed iframes (as long as the provider also included width and height attributes).
Ultimately, dynamic addition of lazy loading of oEmbeds was removed from core because it was found to create issues, especially with embeds of other WordPress posts.
Proposed Solution
This ticket proposes slightly different approach that relies on altering the iframe instance on the fly for each oEmbed. In particular:
- When the oEmbed iframe HTML is displayed, if it doesn't already contain a
loading
attribute, it is run throughwp_iframe_tag_add_loading_attr
to add the lazy loading iframe under the correct conditions- Will not add lazy loading to WordPress iframes because of the
data-secret
check - Only adds lazy loading to iframes with the height and width present.
- Runs before the
oembed_result
filter, so developers can remove the attribute if desired.
- Will not add lazy loading to WordPress iframes because of the
Attachments (6)
Change History (37)
#1
@
17 months ago
- Component changed from General to Embeds
- Focuses performance sustainability added
- Keywords needs-patch needs-unit-tests needs-testing added
- Owner set to adamsilverstein
- Status changed from new to assigned
This ticket was mentioned in PR #4822 on WordPress/wordpress-develop by @adamsilverstein.
17 months ago
#3
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
#4
@
17 months ago
Note in before-change.jpg a default YouTube embed does not contain a loading attribute (on trunk).
With the patch applied in lazy.jpg we see the loading="lazy" attribute be ing applied.
For a test post with a WordPress post embed, note that the data-secret
attribute prevents the addition of the loading="lazy" attribute - see wpembed.jpg.
#5
@
17 months ago
Steps to reproduce
- Create a post with several pages worth of content, then paste a youtube (or other embeddable) link to create the embed block
- Before applying the patch, view the post
- note iframe content loads immediately on page load, including all resources
- note YT iframe lacks loading attribute
- Apply the patch
- Clear the post cache or create a new poat with the same content (
wp embed cache clear POST_ID
) - Check the post on the front end again.
- checking the network panel, note that the YT embed isn't loaded until you scroll down on the page.
- view the page source and note that
loading="lazy"
has been added to the YT iframe
Help needed
- It would be good to manually go through all of the WordPress supported oEmbed types to make sure they all work correctly with this change applied
- It would be nice to have some benchmarks here, although we'll see the biggest change for actual users in the field.
#6
@
17 months ago
What was the issue with the WordPress oEmbed iframes? Can we make it work for them?
#7
@
17 months ago
What was the issue with the WordPress oEmbed iframes? Can we make it work for them?
Possibly, I believe they were removed explicitly because of issues in the way they are rendered - there is JavaScript that communicates from inside the iframe to the containing page at load time that didn't work when we added lazy loading. Worth revisiting for sure, maybe we can fix that part.
#10
@
17 months ago
I created a test site with a bunch of different oEmbeds (although not every type WP supports): https://oembeds-test.instawp.xyz/
In my testing some oEmbed endpoints like YouTube return an iframe and the approach in this ticket properly adds the loading="lazy" attribute. Other oEmbed endpoints instead return a script
tag that generates the oEmbed iframe HTML at runtime when executed.
For example, the twitter oEmbed endpoint returns something like this:
<blockquote class="twitter-tweet" data-width="500" data-dnt="true"> <a href="https://twitter.com/example/status/1634286225552035873?ref_src=twsrc%5Etfw">March 10, 2023</a></blockquote> <script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>"
When the widget.js
script executes, it expands the tweet inline into a full visually rich iframe. Since this happens after load time we can't alter the iframe code. We might consider swapping defer
for async
or adding fetchpriority="low
to these scripts as a way of de-prioritizing them after researching potential benefits or pitfalls.
#11
@
17 months ago
Needs testing -
- oEmbeds in block themes vs classic themes
- Are oembed iframes passed to the content - if so, why doesn't the lazy loading work there? if not, why?
#12
follow-up:
↓ 13
@
17 months ago
After discussing this further with @flixos90 I re-tested and it turns out the regular content filters do work correctly for oEmbed iframes. The issue with my tests is that I was only testing with a single or two oEmbeds and wp_omit_loading_attr_threshold
is now set at 3. Oof!
Querying httparchive (query below), I found that for WordPress sites that use an oEmbed block, the average number of oEmbeds is below 3:
origins with embed block | average_embed_block_count |
48445 | 2.8925172876457887 |
I wonder if we should consider lowering the threshold for iframes back to 1? (or 0 if iframe in viewport initial loads are rare?) Maybe we can query httparchive to see how common the LCP element being an iframe is?
Here is the query I used to check for the oEmbed count:
WITH WPEmbeds AS ( SELECT url, JSON_EXTRACT(payload, '$._cms.wordpress.has_embed_block') AS has_embed_block, CAST( JSON_EXTRACT(payload, '$._cms.wordpress.embed_block_count.total') AS FLOAT64 ) AS embed_block_count_total FROM `httparchive.pages.2023_05_01_desktop` ), CountedEmbeds AS ( SELECT COUNT( DISTINCT url ) AS origins, AVG( embed_block_count_total ) as average_embed_block_count FROM WPEmbeds WHERE embed_block_count_total != 0 ) SELECT * FROM CountedEmbeds
#13
in reply to:
↑ 12
@
17 months ago
Replying to adamsilverstein:
After discussing this further with @flixos90 I re-tested and it turns out the regular content filters do work correctly for oEmbed iframes. The issue with my tests is that I was only testing with a single or two oEmbeds and
wp_omit_loading_attr_threshold
is now set at 3. Oof!
Querying httparchive (query below), I found that for WordPress sites that use an oEmbed block, the average number of oEmbeds is below 3
One consideration here (if I'm not mistaken) is that the wp_omit_loading_attr_threshold()
is cumulative — meaning it will be the sum of all images AND iframes found in the page, not just the number of embeds. This means that WordPress will begin to apply lazy loading to the fourth object on the page regardless. Perhaps we should treat the threshold for iframes differently from images, but it would good to run some numbers on the number of embeds that end up being counted as a part of the LCP value to see if it would make a significant difference.
#14
@
17 months ago
Perhaps we should treat the threshold for iframes differently from images, but it would good to run some numbers on the number of embeds that end up being counted as a part of the LCP value to see if it would make a significant difference.
Intuitively I feel this is likely the case and I agree it would be helpful to some solid numbers to help inform an improved approach.
@adamsilverstein commented on PR #4822:
17 months ago
#15
I am questioning whether that is an appropriate fix to the problem. Embeds should receive their loading attributes the same way that any other items in the_content do, as part of wp_filter_content_tags(), relying on the new wp_get_loading_optimization_attributes().
Agreed, after further testing filter in the_content
is being applied; I commented on the trac ticket - I do think we should reconsider our approach to iframe lazy loading to see if we can improve things for most users, they likely need slightly different treatment than images do.
#16
@
17 months ago
- Summary changed from Lazy load oEmbed iframes to Improve oEmbed iframe lazy loading
#18
@
15 months ago
Is there any issue around tracking. What if you were using an iframe to do tracking? Or a impression count on a video? I am not sure it is a blocker, but this change needs to be called out in the dev notes.
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
15 months ago
#20
@
15 months ago
- Milestone changed from 6.4 to 6.5
This ticket was discussed in this week's performance bug scrub. As this hasn't got an update in 3 months, this feels unlikely to get into a state to commit.
@adamsilverstein if you can get a patch together then you move back to milestone.
#21
@
13 months ago
- Summary changed from Improve oEmbed iframe lazy loading to Improve oEmbed lazy loading
#22
@
13 months ago
I started working on this as an experimental modules for the Performance Lab plugin in https://github.com/WordPress/performance/pull/886
This module attempts to optimize oEmbeds using two techniques:
- For oEmbeds delivered as iframes, ensure the loading=lazy attribute is present
- For oEmbeds built using script tags:
- move the script src to a data attribute
- add a page level IntersectionObserver that sets script src before the user scrolls to the oEmbed (using a 500px margin)
The impact of the change would of course depend of which and how many oEmbeds a site used, however I think this is worth exploring a bit further
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
12 months ago
#24
@
12 months ago
- Milestone changed from 6.5 to Future Release
This ticket was discussed during the Performance bug scrub. Since this feature has been thoroughly tested in the Performance lab module, it has been decided to move it to the Future Release category.
Props @joemcgill @flixos90
This ticket was mentioned in PR #6474 on WordPress/wordpress-develop by @westonruter.
7 months ago
#25
- Keywords has-patch has-unit-tests added; needs-patch removed
- Use
opacity:0
andpointer-events:none
instead ofclip: 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. - Update
wp_iframe_tag_add_loading_attr()
to no longer short-circuit when the iframe is a post embed.
Trac ticket: https://core.trac.wordpress.org/ticket/58773
#26
@
7 months ago
- Milestone changed from Future Release to 6.6
I think I have a solution for fixing lazy-loading of post embed iframes. See https://github.com/WordPress/performance/pull/1192
I found that Chrome didn't reliably load the lazy-loaded post iframe when it had the clip: rect(1px, 1px, 1px, 1px)
style present. Strangely, the iframe would load when navigating to the page but it wouldn't load when reloading the page. However, if the clip
is replaced with opacity:0; pointer-events:none;
then it works as expected. I tested in Chrome, Safari, and Firefox. I've opened a PR to apply this change to core as well: https://github.com/WordPress/wordpress-develop/pull/6474
When testing this, remember to insert three images into the post content prior to the post embed. This I had to first insert 3 images into the post content followed by the post embeds. This is because wp_omit_loading_attr_threshold()
returns 3 so that lazy-loading is only applied to images and iframes after the 3rd.
#27
@
7 months ago
- Focuses accessibility added
Adding a11y focus since this switches away from clip
.
#28
@
7 months ago
Update: After chatting with @joedolson, I found out that visibility:hidden
works just as well if not better than opacity:0
. Not only does the lazy-loaded iframe load consistently across Chrome, Safari, and Firefox but there is also an actual accessibility benefit to using visibility:hidden
over clip
: the entire iframe is made non-interactive. With clip
a user tabbing through links on a page will also encounter tab stops inside the clipped iframe, which is particularly confusing to sighted users navigating by keyboard as focus seems to get lost on the page for several tab presses. In contrast, when an iframe is hidden with visibility
then no tab stops occur in the iframe. The PR has been updated to apply this change.
Side note: I also discovered that clip
is deprecated!
@westonruter commented on PR #6474:
7 months ago
#31
Committed to trunk in r58143 (b4889e4)
Trac ticket: https://core.trac.wordpress.org/ticket/58773