Skip to content

Omit element node index in XPaths up to children of BODY#1790

Merged
westonruter merged 6 commits intotrunkfrom
update/xpath-construction
Jan 13, 2025
Merged

Omit element node index in XPaths up to children of BODY#1790
westonruter merged 6 commits intotrunkfrom
update/xpath-construction

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented Jan 11, 2025 •
edited
Loading

Summary

This is the last PR to address #1787.

Instead of an XPath like:

/*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]/*[1][self::FIGURE]

This PR now results in the first three levels of XPaths for elements in the body to not have the node index specified:

/HTML/BODY/DIV/*[1][self::FIGURE]

This will allow for variation in the content printed at wp_body_open from impacting the XPaths for the rest of the document.

Note that nodes inside the HEAD will retain node indexes since there is no singular wrapper element in HEAD.

The changes located in tests directories are almost all attributable to a simple search-and-replace.

Fixes #1787.

@westonruter westonruter force-pushed the update/xpath-construction branch from d7d0ae8 to e840bdd Compare January 13, 2025 00:53
@westonruter westonruter changed the base branch from update/od-auth-conditions to add/od-root-div-to-tests January 13, 2025 00:53
@westonruter westonruter added [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Plugin] Embed Optimizer Issues for the Embed Optimizer plugin (formerly Auto Sizes) [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Type] Enhancement A suggestion for improvement of an existing feature labels Jan 13, 2025
westonruter
westonruter commented Jan 13, 2025
View reviewed changes
plugins/optimization-detective/class-od-html-tag-processor.php
Comment on lines +567 to +572
foreach ( $this->get_indexed_breadcrumbs() as $i => list( $tag_name, $index ) ) {
if ( $i < 2 || ( 2 === $i && '/HTML/BODY' === $this->current_xpath ) ) {
$this->current_xpath .= "/$tag_name";
} else {
$this->current_xpath .= sprintf( '/*[%d][self::%s]', $index + 1, $tag_name );
}
Copy link
Member Author

@westonruter westonruter Jan 13, 2025

Choose a reason for hiding this comment

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

All of the changes in this PR are a consequence of this change here.

@westonruter westonruter marked this pull request as ready for review January 13, 2025 01:00
@github-actions
Copy link

github-actions bot commented Jan 13, 2025 •
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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: swissspidy <[email protected]>

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

@codecov
Copy link

codecov bot commented Jan 13, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 33.33333% with 8 lines in your changes missing coverage. Please review.

Project coverage is 57.38%. Comparing base (2df83d0) to head (aa07c71).
Report is 7 commits behind head on trunk.

Files with missing lines Patch % Lines
...lugins/optimization-detective/class-od-element.php 0.00% 8 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##            trunk    #1790      +/-   ##
==========================================
- Coverage   57.43%   57.38%   -0.05%     
==========================================
  Files          84       84              
  Lines        6508     6517       +9     
==========================================
+ Hits         3738     3740       +2     
- Misses       2770     2777       +7
Flag Coverage Δ
multisite 57.38% <33.33%> (-0.05%) ⬇️
single 34.50% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@westonruter westonruter mentioned this pull request Jan 13, 2025
Closed
Base automatically changed from add/od-root-div-to-tests to trunk January 13, 2025 14:35
felixarntz
felixarntz reviewed Jan 13, 2025
View reviewed changes
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@westonruter Overall looks good to me, just one thought.

felixarntz
felixarntz approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@westonruter Approving, pending your code suggestion to expand the comment.

@westonruter westonruter merged commit 6ca5c4b into trunk Jan 13, 2025
16 checks passed
@westonruter westonruter deleted the update/xpath-construction branch January 13, 2025 20:15
@westonruter westonruter mentioned this pull request Jan 13, 2025
Merged
@westonruter westonruter changed the title Omit element node index in XPaths up to children of BODY Omit element node index in XPaths up to children of BODY Jan 25, 2025
@westonruter westonruter mentioned this pull request Jan 26, 2025
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixarntz felixarntz felixarntz approved these changes

@adamsilverstein adamsilverstein Awaiting requested review from adamsilverstein adamsilverstein is a code owner

Assignees

No one assigned

Labels

[Plugin] Embed Optimizer Issues for the Embed Optimizer plugin (formerly Auto Sizes) [Plugin] Image Prioritizer Issues for the Image Prioritizer plugin (dependent on Optimization Detective) [Plugin] Optimization Detective Issues for the Optimization Detective plugin [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Milestone

optimization-detective 1.0.0-beta1

Development

Successfully merging this pull request may close these issues.

XPaths for elements in URL Metrics can omit index for children of BODY to avoid needing to vary URL Metrics by whether a user is logged-in

2 participants

@westonruter @felixarntz