wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
687 stars 215 forks source link

3.17 - Make depth and type of LRC candidates filterable #6904

Closed MathieuLamiot closed 15 hours ago

MathieuLamiot commented 3 weeks ago

Context Currently, PHP adds hashes to LRC candidates up to depth 2 inside the body. Note that the depth and type are currently also checked by the beacon, but it should not and will be removed as part of https://github.com/wp-media/rocket-scripts/issues/25 Until then, it is possible to implement this issue and test it by checking what elements get the hash added, but they might not be picked up correctly by the beacon, hence not be in DB at the end of the visit.

Expected behavior We must provide a filter to change how deep we look for eligible LRC candidates to add the hashes to. We must provide a filter to change the list of eligible element types for LRC candidates.

Acceptance Criteria

Proposed grooming I think the solution can be:

The constant here:

$this->add_hash_to_element( $body, 2 );

in /Users/mathieu/Documents/Github/wp-rocket/inc/Engine/Optimization/LazyRenderContent/Frontend/Processor/SimpleHtmlDom.php and /Users/mathieu/Documents/Github/wp-rocket/inc/Engine/Optimization/LazyRenderContent/Frontend/Processor/Dom.php

Same thing for the $skip_tags in:

/Users/mathieu/Documents/Github/wp-rocket/inc/Engine/Optimization/LazyRenderContent/Frontend/Processor/SimpleHtmlDom.php and /Users/mathieu/Documents/Github/wp-rocket/inc/Engine/Optimization/LazyRenderContent/Frontend/Processor/Dom.php /Users/mathieu/Documents/Github/wp-rocket/inc/Engine/Optimization/LazyRenderContent/Frontend/Processor/Regex.php

Effort: XS

jeawhanlee commented 3 weeks ago

@MathieuLamiot I'm not sure to understand the grooming completely to help me review, please could you elaborate. I can see how we can have a depth filter for DOM & SimpleDOM but how so for REGEX, it's not clear for me.

MathieuLamiot commented 3 weeks ago

For regex, we don't. In case of Regex processor, identifying depth was too complex hence discarded. Thisbis why the depth parameter is only for the 2 other processors. The list of eligible elements is used by the 3 processors though.

Therefore, depth control and related ACs don't apply to regex processor

Note that we will likely remove SimpleDom and Regex before the release (To be discussed and confirmed next week)

jeawhanlee commented 3 weeks ago

LGTM then...