From d1a95518be86ceaf9d1a840f9db5caab45f39fa4 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 20 Apr 2020 13:01:06 -0700 Subject: [PATCH] chore: remove old TODOs, add a test (#1879) --- src/dom.ts | 11 +++++------ src/firefox/ffBrowser.ts | 2 +- src/webkit/wkPage.ts | 3 --- test/popup.spec.js | 1 - test/queryselector.spec.js | 26 ++++++++++++++++++++++++++ 5 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/dom.ts b/src/dom.ts index 8ce223af24..e7e9eee9d5 100644 --- a/src/dom.ts +++ b/src/dom.ts @@ -103,6 +103,9 @@ export class ElementHandle extends js.JSHandle { const frameId = await this._page._delegate.getOwnerFrame(this); if (!frameId) return null; + const frame = this._page._frameManager.frame(frameId); + if (frame) + return frame; for (const page of this._page._browserContext.pages()) { const frame = page._frameManager.frame(frameId); if (frame) @@ -378,20 +381,17 @@ export class ElementHandle extends js.JSHandle { return this._page._screenshotter.screenshotElement(this, options); } - $(selector: string): Promise { - // TODO: this should be ownerFrame() instead. + async $(selector: string): Promise { return selectors._query(this._context.frame, selector, this); } - $$(selector: string): Promise[]> { - // TODO: this should be ownerFrame() instead. + async $$(selector: string): Promise[]> { return selectors._queryAll(this._context.frame, selector, this); } async $eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise; async $eval(selector: string, pageFunction: types.FuncOn, arg?: any): Promise; async $eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise { - // TODO: this should be ownerFrame() instead. const handle = await selectors._query(this._context.frame, selector, this); if (!handle) throw new Error(`Error: failed to find element matching selector "${selector}"`); @@ -403,7 +403,6 @@ export class ElementHandle extends js.JSHandle { async $$eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise; async $$eval(selector: string, pageFunction: types.FuncOn, arg?: any): Promise; async $$eval(selector: string, pageFunction: types.FuncOn, arg: Arg): Promise { - // TODO: this should be ownerFrame() instead. const arrayHandle = await selectors._queryArray(this._context.frame, selector, this); const result = await arrayHandle.evaluate(pageFunction, arg); arrayHandle.dispose(); diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts index d4c32f68d2..54036adf42 100644 --- a/src/firefox/ffBrowser.ts +++ b/src/firefox/ffBrowser.ts @@ -75,7 +75,7 @@ export class FFBrowser extends BrowserBase { options = validateBrowserContextOptions(options); let viewport; if (options.viewport) { - // TODO: remove isMobile/hasTouch from the protocol? + // TODO: remove isMobile from the protocol? if (options.isMobile) throw new Error('options.isMobile is not supported in Firefox'); viewport = { diff --git a/src/webkit/wkPage.ts b/src/webkit/wkPage.ts index efee111bce..7738af4e43 100644 --- a/src/webkit/wkPage.ts +++ b/src/webkit/wkPage.ts @@ -655,13 +655,10 @@ export class WKPage implements PageDelegate { } async setBackgroundColor(color?: { r: number; g: number; b: number; a: number; }): Promise { - // TODO: line below crashes, sort it out. await this._session.send('Page.setDefaultBackgroundColorOverride', { color }); } async takeScreenshot(format: string, documentRect: types.Rect | undefined, viewportRect: types.Rect | undefined, quality: number | undefined): Promise { - // TODO: documentRect does not include pageScale, while backend considers it does. - // This brakes mobile screenshots of elements or full page. const rect = (documentRect || viewportRect)!; const result = await this._session.send('Page.snapshotRect', { ...rect, coordinateSystem: documentRect ? 'Page' : 'Viewport' }); const prefix = 'data:image/png;base64,'; diff --git a/test/popup.spec.js b/test/popup.spec.js index ab5f4e3af0..a5c3400151 100644 --- a/test/popup.spec.js +++ b/test/popup.spec.js @@ -344,7 +344,6 @@ describe('Page.Events.Popup', function() { it('should work with fake-clicking target=_blank and rel=noopener', async({browser, server}) => { const context = await browser.newContext(); const page = await context.newPage(); - // TODO: FFOX sends events for "one-style.html" request to both pages. await page.goto(server.EMPTY_PAGE); await page.setContent('yo'); const [popup] = await Promise.all([ diff --git a/test/queryselector.spec.js b/test/queryselector.spec.js index 4e9cc9fcfb..ecb1b035a8 100644 --- a/test/queryselector.spec.js +++ b/test/queryselector.spec.js @@ -238,7 +238,33 @@ describe('ElementHandle.$', function() { const second = await html.$('.third'); expect(second).toBe(null); }); + it('should work for adopted elements', async({page,server}) => { + await page.goto(server.EMPTY_PAGE); + const [popup] = await Promise.all([ + page.waitForEvent('popup'), + page.evaluate(url => window.__popup = window.open(url), server.EMPTY_PAGE), + ]); + const divHandle = await page.evaluateHandle(() => { + const div = document.createElement('div'); + document.body.appendChild(div); + const span = document.createElement('span'); + span.textContent = 'hello'; + div.appendChild(span); + return div; + }); + expect(await divHandle.$('span')).toBeTruthy(); + expect(await divHandle.$eval('span', e => e.textContent)).toBe('hello'); + + await popup.waitForLoadState('domcontentloaded'); + await page.evaluate(() => { + const div = document.querySelector('div'); + window.__popup.document.body.appendChild(div); + }); + expect(await divHandle.$('span')).toBeTruthy(); + expect(await divHandle.$eval('span', e => e.textContent)).toBe('hello'); + }); }); + describe('ElementHandle.$eval', function() { it('should work', async({page, server}) => { await page.setContent('
10
');