chore: remove old TODOs, add a test (#1879)

src/dom.ts

@@ -103,6 +103,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     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<T extends Node = Node> extends js.JSHandle<T> {
     return this._page._screenshotter.screenshotElement(this, options);
   }
 
-  $(selector: string): Promise<ElementHandle | null> {
-    // TODO: this should be ownerFrame() instead.
+  async $(selector: string): Promise<ElementHandle | null> {
     return selectors._query(this._context.frame, selector, this);
   }
 
-  $$(selector: string): Promise<ElementHandle<Element>[]> {
-    // TODO: this should be ownerFrame() instead.
+  async $$(selector: string): Promise<ElementHandle<Element>[]> {
     return selectors._queryAll(this._context.frame, selector, this);
   }
 
   async $eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element, Arg, R>, arg: Arg): Promise<R>;
   async $eval<R>(selector: string, pageFunction: types.FuncOn<Element, void, R>, arg?: any): Promise<R>;
   async $eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element, Arg, R>, arg: Arg): Promise<R> {
-    // 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<T extends Node = Node> extends js.JSHandle<T> {
   async $$eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element[], Arg, R>, arg: Arg): Promise<R>;
   async $$eval<R>(selector: string, pageFunction: types.FuncOn<Element[], void, R>, arg?: any): Promise<R>;
   async $$eval<R, Arg>(selector: string, pageFunction: types.FuncOn<Element[], Arg, R>, arg: Arg): Promise<R> {
-    // 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();

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 = {

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<void> {
-    // 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<Buffer> {
-    // 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,';

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('<a target=_blank rel=noopener href="/one-style.html">yo</a>');
     const [popup] = await Promise.all([

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('<html><body><div class="tweet"><div class="like">100</div><div class="retweets">10</div></div></body></html>');