From 52de75742b8518c9f0264784164ef47ef851434a Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Fri, 21 Jul 2017 00:58:38 -0700 Subject: [PATCH] Implement visible option for Page.waitFor method This patch adds a 'visible' option to the Page.waitFor method, making it possible to wait for the element to become actually visible. References #89, #91. --- docs/api.md | 42 +++++---- lib/FrameManager.js | 93 +++++++++++++------ lib/Page.js | 5 +- test/test.js | 10 ++ utils/doclint/JSBuilder.js | 12 ++- utils/doclint/test/04-bad-arguments/doc.md | 4 +- utils/doclint/test/04-bad-arguments/foo.js | 4 +- .../doclint/test/golden/04-bad-arguments.txt | 2 +- 8 files changed, 117 insertions(+), 55 deletions(-) diff --git a/docs/api.md b/docs/api.md index a40e19cbcd7..54735bd2144 100644 --- a/docs/api.md +++ b/docs/api.md @@ -59,7 +59,7 @@ + [page.url()](#pageurl) + [page.userAgent()](#pageuseragent) + [page.viewport()](#pageviewport) - + [page.waitFor(selector)](#pagewaitforselector) + + [page.waitFor(selector[, options])](#pagewaitforselector-options) + [page.waitForNavigation(options)](#pagewaitfornavigationoptions) * [class: Keyboard](#class-keyboard) + [keyboard.down(key[, options])](#keyboarddownkey-options) @@ -83,7 +83,7 @@ + [frame.name()](#framename) + [frame.parentFrame()](#frameparentframe) + [frame.url()](#frameurl) - + [frame.waitFor(selector)](#framewaitforselector) + + [frame.waitFor(selector[, options])](#framewaitforselector-options) * [class: Request](#class-request) + [request.headers](#requestheaders) + [request.method](#requestmethod) @@ -593,30 +593,18 @@ This is a shortcut for [page.mainFrame().url()](#frameurl) - returns: <[Object]> An object with the save fields as described in [page.setViewport](#pagesetviewportviewport) -#### page.waitFor(selector) +#### page.waitFor(selector[, options]) - `selector` <[string]> A query selector to wait for on the page. +- `options` <[Object]> Optional waiting parameters + - `visible` <[boolean]> wait for element to be present in DOM and to be visible, i.e. to not have `display: none` or `visibility: hidden` CSS properties. - returns: <[Promise]> Promise which resolves when the element matching `selector` appears in the page. -The `page.waitFor` successfully survives page navigations: -```js -const {Browser} = new require('puppeteer'); -const browser = new Browser(); - -browser.newPage().then(async page => { - let currentURL; - page.waitFor('img').then(() => console.log('First URL with image: ' + currentURL)); - for (currentURL of ['https://example.com', 'https://google.com', 'https://bbc.com']) - await page.navigate(currentURL); - browser.close(); -}); -``` +Shortcut for [page.mainFrame().waitFor(selector)](#framewaitforselector). #### page.waitForNavigation(options) - `options` <[Object]> Navigation parameters, same as in [page.navigate](#pagenavigateurl-options). - returns: <[Promise]<[Response]>> Promise which resolves to the main resource response. In case of multiple redirects, the navigation will resolve with the response of the last redirect. -Shortcut for [page.mainFrame().waitFor(selector)](#framewaitforselector). - ### class: Keyboard Keyboard provides an api for managing a virtual keyboard. The high level api is [`keyboard.type`](#keyboardtypetext), which takes raw characters and generates proper keydown, keypress/input, and keyup events on your page. @@ -813,14 +801,30 @@ Returns frame's name as specified in the tag. Returns frame's url. -#### frame.waitFor(selector) +#### frame.waitFor(selector[, options]) - `selector` <[string]> CSS selector of awaited element, +- `options` <[Object]> Optional waiting parameters + - `visible` <[boolean]> wait for element to be present in DOM and to be visible, i.e. to not have `display: none` or `visibility: hidden` CSS properties. - returns: <[Promise]> Promise which resolves when element specified by selector string is added to DOM. Wait for the `selector` to appear in page. If at the moment of calling the method the `selector` already exists, the method will return immediately. +This method works across navigations: +```js +const {Browser} = new require('puppeteer'); +const browser = new Browser(); + +browser.newPage().then(async page => { + let currentURL; + page.waitFor('img').then(() => console.log('First URL with image: ' + currentURL)); + for (currentURL of ['https://example.com', 'https://google.com', 'https://bbc.com']) + await page.navigate(currentURL); + browser.close(); +}); +``` + ### class: Request diff --git a/lib/FrameManager.js b/lib/FrameManager.js index b12f300e18b..868a5e5d4e0 100644 --- a/lib/FrameManager.js +++ b/lib/FrameManager.js @@ -173,40 +173,19 @@ class FrameManager extends EventEmitter { } /** - * @param {string} selector * @param {!Frame} frame + * @param {string} selector + * @param {boolean} waitForVisible * @return {!Promise} */ - async _waitForSelector(selector, frame) { - - function code(selector) { - if (document.querySelector(selector)) - return Promise.resolve(); - - let callback; - const result = new Promise(fulfill => callback = fulfill); - - const mo = new MutationObserver((mutations, observer) => { - if (document.querySelector(selector)) { - observer.disconnect(); - callback(); - return; - } - }); - mo.observe(document, { - childList: true, - subtree: true - }); - return result; - } - + async _waitForSelector(frame, selector, waitForVisible) { let contextId = undefined; if (!frame.isMainFrame()) { contextId = this._frameIdToExecutionContextId.get(frame._id); console.assert(contextId, 'Frame does not have default context to evaluate in!'); } let { exceptionDetails } = await this._client.send('Runtime.evaluate', { - expression: helper.evaluationString(code, selector), + expression: helper.evaluationString(inPageWatchdog, selector, waitForVisible), contextId, awaitPromise: true, returnByValue: false, @@ -215,6 +194,65 @@ class FrameManager extends EventEmitter { let message = await helper.getExceptionMessage(this._client, exceptionDetails); throw new Error('Evaluation failed: ' + message); } + + /** + * @param {string} selector + * @param {boolean} waitForVisible + * @return {!Promise} + */ + function inPageWatchdog(selector, visible) { + return visible ? waitForVisible(selector) : waitInDOM(selector); + + /** + * @param {string} selector + * @return {!Promise} + */ + function waitInDOM(selector) { + let node = document.querySelector(selector); + if (node) + return Promise.resolve(node); + + let fulfill; + const result = new Promise(x => fulfill = x); + const observer = new MutationObserver(mutations => { + const node = document.querySelector(selector); + if (node) { + observer.disconnect(); + fulfill(node); + } + }); + observer.observe(document, { + childList: true, + subtree: true + }); + return result; + } + + /** + * @param {string} selector + * @return {!Promise} + */ + async function waitForVisible(selector) { + let fulfill; + const result = new Promise(x => fulfill = x); + onRaf(); + return result; + + async function onRaf() { + const node = await waitInDOM(selector); + if (!node) { + fulfill(null); + return; + } + const style = window.getComputedStyle(node); + if (style.display === 'none' || style.visibility === 'hidden') { + requestAnimationFrame(onRaf); + return; + } + fulfill(node); + } + } + } } } @@ -304,10 +342,11 @@ class Frame { /** * @param {string} selector + * @param {!Object=} options * @return {!Promise} */ - async waitFor(selector) { - const awaitedElement = new AwaitedElement(() => this._frameManager._waitForSelector(selector, this)); + async waitFor(selector, options = {}) { + const awaitedElement = new AwaitedElement(() => this._frameManager._waitForSelector(this, selector, !!options.visible)); this._awaitedElements.add(awaitedElement); let cleanup = () => this._awaitedElements.delete(awaitedElement); diff --git a/lib/Page.js b/lib/Page.js index 133c8e2bafc..71ce46fda2b 100644 --- a/lib/Page.js +++ b/lib/Page.js @@ -598,10 +598,11 @@ class Page extends EventEmitter { /** * @param {string} selector + * @param {!Object=} options * @return {!Promise} */ - waitFor(selector) { - return this.mainFrame().waitFor(selector); + waitFor(selector, options) { + return this.mainFrame().waitFor(selector, options); } /** diff --git a/test/test.js b/test/test.js index fbb575b6f86..811a75d23d7 100644 --- a/test/test.js +++ b/test/test.js @@ -264,6 +264,16 @@ describe('Puppeteer', function() { await waitFor; expect(boxFound).toBe(true); })); + it('should wait for visible', SX(async function() { + let divFound = false; + let waitFor = page.waitFor('div', {visible: true}).then(() => divFound = true); + await page.setContent(`
`); + expect(divFound).toBe(false); + await page.evaluate(() => document.querySelector('div').style.removeProperty('display')); + expect(divFound).toBe(false); + await page.evaluate(() => document.querySelector('div').style.removeProperty('visibility')); + expect(await waitFor).toBe(true); + })); }); describe('Page.Events.Console', function() { diff --git a/utils/doclint/JSBuilder.js b/utils/doclint/JSBuilder.js index 464725f02a0..b0fd391733e 100644 --- a/utils/doclint/JSBuilder.js +++ b/utils/doclint/JSBuilder.js @@ -66,8 +66,16 @@ class JSOutline { walker.walk(node.value.body); } const args = []; - for (let param of node.value.params) - args.push(new Documentation.Argument(this._extractText(param))); + for (let param of node.value.params) { + if (param.type === 'AssignmentPattern') + args.push(new Documentation.Argument(param.left.name)); + else if (param.type === 'RestElement') + args.push(new Documentation.Argument('...' + param.argument.name)); + else if (param.type === 'Identifier') + args.push(new Documentation.Argument(param.name)); + else + this.errors.push('JS Parsing issue: cannot support parameter of type ' + param.type + ' in method ' + methodName); + } let method = Documentation.Member.createMethod(methodName, args, hasReturn, node.value.async); this._currentClassMembers.push(method); return ESTreeWalker.SkipSubtree; diff --git a/utils/doclint/test/04-bad-arguments/doc.md b/utils/doclint/test/04-bad-arguments/doc.md index 2e4e7c645d5..b1e19eeead8 100644 --- a/utils/doclint/test/04-bad-arguments/doc.md +++ b/utils/doclint/test/04-bad-arguments/doc.md @@ -3,5 +3,5 @@ - `arg1` <[string]> - `arg2` <[string]> -#### foo.test(fileNames) -- `filePaths` <[Array]> +#### foo.test(...files) +- `...filePaths` <[string]> diff --git a/utils/doclint/test/04-bad-arguments/foo.js b/utils/doclint/test/04-bad-arguments/foo.js index 6538ac8e513..72642e5fce9 100644 --- a/utils/doclint/test/04-bad-arguments/foo.js +++ b/utils/doclint/test/04-bad-arguments/foo.js @@ -1,7 +1,7 @@ class Foo { - constructor(arg1, arg3) { + constructor(arg1, arg3 = {}) { } - test(filePaths) { + test(...filePaths) { } } diff --git a/utils/doclint/test/golden/04-bad-arguments.txt b/utils/doclint/test/golden/04-bad-arguments.txt index 2b03dbcea04..bf6fc6ea569 100644 --- a/utils/doclint/test/golden/04-bad-arguments.txt +++ b/utils/doclint/test/golden/04-bad-arguments.txt @@ -1,4 +1,4 @@ [MarkDown] Method Foo.constructor() fails to describe its parameters: - Argument not found: arg3 - Non-existing argument found: arg2 -[MarkDown] Heading arguments for "foo.test(fileNames)" do not match described ones, i.e. "fileNames" != "filePaths" \ No newline at end of file +[MarkDown] Heading arguments for "foo.test(...files)" do not match described ones, i.e. "...files" != "...filePaths" \ No newline at end of file