From 92694807ea2e16102181b38d53cc62f54421a988 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Wed, 9 Oct 2024 11:44:33 +0100 Subject: [PATCH 1/6] Closes #6959 UI issue after clear cache and apply LRC with certain template (#30) * Add new method to avoid conflicts with lrc -- :closes: wp-media/wp-rocket#6959 * Add tests for lrc conflicts method * Fixed codacy error * :sparkles: fixed codacy issues --------- Co-authored-by: WordPress Fan <146129302+wordpressfan@users.noreply.github.com> --- src/BeaconLrc.js | 57 ++++++++++++++++++++++ test/BeaconLrc.test.js | 104 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 157 insertions(+), 4 deletions(-) diff --git a/src/BeaconLrc.js b/src/BeaconLrc.js index 52fd10d..404297d 100644 --- a/src/BeaconLrc.js +++ b/src/BeaconLrc.js @@ -72,6 +72,57 @@ class BeaconLrc { return false; } + _checkLcrConflict(element) { + const conflictingElements = []; + const computedStyle = window.getComputedStyle(element); + + const validMargins = ['marginTop', 'marginRight', 'marginBottom', 'marginLeft']; + + const negativeMargins = validMargins + .some(margin => parseFloat(computedStyle[margin]) < 0); + + const currentElementConflicts = negativeMargins || + computedStyle.contentVisibility === 'auto' || + computedStyle.contentVisibility === 'hidden'; + + if (currentElementConflicts) { + conflictingElements.push({ + element, + conflicts: [ + negativeMargins && 'negative margin', + computedStyle.contentVisibility === 'auto' && 'content-visibility:auto', + computedStyle.contentVisibility === 'hidden' && 'content-visibility:hidden' + ].filter(Boolean) + }); + } + + Array.from(element.children).forEach(child => { + const childStyle = window.getComputedStyle(child); + + const validMargins = ['marginTop', 'marginRight', 'marginBottom', 'marginLeft']; + + const childNegativeMargins = validMargins + .some(margin => parseFloat(childStyle[margin]) < 0); + + const childConflicts = childNegativeMargins || + childStyle.position === 'absolute' || + childStyle.position === 'fixed'; + + if (childConflicts) { + conflictingElements.push({ + element: child, + conflicts: [ + childNegativeMargins && 'negative margin', + childStyle.position === 'absolute' && 'position:absolute', + childStyle.position === 'fixed' && 'position:fixed' + ].filter(Boolean) + }); + } + }); + + return conflictingElements; + } + _processElements(elements) { elements.forEach(({ element, depth, distance, hash }) => { if (this._shouldSkipElement(element, this.config.exclusions || [])) { @@ -82,6 +133,12 @@ class BeaconLrc { return; } + const conflicts = this._checkLcrConflict(element); + if (conflicts.length > 0) { + this.logger.logMessage('Skipping element due to conflicts:', conflicts); + return; + } + const can_push_hash = element.parentElement && this._getElementDistance(element.parentElement) < this.config.lrc_threshold && distance >= this.config.lrc_threshold; const color = can_push_hash ? "green" : distance === 0 ? "red" : ""; diff --git a/test/BeaconLrc.test.js b/test/BeaconLrc.test.js index d0724be..ef1f6d3 100644 --- a/test/BeaconLrc.test.js +++ b/test/BeaconLrc.test.js @@ -16,7 +16,8 @@ describe('BeaconLrc', function() { }, getAttribute: () => 'hash1', hasAttribute: () => true, - dataset: { rocketLocationHash: 'hash1' } + dataset: { rocketLocationHash: 'hash1' }, + children: [] }, { getBoundingClientRect: () => { @@ -26,7 +27,8 @@ describe('BeaconLrc', function() { }, getAttribute: () => 'hash2', hasAttribute: () => true, - dataset: { rocketLocationHash: 'hash2' } + dataset: { rocketLocationHash: 'hash2' }, + children: [] }, { getBoundingClientRect: () => { @@ -36,7 +38,8 @@ describe('BeaconLrc', function() { }, getAttribute: () => 'hash3', hasAttribute: () => true, - dataset: { rocketLocationHash: 'hash3' } + dataset: { rocketLocationHash: 'hash3' }, + children: [] }, { getBoundingClientRect: () => { @@ -46,7 +49,8 @@ describe('BeaconLrc', function() { }, getAttribute: () => 'hash4', hasAttribute: () => true, - dataset: { rocketLocationHash: 'hash4' } + dataset: { rocketLocationHash: 'hash4' }, + children: [] }, ]; @@ -74,6 +78,18 @@ describe('BeaconLrc', function() { // Mocking window.pageYOffset global.window = { pageYOffset: 100, innerHeight: 500 }; + + if (typeof global.window.getComputedStyle !== 'function') { + global.window.getComputedStyle = () => ({ + marginTop: '0px', + marginRight: '0px', + marginBottom: '0px', + marginLeft: '0px', + contentVisibility: 'visible', + position: 'static', + overflow: 'visible' + }); + } }); afterEach(function() { @@ -232,4 +248,84 @@ describe('BeaconLrc', function() { _getElementXPathStub.restore(); }); + + it('should detect conflict when element has negative margins', () => { + const element = mockElements[0]; + + sinon.stub(window, 'getComputedStyle').callsFake(() => ({ + marginTop: '-10px', + marginRight: '0px', + marginBottom: '0px', + marginLeft: '0px', + contentVisibility: 'visible', + position: 'static', + overflow: 'visible' + })); + + const result = beaconLrc._checkLcrConflict(element); + + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].conflicts.includes('negative margin'), true); + window.getComputedStyle.restore(); + }); + + it('should detect conflict when content visibility is hidden', () => { + const element = mockElements[0]; + + sinon.stub(window, 'getComputedStyle').callsFake(() => ({ + marginTop: '0px', + marginRight: '0px', + marginBottom: '0px', + marginLeft: '0px', + contentVisibility: 'hidden', + position: 'static', + overflow: 'visible' + })); + + const result = beaconLrc._checkLcrConflict(element); + + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].conflicts.includes('content-visibility:hidden'), true); + window.getComputedStyle.restore(); + }); + + it('should detect conflict when child has negative margins', () => { + const element = mockElements[0]; + + const child = { + getBoundingClientRect: () => ({ top: 500 }), + getAttribute: () => null, + hasAttribute: () => false, + children: [] + }; + element.children = [child]; + + sinon.stub(window, 'getComputedStyle') + .onCall(0).returns({ + marginTop: '0px', + marginRight: '0px', + marginBottom: '0px', + marginLeft: '0px', + contentVisibility: 'visible', + position: 'static', + overflow: 'visible' + }) + .onCall(1).returns({ + marginTop: '-20px', + marginRight: '0px', + marginBottom: '0px', + marginLeft: '0px', + contentVisibility: 'visible', + position: 'absolute', + overflow: 'visible' + }); + + const result = beaconLrc._checkLcrConflict(element); + + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].element, child); + assert.strictEqual(result[0].conflicts.includes('negative margin'), true); + + window.getComputedStyle.restore(); + }) }); \ No newline at end of file From 261e1f80730f13d40db963d15ca19646067e6a35 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Wed, 9 Oct 2024 12:52:52 +0100 Subject: [PATCH 2/6] Change package version (#36) * Add new method to avoid conflicts with lrc -- :closes: wp-media/wp-rocket#6959 * Add tests for lrc conflicts method * Fixed codacy error * :sparkles: fixed codacy issues * change package version number --------- Co-authored-by: WordPress Fan <146129302+wordpressfan@users.noreply.github.com> --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 178b97f..e727432 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "wp-rocket-scripts", - "version": "1.0.6", + "version": "1.0.7", "description": "Rocket main scripts packages", "type": "module", "main": "./src/BeaconEntryPoint.js", From 9f02ec4e0ed71a8124f8ab4e7d5f0881cc3e3865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Tue, 15 Oct 2024 17:38:59 +0200 Subject: [PATCH 3/6] Closes #34: exclude svg use from lrc (#37) * Closes #34: exclude svg use from lrc * Added test * Added test --------- Co-authored-by: Michael Lee --- src/BeaconLrc.js | 28 +++++++++++++++++++++++++++- test/BeaconLrc.test.js | 27 ++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/BeaconLrc.js b/src/BeaconLrc.js index 404297d..53893ed 100644 --- a/src/BeaconLrc.js +++ b/src/BeaconLrc.js @@ -23,12 +23,23 @@ class BeaconLrc { _getLazyRenderElements() { const elements = document.querySelectorAll('[data-rocket-location-hash]'); + const svgUseTargets = this._getSvgUseTargets(); if (elements.length <= 0) { return []; } - const validElements = Array.from(elements).filter(element => !this._skipElement(element)); + const validElements = Array.from(elements).filter(element => { + if (this._skipElement(element)) { + return false; + } + if (svgUseTargets.includes(element)) { + this.logger.logColoredMessage(`Element skipped because of SVG: ${element.tagName}`, 'orange'); + return false; + } + return true; + }); + return validElements.map(element => ({ element: element, @@ -192,6 +203,21 @@ class BeaconLrc { : 'No hash detected'; } + _getSvgUseTargets() { + const useElements = document.querySelectorAll('use'); + const targets = new Set(); + + useElements.forEach(use => { + let parent = use.parentElement; + while (parent && parent !== document.body) { + targets.add(parent); + parent = parent.parentElement; + } + }); + + return Array.from(targets); + } + getResults() { return this.lazyRenderElements; } diff --git a/test/BeaconLrc.test.js b/test/BeaconLrc.test.js index ef1f6d3..3487515 100644 --- a/test/BeaconLrc.test.js +++ b/test/BeaconLrc.test.js @@ -57,7 +57,7 @@ describe('BeaconLrc', function() { // Mocking document.querySelectorAll global.document = { querySelectorAll: (selector) => { - if (selector === '[data-rocket-location-hash]') { + if (selector === '[data-rocket-location-hash]' || selector === 'use') { return mockElements; } return []; @@ -148,6 +148,21 @@ describe('BeaconLrc', function() { _skipElementStub.restore(); }); + it('should skip elements with svg use', function() { + const _getElementDepthStub = sinon.stub(beaconLrc, '_getElementDepth'); + _getElementDepthStub.returns(1); + + const _svgElementStub = sinon.stub(beaconLrc, '_getSvgUseTargets'); + _svgElementStub.returns([mockElements[2]]); + + const elements = beaconLrc._getLazyRenderElements(); + const skippedElement = elements.find(el => el.hash === 'hash3'); + assert.strictEqual(skippedElement, undefined); + + _getElementDepthStub.restore(); + _svgElementStub.restore(); + }); + it('should return correct distance', () => { const distance = beaconLrc._getElementDistance(mockElements[2]); assert.strictEqual(distance, 600); @@ -328,4 +343,14 @@ describe('BeaconLrc', function() { window.getComputedStyle.restore(); }) + + it('should return the correct SVG use target elements', function() { + mockElements[0].parentElement = { tagName: 'svg', parentElement: null }; + mockElements[1].parentElement = { tagName: 'div', parentElement: null }; + + const targets = beaconLrc._getSvgUseTargets(); + assert.strictEqual(targets.length, 2); + assert.strictEqual(targets[0].tagName, 'svg'); + assert.strictEqual(targets[1].tagName, 'div'); + }); }); \ No newline at end of file From b612364bcf853bb457a230b3d9c0b0065cde6749 Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Tue, 15 Oct 2024 16:42:26 +0100 Subject: [PATCH 4/6] Closes #6728 Largest element with a background-image that is not kept (#33) * fixed lcp background image :bug: :closes: wp-media/wp-rocket#6728 * Add tests * Improve test coverage --- src/BeaconLcp.js | 8 ++++- test/BeaconLcp.test.js | 70 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/BeaconLcp.js b/src/BeaconLcp.js index a087b96..5dc5ab1 100644 --- a/src/BeaconLcp.js +++ b/src/BeaconLcp.js @@ -153,6 +153,10 @@ class BeaconLcp { element_info.bg_set = matches.map(m => m[1] ? { src: m[1].trim() } : {}); } + if (element_info.bg_set.length <= 0) { + return null; + } + if (element_info.bg_set.length > 0) { element_info.src = element_info.bg_set[0].src; if (element_info.type === "bg-img-set") { @@ -165,7 +169,9 @@ class BeaconLcp { } _initWithFirstElementWithInfo(elements) { - const firstElementWithInfo = elements.find(item => item.elementInfo !== null); + const firstElementWithInfo = elements.find(item => { + return item.elementInfo !== null && (item.elementInfo.src || item.elementInfo.srcset); + }); if (!firstElementWithInfo) { this.logger.logMessage("No LCP candidate found."); diff --git a/test/BeaconLcp.test.js b/test/BeaconLcp.test.js index 297b152..af13966 100644 --- a/test/BeaconLcp.test.js +++ b/test/BeaconLcp.test.js @@ -1,13 +1,41 @@ import assert from 'assert'; import BeaconLcp from '../src/BeaconLcp.js'; import node_fetch from 'node-fetch'; +import sinon from 'sinon'; global.fetch = node_fetch; describe('BeaconManager', function() { - let beacon; + let beacon, + mockLogger; + const config = { nonce: 'test', url: 'http://example.com', is_mobile: false }; beforeEach(function() { - beacon = new BeaconLcp(config); + mockLogger = { logMessage: function(message) {} }; + + beacon = new BeaconLcp(config, mockLogger); + + global.window = {}; + global.document = {}; + + global.window.getComputedStyle = sinon.stub().returns({ + getPropertyValue: sinon.stub().returns('none'), + }); + + global.getComputedStyle = (element, pseudoElement) => { + return { + getPropertyValue: (prop) => { + if (prop === "background-image") { + return "none"; + } + return ""; + } + }; + }; + }); + + afterEach(function () { + sinon.restore(); + delete global.window; }); describe('#constructor()', function() { @@ -30,4 +58,42 @@ describe('BeaconManager', function() { }); }); + describe('#_initWithFirstElementWithInfo()', function() { + it('should initialize performanceImages with the first valid element info', function() { + const elements = [ + { element: { nodeName: 'div' }, elementInfo: null }, // invalid, no elementInfo + { element: { nodeName: 'img', src: 'http://example.com/image1.jpg' }, elementInfo: { type: 'img', src: 'http://example.com/image1.jpg' } }, + { element: { nodeName: 'img', src: 'http://example.com/image2.jpg' }, elementInfo: { type: 'img', src: 'http://example.com/image2.jpg' } }, + ]; + + beacon._initWithFirstElementWithInfo(elements); + + assert.strictEqual(beacon.performanceImages.length, 1); + assert.strictEqual(beacon.performanceImages[0].src, 'http://example.com/image1.jpg'); + assert.strictEqual(beacon.performanceImages[0].label, 'lcp'); + }); + + it('should not initialize performanceImages if no valid element info is found', function() { + const elements = [ + { element: { nodeName: 'div' }, elementInfo: null }, + { element: { nodeName: 'div' }, elementInfo: null }, + ]; + + beacon._initWithFirstElementWithInfo(elements); + + assert.strictEqual(beacon.performanceImages.length, 0); + }); + }); + + describe('#_getElementInfo()', function() { + it('should return null when there are no valid background images', function() { + const element = { + nodeName: 'div' + }; + + const elementInfo = beacon._getElementInfo(element); + + assert.strictEqual(elementInfo, null); + }); + }); }); From 9fee2d88740a1d32dc6c524756fba4da2ace9659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Wed, 16 Oct 2024 10:06:25 +0200 Subject: [PATCH 5/6] Closes #31: Add a check if page is scrolled down. (#32) * Closes #31: Add a check if page is scrolled down. * move isPageScrolled method to Utils class --------- Co-authored-by: WordPressFan --- src/BeaconManager.js | 6 ++++ src/Utils.js | 4 +++ test/BeaconManager.test.js | 56 +++++++++++++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/BeaconManager.js b/src/BeaconManager.js index 4f62581..c926d70 100644 --- a/src/BeaconManager.js +++ b/src/BeaconManager.js @@ -22,6 +22,12 @@ class BeaconManager { return; } + if (BeaconUtils.isPageScrolled()) { + this.logger.logMessage('Bailing out because the page has been scrolled'); + this._finalize(); + return; + } + this.infiniteLoopId = setTimeout(() => { this._handleInfiniteLoop(); }, 10000); diff --git a/src/Utils.js b/src/Utils.js index 98833de..aba6cbc 100644 --- a/src/Utils.js +++ b/src/Utils.js @@ -35,6 +35,10 @@ class BeaconUtils { ); } + static isPageScrolled() { + return window.pageYOffset > 0 || document.documentElement.scrollTop > 0; + } + } export default BeaconUtils; \ No newline at end of file diff --git a/test/BeaconManager.test.js b/test/BeaconManager.test.js index 99b2f68..e9bb29b 100644 --- a/test/BeaconManager.test.js +++ b/test/BeaconManager.test.js @@ -10,8 +10,21 @@ describe('BeaconManager', function() { let beacon; const config = { nonce: 'test', url: 'http://example.com', is_mobile: false, status: {atf: true}, width_threshold: 100, height_threshold: 100 }; beforeEach(function() { - //Deep copy of config + // Deep copy of config beacon = new BeaconManager(JSON.parse(JSON.stringify(config))); + + // Mock window and document objects + global.window = { + pageYOffset: 0 + }; + global.document = { + documentElement: { + scrollTop: 0 + }, + querySelector: sinon.stub().returns({ + setAttribute: sinon.spy() + }) + }; }); describe('#constructor()', function() { @@ -109,6 +122,47 @@ describe('BeaconManager', function() { }); }); + describe('#init()', function() { + let fetchStub; + let _isValidPreconditionsStub; + let isPageCachedStub; + let _finalizeStub; + + beforeEach(function() { + // Stub the global fetch method + _isValidPreconditionsStub = sinon.stub(beacon, '_isValidPreconditions'); + isPageCachedStub = sinon.stub(BeaconUtils, 'isPageCached'); + _finalizeStub = sinon.stub(beacon, '_finalize'); + fetchStub = sinon.stub(global, 'fetch').resolves({ + json: () => Promise.resolve({ data: false }) + }); + }); + + afterEach(function() { + // Restore the original fetch method + _isValidPreconditionsStub.restore(); + isPageCachedStub.restore(); + _finalizeStub.restore(); + fetchStub.restore(); + }); + + it('should bail out if the page is scrolled', async function() { + // Mock _isValidPreconditions + _isValidPreconditionsStub.resolves(true); + isPageCachedStub.returns(false); + + // Simulate page being scrolled + global.window.pageYOffset = 100; + global.document.documentElement.scrollTop = 100; + + await beacon.init(); + + assert.strictEqual(_isValidPreconditionsStub.calledOnce, true); + assert.strictEqual(_finalizeStub.calledOnce, true); + assert.strictEqual(fetchStub.notCalled, true); + }); + }); + describe('#_isValidPreconditions()', function() { it('should return true for desktop screensize larger than threshold', async function() { // Mocking window properties and methods since they are used in _isValidPreconditions From 22282210be27c3d813039fb5685d6e7b0c1741bd Mon Sep 17 00:00:00 2001 From: Opeyemi Ibrahim Date: Wed, 16 Oct 2024 10:14:45 +0100 Subject: [PATCH 6/6] Update version (#39) * Release v1.0.7 (#35) * Closes #6959 UI issue after clear cache and apply LRC with certain template (#30) * Add new method to avoid conflicts with lrc -- :closes: wp-media/wp-rocket#6959 * Add tests for lrc conflicts method * Fixed codacy error * :sparkles: fixed codacy issues --------- Co-authored-by: WordPress Fan <146129302+wordpressfan@users.noreply.github.com> * Change package version (#36) * Add new method to avoid conflicts with lrc -- :closes: wp-media/wp-rocket#6959 * Add tests for lrc conflicts method * Fixed codacy error * :sparkles: fixed codacy issues * change package version number --------- Co-authored-by: WordPress Fan <146129302+wordpressfan@users.noreply.github.com> --------- Co-authored-by: WordPress Fan <146129302+wordpressfan@users.noreply.github.com> * Update version to 1.0.8 --------- Co-authored-by: WordPress Fan <146129302+wordpressfan@users.noreply.github.com> --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e727432..c9aae48 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "wp-rocket-scripts", - "version": "1.0.7", + "version": "1.0.8", "description": "Rocket main scripts packages", "type": "module", "main": "./src/BeaconEntryPoint.js",