Skip to content

Commit

Permalink
Add test case and improve setting meta data attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
@westonruter
westonruter committed Jun 1, 2024
1 parent 2d56df7 commit c0ed712
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 21 deletions.
13 changes: 2 additions & 11 deletions plugins/image-prioritizer/class-ip-img-tag-visitor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,13 @@ public function __invoke( OD_HTML_Tag_Walker $walker ): bool {
$common_lcp_element = $this->url_metrics_group_collection->get_common_lcp_element();
if ( ! is_null( $common_lcp_element ) && $xpath === $common_lcp_element['xpath'] ) {
if ( 'high' === $walker->get_attribute( 'fetchpriority' ) ) {
$walker->set_attribute( 'data-od-fetchpriority-already-added', true );
$walker->set_meta_attribute( 'fetchpriority-already-added', true );
} else {
$walker->set_attribute( 'fetchpriority', 'high' );
$walker->set_attribute( 'data-od-added-fetchpriority', true );
}

// Never include loading=lazy on the LCP image common across all breakpoints.
if ( 'lazy' === $walker->get_attribute( 'loading' ) ) {
$walker->set_attribute( 'data-od-removed-loading', $walker->get_attribute( 'loading' ) );
$walker->remove_attribute( 'loading' );
}
} elseif ( is_string( $walker->get_attribute( 'fetchpriority' ) ) && $this->url_metrics_group_collection->is_every_group_populated() ) {
Expand All @@ -68,7 +66,6 @@ public function __invoke( OD_HTML_Tag_Walker $walker ): bool {
* Note also that if this is the LCP element for _some_ of the viewport groups, it will still get
* fetchpriority=high by means of the preload link (with a media query) that is added further below.
*/
$walker->set_attribute( 'data-od-removed-fetchpriority', $walker->get_attribute( 'fetchpriority' ) );
$walker->remove_attribute( 'fetchpriority' );
}

Expand All @@ -77,21 +74,15 @@ public function __invoke( OD_HTML_Tag_Walker $walker ): bool {

// If the element was not found, we don't know if it was visible for not, so don't do anything.
if ( is_null( $element_max_intersection_ratio ) ) {
$walker->set_attribute( 'data-ip-unknown-tag', true ); // Mostly useful for debugging why an IMG isn't optimized.
$walker->set_meta_attribute( 'unknown-tag', true ); // Mostly useful for debugging why an IMG isn't optimized.
} else {
// Otherwise, make sure visible elements omit the loading attribute, and hidden elements include loading=lazy.
$is_visible = $element_max_intersection_ratio > 0.0;
$loading = (string) $walker->get_attribute( 'loading' );
if ( $is_visible && 'lazy' === $loading ) {
$walker->set_attribute( 'data-od-removed-loading', $loading ); // TODO: This should be automatically added when calling remove_attribute().
$walker->remove_attribute( 'loading' );
} elseif ( ! $is_visible && 'lazy' !== $loading ) {
$walker->set_attribute( 'loading', 'lazy' );
if ( '' !== $loading ) {
$walker->set_attribute( 'data-od-replaced-loading', $loading ); // TODO: This should be added automatically when forcing loading=lazy above.
} else {
$walker->set_attribute( 'data-od-added-loading', true ); // TODO: This should be automatically added when calling set_attribute().
}
}
}
// TODO: If an image is the LCP element for one breakpoint but not another, add loading=lazy. This won't hurt performance since the image is being preloaded.
Expand Down
8 changes: 4 additions & 4 deletions plugins/image-prioritizer/tests/test-helper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function data_provider_test_ip_filter_tag_walker_visitors(): array {
<title>...</title>
</head>
<body>
<img data-ip-unknown-tag data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]" src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" loading="lazy">
<img data-od-unknown-tag data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]" src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800" loading="lazy">
<script type="module">/* import detect ... */</script>
</body>
</html>
Expand Down Expand Up @@ -362,7 +362,7 @@ public function data_provider_test_ip_filter_tag_walker_visitors(): array {
</head>
<body>
<script>/* Something injected with wp_body_open */</script>
<img data-ip-unknown-tag src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800">
<img data-od-unknown-tag src="https://example.com/foo.jpg" alt="Foo" width="1200" height="800">
</body>
</html>
',
Expand Down Expand Up @@ -829,7 +829,7 @@ static function () {
<body>
<img src="https://example.com/mobile-logo.png" alt="Mobile Logo" width="600" height="600">
<p>New paragraph since URL Metrics were captured!</p>
<img data-ip-unknown-tag src="https://example.com/desktop-logo.png" alt="Desktop Logo" width="600" height="600">
<img data-od-unknown-tag src="https://example.com/desktop-logo.png" alt="Desktop Logo" width="600" height="600">
</body>
</html>
',
Expand All @@ -843,7 +843,7 @@ static function () {
<body>
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]" src="https://example.com/mobile-logo.png" alt="Mobile Logo" width="600" height="600">
<p>New paragraph since URL Metrics were captured!</p>
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::IMG]" data-ip-unknown-tag src="https://example.com/desktop-logo.png" alt="Desktop Logo" width="600" height="600">
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[3][self::IMG]" data-od-unknown-tag src="https://example.com/desktop-logo.png" alt="Desktop Logo" width="600" height="600">
<script type="module">/* import detect ... */</script>
</body>
</html>
Expand Down
31 changes: 29 additions & 2 deletions plugins/optimization-detective/class-od-html-tag-walker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,29 @@ public function get_attribute( string $name ) {
* @return bool Whether an attribute value was set.
*/
public function set_attribute( string $name, $value ): bool {
return $this->processor->set_attribute( $name, $value );
$existing_value = $this->processor->get_attribute( $name );
$result = $this->processor->set_attribute( $name, $value );
if ( $result ) {
if ( is_string( $existing_value ) ) {
$this->set_meta_attribute( "replaced-{$name}", $existing_value );
} else {
$this->set_meta_attribute( "added-{$name}", true );
}
}
return $result;
}

/**
* Sets a meta attribute.
*
* All meta attributes are prefixed with 'data-od-'.
*
* @param string $name Meta attribute name.
* @param string|true $value Value.
* @return bool Whether an attribute was set.
*/
public function set_meta_attribute( string $name, $value ): bool {
return $this->processor->set_attribute( "data-od-{$name}", $value );
}

/**
Expand All @@ -469,7 +491,12 @@ public function set_attribute( string $name, $value ): bool {
* @return bool Whether an attribute was removed.
*/
public function remove_attribute( string $name ): bool {
return $this->processor->remove_attribute( $name );
$old_value = $this->processor->get_attribute( $name );
$result = $this->processor->remove_attribute( $name );
if ( $result ) {
$this->set_meta_attribute( "removed-{$name}", is_string( $old_value ) ? $old_value : true );
}
return $result;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion plugins/optimization-detective/optimization.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ function od_optimize_template_output_buffer( string $buffer ): string {
}

if ( $did_visit && $needs_detection ) {
$walker->set_attribute( 'data-od-xpath', $walker->get_xpath() );
$walker->set_meta_attribute( 'xpath', $walker->get_xpath() );
}
$generator->next();
}
Expand Down
18 changes: 15 additions & 3 deletions plugins/optimization-detective/tests/test-class-od-html-tag-walker.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,8 @@ public function data_provider_sample_documents(): array {
* @param string $document Document.
* @param string[] $open_tags Open tags.
* @param string[] $xpaths XPaths.
*
* @throws Exception But not really.
*/
public function test_open_tags_and_get_xpath( string $document, array $open_tags, array $xpaths ): void {
$p = new OD_HTML_Tag_Walker( $document );
Expand Down Expand Up @@ -348,6 +350,8 @@ public function test_open_tags_throwing_exception(): void {
*
* @covers ::append_head_html
* @covers OD_HTML_Tag_Processor::append_html
*
* @throws Exception But not really.
*/
public function test_append_head_html(): void {
$html = '
Expand Down Expand Up @@ -396,6 +400,8 @@ public function test_append_head_html(): void {
* @covers ::append_head_html
* @covers ::append_body_html
* @covers OD_HTML_Tag_Processor::append_html
*
* @throws Exception But not really.
*/
public function test_append_head_and_body_html(): void {
$html = '
Expand Down Expand Up @@ -457,18 +463,24 @@ public function test_append_head_and_body_html(): void {
* @covers ::set_attribute
* @covers ::remove_attribute
* @covers ::get_updated_html
* @covers ::set_meta_attribute
*
* @throws Exception But not really.
*/
public function test_html_tag_processor_wrapper_methods(): void {
$processor = new OD_HTML_Tag_Walker( '<html lang="en" xml:lang="en"></html>' );
$processor = new OD_HTML_Tag_Walker( '<html lang="en" dir="ltr"></html>' );
foreach ( $processor->open_tags() as $open_tag ) {
if ( 'HTML' === $open_tag ) {
$this->assertSame( $open_tag, $processor->get_tag() );
$this->assertSame( 'en', $processor->get_attribute( 'lang' ) );
$processor->set_attribute( 'lang', 'es' );
$processor->remove_attribute( 'xml:lang' );
$processor->remove_attribute( 'dir' );
$processor->set_attribute( 'id', 'root' );
$processor->set_meta_attribute( 'foo', 'bar' );
$processor->set_meta_attribute( 'baz', true );
}
}
$this->assertSame( '<html lang="es" ></html>', $processor->get_updated_html() );
$this->assertSame( '<html data-od-added-id data-od-baz data-od-foo="bar" data-od-removed-dir="ltr" data-od-replaced-lang="en" id="root" lang="es" ></html>', $processor->get_updated_html() );
}

/**
Expand Down

0 comments on commit c0ed712

Please sign in to comment.