Skip to content

Fix #7505: Enhance LCP candidate generation to filter out invisible elements#62

Merged
Mai-Saad merged 2 commits intodevelopfrom
enhancement/7505-lcp-ignores-image-visibility
Jul 30, 2025
Merged

Fix #7505: Enhance LCP candidate generation to filter out invisible elements#62
Mai-Saad merged 2 commits intodevelopfrom
enhancement/7505-lcp-ignores-image-visibility

Conversation

@Miraeld
Copy link
Copy Markdown
Contributor

@Miraeld Miraeld commented Jul 17, 2025

Description

Fixes #7505
This fix resolves a bug in WP Rocket's LCP (Largest Contentful Paint) detection script where images with opacity: 0, visibility: hidden, or display: none were incorrectly being selected as LCP candidates.
This caused WP Rocket to preload and optimize invisible images instead of the actual visible content that users see, negatively impacting performance optimization accuracy.
User Impact: WP Rocket now correctly identifies only visually rendered images as LCP candidates

Type of change

  • Enhancement (non-breaking change which improves an existing functionality).

Detailed scenario

What was tested

Automated Tests:

  • ✅ Elements with opacity: 0 are filtered out from LCP candidates
  • ✅ Elements with visibility: hidden are filtered out from LCP candidates
  • ✅ Elements with display: none are filtered out from LCP candidates
  • ✅ Elements with visible styles (opacity: 1, visibility: visible, display: block) are included
  • ✅ Edge cases like very low opacity (0.01) are handled correctly (included since not exactly 0)
  • ✅ Multiple hidden elements with different visibility issues are all filtered out
  • ✅ Existing functionality for zero-dimension elements is maintained

Manual Testing:

  • Tested with the provided test template that simulates the belivria.com issue
  • Verified that visible images are correctly identified as LCP candidates while hidden images are ignored

How to test

Use the provided template and run the beacon script, you'll see the hidden image is ignored in LCP candidates.

Technical description

Documentation

This pull request enhances the BeaconLcp class by improving the filtering logic for identifying visible elements and adds comprehensive test coverage for the changes. The most significant updates include the addition of visibility checks in the filtering logic and new tests to validate these changes.

Enhancements to visibility filtering in BeaconLcp:

  • Updated the filtering logic in BeaconLcp to include a check for element visibility using window.getComputedStyle. Elements with display: none, visibility: hidden, or opacity: 0 are now excluded from the LCP candidate list. (src/BeaconLcp.js, src/BeaconLcp.jsR57-R63)

Comprehensive test coverage:

  • Added a new test suite, #_generateLcpCandidates(), in BeaconLcp.test.js to validate the improved visibility filtering logic. Tests cover various scenarios, including:
    • Elements with opacity: 0, visibility: hidden, or display: none being excluded.
    • Edge cases such as very low opacity (e.g., opacity: 0.01) and elements with zero dimensions.
    • A reproduction and fix verification for a specific bug scenario involving hidden and visible images.

New dependencies

None

Risks

None

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote a self-explanatory code about what it does.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.
  • Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

@Miraeld Miraeld self-assigned this Jul 17, 2025
Copilot AI review requested due to automatic review settings July 17, 2025 01:04
@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented Jul 17, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+3.18% (target: -1.00%) 100.00% (target: 50.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (347894a) 1828 1460 79.87%
Head commit (ca28a62) 1905 (+77) 1582 (+122) 83.04% (+3.18%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#62) 94 94 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the LCP candidate generation in BeaconLcp by filtering out elements that are not visually rendered (display: none, visibility: hidden, opacity: 0) and adds a comprehensive suite of tests to validate this behavior.

  • Added style-based visibility checks in _generateLcpCandidates.
  • Introduced extensive tests in BeaconLcp.test.js covering various visibility rules, edge cases, and the specific bug reproduction.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/BeaconLcp.js Added window.getComputedStyle checks to exclude invisible elements.
test/BeaconLcp.test.js Added tests for filtering by opacity, visibility, display, edge cases, and bug repro.

Comment thread src/BeaconLcp.js Outdated
Comment thread src/BeaconLcp.js Outdated
Comment thread test/BeaconLcp.test.js
@Miraeld Miraeld linked an issue Jul 17, 2025 that may be closed by this pull request
@Miraeld Miraeld requested a review from a team July 21, 2025 06:17
Copy link
Copy Markdown
Collaborator

@wordpressfan wordpressfan left a comment

Choose a reason for hiding this comment

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

isElementVisible(element) {
const style = window.getComputedStyle(element);
const rect = element.getBoundingClientRect();
// Exclude elements with transparent text properties
if (this.hasTransparentText(element)) {
return false;
}
return !(
style.display === 'none' ||
style.visibility === 'hidden' ||
style.opacity === '0' ||
rect.width === 0 ||
rect.height === 0
);
}

What do u think of making use of this method? we can move it into Utils class to be used when needed, what do u think?

@Miraeld
Copy link
Copy Markdown
Contributor Author

Miraeld commented Jul 22, 2025

@wordpressfan it's been updated

@jeawhanlee jeawhanlee dismissed wordpressfan’s stale review July 24, 2025 13:04

Requested changes has been implemented.

@Mai-Saad Mai-Saad merged commit 3be7674 into develop Jul 30, 2025
6 checks passed
@Mai-Saad Mai-Saad deleted the enhancement/7505-lcp-ignores-image-visibility branch July 30, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LCP detection ignores image visibility

5 participants