diff --git a/src/injected/util.ts b/src/injected/util.ts index 5279050c0cdc4..7895b50e45175 100644 --- a/src/injected/util.ts +++ b/src/injected/util.ts @@ -19,6 +19,8 @@ export const createFunction = ( return fn; }; +const HIDDEN_VISIBILITY_VALUES = ['hidden', 'collapse']; + /** * @internal */ @@ -38,11 +40,20 @@ export const checkVisibility = ( const style = window.getComputedStyle(element); const isVisible = - style && style.visibility !== 'hidden' && isBoundingBoxVisible(element); + style && + !HIDDEN_VISIBILITY_VALUES.includes(style.visibility) && + isBoundingBoxVisible(element); return visible === isVisible ? node : false; }; function isBoundingBoxVisible(element: Element): boolean { const rect = element.getBoundingClientRect(); - return !!(rect.top || rect.bottom || rect.width || rect.height); + return ( + rect.width > 0 && + rect.height > 0 && + rect.right > 0 && + rect.bottom > 0 && + rect.left < self.innerWidth && + rect.top < self.innerHeight + ); } diff --git a/test/src/ariaqueryhandler.spec.ts b/test/src/ariaqueryhandler.spec.ts index 5071432eec334..fabdf92175e60 100644 --- a/test/src/ariaqueryhandler.spec.ts +++ b/test/src/ariaqueryhandler.spec.ts @@ -446,7 +446,7 @@ describe('AriaQueryHandler', () => { let divHidden = false; await page.setContent( - `
` + `
text
` ); const waitForSelector = page .waitForSelector('aria/[role="button"]', {hidden: true}) @@ -468,7 +468,9 @@ describe('AriaQueryHandler', () => { const {page} = getTestState(); let divHidden = false; - await page.setContent(`
`); + await page.setContent( + `
text
` + ); const waitForSelector = page .waitForSelector('aria/[role="main"]', {hidden: true}) .then(() => { @@ -488,7 +490,7 @@ describe('AriaQueryHandler', () => { it('hidden should wait for removal', async () => { const {page} = getTestState(); - await page.setContent(`
`); + await page.setContent(`
text
`); let divRemoved = false; const waitForSelector = page .waitForSelector('aria/[role="main"]', {hidden: true}) @@ -516,13 +518,13 @@ describe('AriaQueryHandler', () => { it('should respect timeout', async () => { const {page, puppeteer} = getTestState(); - let error!: Error; - await page - .waitForSelector('aria/[role="button"]', {timeout: 10}) - .catch(error_ => { - return (error = error_); + const error = await page + .waitForSelector('aria/[role="button"]', { + timeout: 10, + }) + .catch(error => { + return error; }); - expect(error).toBeTruthy(); expect(error.message).toContain( 'Waiting for selector `[role="button"]` failed: Waiting failed: 10ms exceeded' ); @@ -532,17 +534,15 @@ describe('AriaQueryHandler', () => { it('should have an error message specifically for awaiting an element to be hidden', async () => { const {page} = getTestState(); - await page.setContent(`
`); - let error!: Error; - await page - .waitForSelector('aria/[role="main"]', {hidden: true, timeout: 10}) - .catch(error_ => { - return (error = error_); - }); - expect(error).toBeTruthy(); - expect(error.message).toContain( - 'Waiting for selector `[role="main"]` failed: Waiting failed: 10ms exceeded' - ); + await page.setContent(`
text
`); + const promise = page.waitForSelector('aria/[role="main"]', { + hidden: true, + timeout: 10, + }); + await expect(promise).rejects.toMatchObject({ + message: + 'Waiting for selector `[role="main"]` failed: Waiting failed: 10ms exceeded', + }); }); it('should respond to node attribute mutation', async () => { diff --git a/test/src/mocha-utils.ts b/test/src/mocha-utils.ts index 6e193f49dd76c..7d819921d0a12 100644 --- a/test/src/mocha-utils.ts +++ b/test/src/mocha-utils.ts @@ -295,3 +295,14 @@ export const shortWaitForArrayToHaveAtLeastNElements = async ( }); } }; + +export const createTimeout = ( + n: number, + value?: T +): Promise => { + return new Promise(resolve => { + setTimeout(() => { + return resolve(value); + }, n); + }); +}; diff --git a/test/src/waittask.spec.ts b/test/src/waittask.spec.ts index 69dbad488d6ea..28c4766982fed 100644 --- a/test/src/waittask.spec.ts +++ b/test/src/waittask.spec.ts @@ -17,6 +17,7 @@ import expect from 'expect'; import {isErrorLike} from '../../lib/cjs/puppeteer/util/ErrorLike.js'; import { + createTimeout, getTestState, setupTestBrowserHooks, setupTestPageAndContextHooks, @@ -478,113 +479,186 @@ describe('waittask specs', function () { await waitForSelector; expect(boxFound).toBe(true); }); - it('should wait for visible', async () => { + it('should wait for element to be visible (display)', async () => { const {page} = getTestState(); - let divFound = false; - const waitForSelector = page - .waitForSelector('div', {visible: true}) - .then(() => { - return (divFound = true); - }); - await page.setContent( - `
1
` - ); - expect(divFound).toBe(false); - await page.evaluate(() => { - return document.querySelector('div')?.style.removeProperty('display'); + const promise = page.waitForSelector('div', {visible: true}); + await page.setContent('
text
'); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; }); - expect(divFound).toBe(false); - await page.evaluate(() => { - return document - .querySelector('div') - ?.style.removeProperty('visibility'); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.removeProperty('display'); }); - expect(await waitForSelector).toBe(true); - expect(divFound).toBe(true); + await expect(promise).resolves.toBeTruthy(); }); - it('should wait for visible recursively', async () => { + it('should wait for element to be visible (visibility)', async () => { const {page} = getTestState(); - let divVisible = false; - const waitForSelector = page - .waitForSelector('div#inner', {visible: true}) - .then(() => { - return (divVisible = true); - }); + const promise = page.waitForSelector('div', {visible: true}); + await page.setContent('
text
'); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.setProperty('visibility', 'collapse'); + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.removeProperty('visibility'); + }); + await expect(promise).resolves.toBeTruthy(); + }); + it('should wait for element to be visible (bounding box)', async () => { + const {page} = getTestState(); + + const promise = page.waitForSelector('div', {visible: true}); + await page.setContent('
text
'); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.setProperty('height', '0'); + e.style.removeProperty('width'); + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.setProperty('position', 'absolute'); + e.style.setProperty('right', '100vw'); + e.style.removeProperty('height'); + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.setProperty('left', '100vw'); + e.style.removeProperty('right'); + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.setProperty('top', '100vh'); + e.style.removeProperty('left'); + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.setProperty('bottom', '100vh'); + e.style.removeProperty('top'); + }); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + // Just peeking + e.style.setProperty('bottom', '99vh'); + }); + await expect(promise).resolves.toBeTruthy(); + }); + it('should wait for element to be visible recursively', async () => { + const {page} = getTestState(); + + const promise = page.waitForSelector('div#inner', { + visible: true, + }); await page.setContent( `
hi
` ); - expect(divVisible).toBe(false); - await page.evaluate(() => { - return document.querySelector('div')?.style.removeProperty('display'); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; }); - expect(divVisible).toBe(false); - await page.evaluate(() => { - return document - .querySelector('div') - ?.style.removeProperty('visibility'); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + return e.style.removeProperty('display'); }); - expect(await waitForSelector).toBe(true); - expect(divVisible).toBe(true); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + return e.style.removeProperty('visibility'); + }); + await expect(promise).resolves.toBeTruthy(); }); - it('hidden should wait for visibility: hidden', async () => { + it('should wait for element to be hidden (visibility)', async () => { const {page} = getTestState(); - let divHidden = false; - await page.setContent(`
`); - const waitForSelector = page - .waitForSelector('div', {hidden: true}) - .then(() => { - return (divHidden = true); - }); - await page.waitForSelector('div'); // do a round trip - expect(divHidden).toBe(false); - await page.evaluate(() => { - return document - .querySelector('div') - ?.style.setProperty('visibility', 'hidden'); + const promise = page.waitForSelector('div', {hidden: true}); + await page.setContent(`
text
`); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; }); - expect(await waitForSelector).toBe(true); - expect(divHidden).toBe(true); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + return e.style.setProperty('visibility', 'hidden'); + }); + await expect(promise).resolves.toBeTruthy(); }); - it('hidden should wait for display: none', async () => { + it('should wait for element to be hidden (display)', async () => { const {page} = getTestState(); - let divHidden = false; - await page.setContent(`
`); - const waitForSelector = page - .waitForSelector('div', {hidden: true}) - .then(() => { - return (divHidden = true); - }); - await page.waitForSelector('div'); // do a round trip - expect(divHidden).toBe(false); - await page.evaluate(() => { - return document - .querySelector('div') - ?.style.setProperty('display', 'none'); + const promise = page.waitForSelector('div', {hidden: true}); + await page.setContent(`
text
`); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; }); - expect(await waitForSelector).toBe(true); - expect(divHidden).toBe(true); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + return e.style.setProperty('display', 'none'); + }); + await expect(promise).resolves.toBeTruthy(); }); - it('hidden should wait for removal', async () => { + it('should wait for element to be hidden (bounding box)', async () => { const {page} = getTestState(); - await page.setContent(`
`); - let divRemoved = false; - const waitForSelector = page - .waitForSelector('div', {hidden: true}) - .then(() => { - return (divRemoved = true); - }); - await page.waitForSelector('div'); // do a round trip - expect(divRemoved).toBe(false); - await page.evaluate(() => { - return document.querySelector('div')?.remove(); + const promise = page.waitForSelector('div', {hidden: true}); + await page.setContent('
text
'); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; }); - expect(await waitForSelector).toBe(true); - expect(divRemoved).toBe(true); + await expect( + Promise.race([promise, createTimeout(40)]) + ).resolves.toBeFalsy(); + await element.evaluate(e => { + e.style.setProperty('height', '0'); + }); + await expect(promise).resolves.toBeTruthy(); + }); + it('should wait for element to be hidden (removal)', async () => { + const {page} = getTestState(); + + const promise = page.waitForSelector('div', {hidden: true}); + await page.setContent(`
text
`); + const element = await page.evaluateHandle(() => { + return document.getElementsByTagName('div')[0]!; + }); + await expect( + Promise.race([promise, createTimeout(40, true)]) + ).resolves.toBeTruthy(); + await element.evaluate(e => { + e.remove(); + }); + await expect(promise).resolves.toBeFalsy(); }); it('should return null if waiting to hide non-existing element', async () => { const {page} = getTestState(); @@ -609,7 +683,7 @@ describe('waittask specs', function () { it('should have an error message specifically for awaiting an element to be hidden', async () => { const {page} = getTestState(); - await page.setContent(`
`); + await page.setContent(`
text
`); let error!: Error; await page .waitForSelector('div', {hidden: true, timeout: 10}) @@ -725,7 +799,7 @@ describe('waittask specs', function () { const {page} = getTestState(); let divHidden = false; - await page.setContent(`
`); + await page.setContent(`
text
`); const waitForXPath = page .waitForXPath('//div', {hidden: true}) .then(() => {