Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mai-Saad merged 2 commits into from
Jul 30, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95320ba
Enhance LCP candidate generation to filter out invisible elements
Miraeld Jul 17, 2025
ca28a62
Refactor visibility checks to use BeaconUtils methods for consistency…
Miraeld Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/BeaconLcp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,13 @@ class BeaconLcp {
})
.filter(item => item !== null)
.filter(item => {
const style = window.getComputedStyle(item.element);
const isVisible = !(style.display === "none" || style.visibility === "hidden" || style.opacity === "0");
Comment thread
Miraeld marked this conversation as resolved.
Outdated
return (
item.rect.width > 0 &&
item.rect.height > 0 &&
BeaconUtils.isIntersecting(item.rect)
BeaconUtils.isIntersecting(item.rect) &&
isVisible
);
Comment thread
Miraeld marked this conversation as resolved.
Outdated
})
.map(item => ({
Expand Down
250 changes: 250 additions & 0 deletions test/BeaconLcp.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,254 @@ describe('BeaconManager', function() {
assert.strictEqual(elementInfo, null);
});
});

describe('#_generateLcpCandidates()', function() {
let mockElements;

beforeEach(function() {
mockElements = [
{
nodeName: 'IMG',
parentElement: { nodeName: 'div' },
getBoundingClientRect: () => ({
width: 300,
height: 200,
top: 0,
left: 0,
bottom: 200,
right: 300
})
},
{
nodeName: 'IMG',
parentElement: { nodeName: 'div' },
getBoundingClientRect: () => ({
width: 250,
height: 150,
top: 10,
left: 10,
bottom: 160,
right: 260
})
},
{
nodeName: 'IMG',
parentElement: { nodeName: 'div' },
getBoundingClientRect: () => ({
width: 200,
height: 100,
top: 20,
left: 20,
bottom: 120,
right: 220
})
}
];

global.document = {
querySelectorAll: () => mockElements
};

global.window = {
...global.window,
innerWidth: 1200,
innerHeight: 800,
getComputedStyle: sinon.stub()
};

// Mock BeaconUtils.isIntersecting to return true by default
const UtilsStub = sinon.stub();
UtilsStub.isIntersecting = sinon.stub().returns(true);
beacon.constructor.Utils = UtilsStub;
});

afterEach(function() {
sinon.restore();
});

it('should filter out elements with opacity: 0', function() {
// Setup: first element has opacity 0, second is visible
global.window.getComputedStyle
.onCall(0).returns({ display: 'block', visibility: 'visible', opacity: '0' })
.onCall(1).returns({ display: 'block', visibility: 'visible', opacity: '1' })
.onCall(2).returns({ display: 'block', visibility: 'visible', opacity: '1' });

beacon.config = { elements: 'img' };

// Mock _getElementInfo to return valid info for visible elements
sinon.stub(beacon, '_getElementInfo').returns({ src: 'test.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

// Should only return 2 candidates (excluding the one with opacity: 0)
assert.strictEqual(candidates.length, 2);
assert.strictEqual(candidates[0].element, mockElements[1]);
assert.strictEqual(candidates[1].element, mockElements[2]);
});

it('should filter out elements with visibility: hidden', function() {
// Setup: first element has visibility hidden, others are visible
global.window.getComputedStyle
.onCall(0).returns({ display: 'block', visibility: 'hidden', opacity: '1' })
.onCall(1).returns({ display: 'block', visibility: 'visible', opacity: '1' })
.onCall(2).returns({ display: 'block', visibility: 'visible', opacity: '1' });

beacon.config = { elements: 'img' };
sinon.stub(beacon, '_getElementInfo').returns({ src: 'test.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

assert.strictEqual(candidates.length, 2);
assert.strictEqual(candidates[0].element, mockElements[1]);
assert.strictEqual(candidates[1].element, mockElements[2]);
});

it('should filter out elements with display: none', function() {
// Setup: first element has display none, others are visible
global.window.getComputedStyle
.onCall(0).returns({ display: 'none', visibility: 'visible', opacity: '1' })
.onCall(1).returns({ display: 'block', visibility: 'visible', opacity: '1' })
.onCall(2).returns({ display: 'block', visibility: 'visible', opacity: '1' });

beacon.config = { elements: 'img' };
sinon.stub(beacon, '_getElementInfo').returns({ src: 'test.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

assert.strictEqual(candidates.length, 2);
assert.strictEqual(candidates[0].element, mockElements[1]);
assert.strictEqual(candidates[1].element, mockElements[2]);
});

it('should include elements with visible styles', function() {
// All elements are visible
global.window.getComputedStyle.returns({ display: 'block', visibility: 'visible', opacity: '1' });

beacon.config = { elements: 'img' };
sinon.stub(beacon, '_getElementInfo').returns({ src: 'test.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

assert.strictEqual(candidates.length, 3);
// Should be sorted by area (largest first)
assert.strictEqual(candidates[0].element, mockElements[0]); // 300x200 = 60000
assert.strictEqual(candidates[1].element, mockElements[1]); // 250x150 = 37500
assert.strictEqual(candidates[2].element, mockElements[2]); // 200x100 = 20000
});

it('should handle multiple hidden elements with different visibility issues', function() {
// Mix of visibility issues
global.window.getComputedStyle
.onCall(0).returns({ display: 'none', visibility: 'visible', opacity: '1' }) // hidden by display
.onCall(1).returns({ display: 'block', visibility: 'hidden', opacity: '1' }) // hidden by visibility
.onCall(2).returns({ display: 'block', visibility: 'visible', opacity: '0' }); // hidden by opacity

beacon.config = { elements: 'img' };
sinon.stub(beacon, '_getElementInfo').returns({ src: 'test.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

// All elements should be filtered out
assert.strictEqual(candidates.length, 0);
});

it('should handle edge case with very low opacity but not zero', function() {
// Test with very low opacity (0.01) - should be included as it's not exactly 0
global.window.getComputedStyle
.onCall(0).returns({ display: 'block', visibility: 'visible', opacity: '0.01' })
.onCall(1).returns({ display: 'block', visibility: 'visible', opacity: '1' })
.onCall(2).returns({ display: 'block', visibility: 'visible', opacity: '0' });

beacon.config = { elements: 'img' };
sinon.stub(beacon, '_getElementInfo').returns({ src: 'test.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

// Should include elements with opacity 0.01 and 1, but not 0
assert.strictEqual(candidates.length, 2);
assert.strictEqual(candidates[0].element, mockElements[0]);
assert.strictEqual(candidates[1].element, mockElements[1]);
});

it('should maintain existing functionality for elements with zero dimensions', function() {
// Test that elements with zero width/height are still filtered out
mockElements[0].getBoundingClientRect = () => ({
width: 0,
height: 200,
top: 0,
left: 0,
bottom: 200,
right: 0
});

global.window.getComputedStyle.returns({ display: 'block', visibility: 'visible', opacity: '1' });

beacon.config = { elements: 'img' };
sinon.stub(beacon, '_getElementInfo').returns({ src: 'test.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

// Should exclude element with zero width
assert.strictEqual(candidates.length, 2);
assert.strictEqual(candidates[0].element, mockElements[1]);
assert.strictEqual(candidates[1].element, mockElements[2]);
});

it('should reproduce and fix the belivria.com bug scenario', function() {
// Simulate the belivria.com scenario from the bug report
const hiddenImage = {
nodeName: 'IMG',
src: 'test_inline2.jpeg',
parentElement: { nodeName: 'div' },
getBoundingClientRect: () => ({
width: 350,
height: 350,
top: 50,
left: 50,
bottom: 400,
right: 400
})
};

const visibleImage = {
nodeName: 'IMG',
src: 'img_nature.jpg',
parentElement: { nodeName: 'div' },
getBoundingClientRect: () => ({
width: 350,
height: 350,
top: 50,
left: 50,
bottom: 400,
right: 400
})
};

global.document.querySelectorAll = () => [hiddenImage, visibleImage];
Comment thread
Miraeld marked this conversation as resolved.

// Mock computed styles - hidden image has opacity: 0, visible image has opacity: 1
global.window.getComputedStyle
.withArgs(hiddenImage).returns({ display: 'block', visibility: 'visible', opacity: '0' })
.withArgs(visibleImage).returns({ display: 'block', visibility: 'visible', opacity: '1' });

beacon.config = { elements: 'img' };

// Mock _getElementInfo to return valid image info
sinon.stub(beacon, '_getElementInfo')
.withArgs(hiddenImage).returns({ src: 'test_inline2.jpeg', type: 'img' })
.withArgs(visibleImage).returns({ src: 'img_nature.jpg', type: 'img' });

const candidates = beacon._generateLcpCandidates(10);

// Before fix: would return hiddenImage as first candidate due to DOM order
// After fix: should only return visibleImage
assert.strictEqual(candidates.length, 1);
assert.strictEqual(candidates[0].element, visibleImage);
assert.strictEqual(candidates[0].elementInfo.src, 'img_nature.jpg');

// Verify the hidden image is not in candidates
const hiddenImageCandidate = candidates.find(c => c.element === hiddenImage);
assert.strictEqual(hiddenImageCandidate, undefined);
});
});
});