From 9abd48a062a4a30fb93d0b555f2fa03d3dc410f3 Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com> Date: Thu, 1 Jun 2023 21:51:16 +0200 Subject: [PATCH] fix: content() not showing comments outside html tag (#10293) --- .eslintrc.js | 6 ++++++ packages/puppeteer-core/src/api/Locator.ts | 5 ++++- .../src/common/IsolatedWorld.ts | 21 +++++-------------- .../src/common/bidi/BrowsingContext.ts | 18 +++++----------- packages/puppeteer-core/src/common/util.ts | 21 ++++++++++++++++++- test/src/page.spec.ts | 8 +++++++ 6 files changed, 48 insertions(+), 31 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 9ece15fe..549d58ae 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -200,6 +200,12 @@ module.exports = { selector: 'MemberExpression[object.name="Promise"][property.name="race"]', }, + { + message: + 'Deferred `valueOrThrow` should not be called in `Deferred.race()` pass deferred directly', + selector: + 'CallExpression[callee.object.name="Deferred"][callee.property.name="race"] > ArrayExpression > CallExpression[callee.property.name="valueOrThrow"]', + }, ], '@typescript-eslint/no-floating-promises': [ 'error', diff --git a/packages/puppeteer-core/src/api/Locator.ts b/packages/puppeteer-core/src/api/Locator.ts index 207c1bde..cc0bb7e9 100644 --- a/packages/puppeteer-core/src/api/Locator.ts +++ b/packages/puppeteer-core/src/api/Locator.ts @@ -22,7 +22,10 @@ import {isErrorLike} from '../util/ErrorLike.js'; import {ElementHandle, BoundingBox, ClickOptions} from './ElementHandle.js'; import type {Page} from './Page.js'; -type VisibilityOption = 'hidden' | 'visible' | null; +/** + * @internal + */ +export type VisibilityOption = 'hidden' | 'visible' | null; /** * @internal diff --git a/packages/puppeteer-core/src/common/IsolatedWorld.ts b/packages/puppeteer-core/src/common/IsolatedWorld.ts index 34610845..02c2d7d1 100644 --- a/packages/puppeteer-core/src/common/IsolatedWorld.ts +++ b/packages/puppeteer-core/src/common/IsolatedWorld.ts @@ -41,6 +41,7 @@ import { addPageBinding, createJSHandle, debugError, + getPageContent, setPageContent, withSourcePuppeteerURLIfNone, } from './util.js'; @@ -272,16 +273,7 @@ export class IsolatedWorld { } async content(): Promise { - return await this.evaluate(() => { - let retVal = ''; - if (document.doctype) { - retVal = new XMLSerializer().serializeToString(document.doctype); - } - if (document.documentElement) { - retVal += document.documentElement.outerHTML; - } - return retVal; - }); + return await this.evaluate(getPageContent); } async setContent( @@ -533,12 +525,9 @@ class Mutex { this.#locked = true; return Promise.resolve(); } - let resolve!: () => void; - const promise = new Promise(res => { - resolve = res; - }); - this.#acquirers.push(resolve); - return promise; + const deferred = Deferred.create(); + this.#acquirers.push(deferred.resolve.bind(deferred)); + return deferred.valueOrThrow(); } release(): void { diff --git a/packages/puppeteer-core/src/common/bidi/BrowsingContext.ts b/packages/puppeteer-core/src/common/bidi/BrowsingContext.ts index 1c45ab61..eda96a6c 100644 --- a/packages/puppeteer-core/src/common/bidi/BrowsingContext.ts +++ b/packages/puppeteer-core/src/common/bidi/BrowsingContext.ts @@ -11,6 +11,7 @@ import {TimeoutSettings} from '../TimeoutSettings.js'; import {EvaluateFunc, HandleFor} from '../types.js'; import { PuppeteerURL, + getPageContent, getSourcePuppeteerURLIfAvailable, isString, setPageContent, @@ -240,7 +241,7 @@ export class BrowsingContext extends EventEmitter { timeout = this.#timeoutSettings.navigationTimeout(), } = options; - const waitUntilCommand = lifeCycleToSubscribedEvent.get( + const waitUntilEvent = lifeCycleToSubscribedEvent.get( getWaitUntilSingle(waitUntil) ) as string; @@ -248,27 +249,18 @@ export class BrowsingContext extends EventEmitter { setPageContent(this, html), waitWithTimeout( new Promise(resolve => { - this.once(waitUntilCommand, () => { + this.once(waitUntilEvent, () => { resolve(); }); }), - waitUntilCommand, + waitUntilEvent, timeout ), ]); } async content(): Promise { - return await this.evaluate(() => { - let retVal = ''; - if (document.doctype) { - retVal = new XMLSerializer().serializeToString(document.doctype); - } - if (document.documentElement) { - retVal += document.documentElement.outerHTML; - } - return retVal; - }); + return await this.evaluate(getPageContent); } async sendCDPCommand( diff --git a/packages/puppeteer-core/src/common/util.ts b/packages/puppeteer-core/src/common/util.ts index 9ed8f729..71922281 100644 --- a/packages/puppeteer-core/src/common/util.ts +++ b/packages/puppeteer-core/src/common/util.ts @@ -510,7 +510,7 @@ export async function waitWithTimeout( timeout, }); - return await Deferred.race([promise, deferred.valueOrThrow()]).finally(() => { + return await Deferred.race([promise, deferred]).finally(() => { deferred.reject(new Error('Cleared')); }); } @@ -627,3 +627,22 @@ export async function setPageContent( document.close(); }, content); } + +/** + * @internal + */ +export function getPageContent(): string { + let content = ''; + for (const node of document.childNodes) { + switch (node) { + case document.documentElement: + content += document.documentElement.outerHTML; + break; + default: + content += new XMLSerializer().serializeToString(node); + break; + } + } + + return content; +} diff --git a/test/src/page.spec.ts b/test/src/page.spec.ts index 48889900..18606ccc 100644 --- a/test/src/page.spec.ts +++ b/test/src/page.spec.ts @@ -1555,6 +1555,14 @@ describe('Page', function () { }) ).toBe('\n'); }); + it('should work with comments outside HTML tag', async () => { + const {page} = getTestState(); + + const comment = ''; + await page.setContent(`${comment}
hello
`); + const result = await page.content(); + expect(result).toBe(`${comment}${expectedOutput}`); + }); }); describe('Page.setBypassCSP', function () {