-
Notifications
You must be signed in to change notification settings - Fork 99
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.
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
Leverage Optimization Detective to optimize embeds in Embed Optimizer #1302
Conversation
1eaffd7
to
b6a78d1
Compare
a90d933
to
a95c4b2
Compare
…utes" This reverts commit 8e27c27. The reason is that PhpStorm is reporting a problem: > Duplicate type 'PreloadLinkAttributes' It's not a problem with PHPStan, however.
…nd add tests for link collection
Co-authored-by: Pascal Birchler <pascalb@google.com>
Co-authored-by: Pascal Birchler <pascalb@google.com>
Co-authored-by: Pascal Birchler <pascalb@google.com>
Co-authored-by: Pascal Birchler <pascalb@google.com>
Co-authored-by: Pascal Birchler <pascalb@google.com>
… add/od-embed-optimizer * 'trunk' of https://github.com/WordPress/performance: Facilitate embedding of Embed Optimizer Lowercase context Improve testcase structure Update WP version in CONTRIBUTING.md Test less verbose Remove redundant tests Improve testcase Add test for alt attribute and improve test for source Remove unuse code Update testcase Add srcset back to the original image Revert "Add alt tag to original_image_without_srcset" Add alt tag to original_image_without_srcset Improve I18N Issue Based on Modern Image Formats 2.0.1
$generator = $walker->open_tags(); | ||
while ( $generator->valid() ) { | ||
$visitors = iterator_to_array( $tag_visitor_registry ); | ||
while ( $processor->next_open_tag() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I had this as $processor->next_tag()
which was not correct as it was resulting in visits to closing tags. Then I added a $processor->is_tag_closer()
check which would continue
if true
. But then I remembered this subclass's next_open_tag()
helper method which "works a treat".
plugins/embed-optimizer/hooks.php
Outdated
// over all tags in the document, and this embed_optimizer_update_markup() is usd as part of the tag visitor | ||
// from Embed Optimizer. On the other hand, if $html_processor is not an OD_HTML_Tag_Processor then this is | ||
// iterating over the tags of the embed markup alone as is passed into the embed_oembed_html filter. | ||
if ( $html_processor instanceof OD_HTML_Tag_Processor ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feels a bit hacky to determine the context this way. could you add an additional parameter to embed_optimizer_update_markup
instead (and then use a wrapper for the filter callback). not a blocker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fair enough. Changed in 357364e.
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
plugins/embed-optimizer/readme.txt
Outdated
@@ -13,6 +13,10 @@ Optimizes the performance of embeds by lazy-loading iframes and scripts. | |||
|
|||
This plugin's purpose is to optimize the performance of [embeds in WordPress](https://wordpress.org/documentation/article/embeds/), such as YouTube videos, TikToks, and so on. Initially this is achieved by lazy-loading them only when they come into view. This improves performance because embeds are generally very resource-intensive and so lazy-loading them ensures that they don't compete with resources when the page is loading. [Other optimizations](https://github.com/WordPress/performance/issues?q=is%3Aissue+is%3Aopen+label%3A%22%5BPlugin%5D+Embed+Optimizer%22) are planned for the future. | |||
|
|||
This plugin also recommends that you install and activate the [Optimization Detective](https://wordpress.org/plugins/optimization-detective/) plugin. When it is active, it will start learning about which embeds appear in the initial viewport based on actual visitors to your site. With this information in hand, Embed Optimizer will then avoid lazy-loading embeds which appear in the initial viewport (above the fold). This is important because lazy-loading adds a delay which can hurt the user experience and even degrade the Largest Contentful Paint (LCP) score for the page. In addition to not lazy-loading such above-the-fold embeds, Embed Optimizer will add preconnect links for the hosts of network resources known to be required for embeds (e.g. YouTube, Twitter, and WordPress TV); this can further speed up the loading of critical embeds. Again, these performance enhancements are only enabled when Optimization Detective is active. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice you don't mention breakpoints, are you planning to add that in a future PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you're referring to https://github.com/WordPress/performance/pull/1302/files#r1670865833 ?
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
… document Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, left some small feedback/questions
Summary
Relevant technical choices
OD_HTML_Tag_Walker
class has been eliminated. This was a wrapper around theWP_HTML_Tag_Processor
subclass to ensure that any of the visitors would not callnext_tag
orseek
and thus cause the processor to lose track of where it was in the document which would cause problems with generating XPaths for the tags. For Image Optimizer, iterating over the document tag-by-tag in a forward direction was sufficient. For Embed Optimizer, however, it was too limiting. Embed Optimizer needs to be able to iterate over the tags in an embed, and then based on what it discovers, go back and apply the lazy-loading optimizations. Extenders who write tag visitors will have the fullWP_HTML_Tag_Processor
API available to them. Many of the changes you'll see are just swapping outwalker
forprocessor
.OD_HTML_Tag_Processor::append_head_html()
andOD_HTML_Tag_Processor::append_body_html()
no longer require that the closing</head>
and</body>
tag to have first been visited to be successful. The supplied HTML is buffered until the time thatget_updated_html()
is called.OD_Preload_Link_Collection
class has been made more generic as anOD_Link_Collection
class, allowing tag visitors to register links of any relation type. For Embed Optimizer, this is leveraged to addpreconnect
links. Support for additional link attributes has also been added.plugins/$slug/tests/bootstrap.php
. This is needed for Embed Optimizer to load Optimization Detective to test its integration.