3.17: Beacon script isnot injected in the uncached page if URL have LRC and not have LCP
Before submitting an issue please check that you’ve completed the following steps:
-
Made sure you’re on the latest version => commit b34405a03 https://github.com/wp-media/wp-rocket/pull/6888
-
Used the search feature to ensure that the bug hasn’t been reported before
Describe the bug If URL have LRC and not LCP, the script is not injected when visiting the uncached page
To Reproduce Steps to reproduce the behavior:
- Visit a page => LCP and LRC added to DB
- Manually remove the LCP entry
- Clear cache
- visit the page
Expected behavior If one of the performance hints not exist in DB then script is injected to an uncached visit
Screenshots If applicable, add screenshots to help explain your problem.
Additional context Add any other context about the problem here.
Acceptance Criteria (for WP Media team use only) Clear instructions for developers, to be added before the grooming
- Clear performance hints and visit a page => script is injected as neither LCP nor LRC are in DB
- Clear cache and revisit the page => script not injected as both LCP and LRC are in DB
- Delete LRC data then clear cache and revisit the page => script is injected as LRC is not in DB
- Delete LCP data then clear cache and revisit the page => script is injected as LCP is not in DB
@jeawhanlee this issue has been moved to In Progress without grooming. I assume because it was possibly a quick fix in the first place? But it has been 4 days so maybe it is not and should get a grooming?
I don't know if this is linked but I was investigating something else and found this code: https://github.com/wp-media/wp-rocket/blob/fb50ef3b9db98b09b4b90797f5c16117a1560435/inc/Engine/Common/PerformanceHints/Frontend/Processor.php#L61-L78
$optimization_applied is badly handled and could be a problem for this issue:
- It should be set to true before the for loop.
- it should be set to false in if ( empty( $row ) ) { (as it is already the case, to capture the fact that at least one optimization could not be applied).
- it should not be set to true at the end of the loop, to avoid resetting the value to true after it was false once.
The current code only accounts for the last factory of the loop because of this. Hence, if the last factory is LRC and LRC data exists, then $optimization_applied is set to true despite the fact that it was false after the LCP check.
The comment is misleading to. We should inject beacon if at least one optimization was not applied.