feat(dom): migrate innerText, innerHTML and getAttribute to tasks (#2782)

This ensures synchronous access to avoid element recycling.

Author: dgozman

src/dom.ts

@@ -734,7 +734,7 @@ export function toFileTransferPayload(files: types.FilePayload[]): types.FileTra
   }));
 }
 
-function throwFatalDOMError<T>(result: T | FatalDOMError): T {
+export function throwFatalDOMError<T>(result: T | FatalDOMError): T {
   if (result === 'error:notelement')
     throw new Error('Node is not an element');
   if (result === 'error:nothtmlelement')
@@ -807,9 +807,10 @@ export function dispatchEventTask(selector: SelectorInfo, type: string, eventIni
   return injectedScript => injectedScript.evaluateHandle((injected, { parsed, type, eventInit }) => {
     return injected.pollRaf((progress, continuePolling) => {
       const element = injected.querySelector(parsed, document);
-      if (element)
-        injected.dispatchEvent(element, type, eventInit);
-      return element ? undefined : continuePolling;
+      if (!element)
+        return continuePolling;
+      progress.log(`  selector resolved to ${injected.previewNode(element)}`);
+      injected.dispatchEvent(element, type, eventInit);
     });
   }, { parsed: selector.parsed, type, eventInit });
 }
@@ -818,7 +819,48 @@ export function textContentTask(selector: SelectorInfo): SchedulableTask<string
   return injectedScript => injectedScript.evaluateHandle((injected, parsed) => {
     return injected.pollRaf((progress, continuePolling) => {
       const element = injected.querySelector(parsed, document);
-      return element ? element.textContent : continuePolling;
+      if (!element)
+        return continuePolling;
+      progress.log(`  selector resolved to ${injected.previewNode(element)}`);
+      return element.textContent;
     });
   }, selector.parsed);
 }
+
+export function innerTextTask(selector: SelectorInfo): SchedulableTask<'error:nothtmlelement' | { innerText: string }> {
+  return injectedScript => injectedScript.evaluateHandle((injected, parsed) => {
+    return injected.pollRaf((progress, continuePolling) => {
+      const element = injected.querySelector(parsed, document);
+      if (!element)
+        return continuePolling;
+      progress.log(`  selector resolved to ${injected.previewNode(element)}`);
+      if (element.namespaceURI !== 'http://www.w3.org/1999/xhtml')
+        return 'error:nothtmlelement';
+      return { innerText: (element as HTMLElement).innerText };
+    });
+  }, selector.parsed);
+}
+
+export function innerHTMLTask(selector: SelectorInfo): SchedulableTask<string> {
+  return injectedScript => injectedScript.evaluateHandle((injected, parsed) => {
+    return injected.pollRaf((progress, continuePolling) => {
+      const element = injected.querySelector(parsed, document);
+      if (!element)
+        return continuePolling;
+      progress.log(`  selector resolved to ${injected.previewNode(element)}`);
+      return element.innerHTML;
+    });
+  }, selector.parsed);
+}
+
+export function getAttributeTask(selector: SelectorInfo, name: string): SchedulableTask<string | null> {
+  return injectedScript => injectedScript.evaluateHandle((injected, { parsed, name }) => {
+    return injected.pollRaf((progress, continuePolling) => {
+      const element = injected.querySelector(parsed, document);
+      if (!element)
+        return continuePolling;
+      progress.log(`  selector resolved to ${injected.previewNode(element)}`);
+      return element.getAttribute(name);
+    });
+  }, { parsed: selector.parsed, name });
+}

src/frames.ts

@@ -759,21 +759,37 @@ export class Frame {
     const info = selectors._parseSelector(selector);
     const task = dom.textContentTask(info);
     return this._page._runAbortableTask(async progress => {
-      progress.logger.info(`Retrieving text context from "${selector}"...`);
+      progress.logger.info(`  retrieving textContent from "${selector}"`);
       return this._scheduleRerunnableTask(progress, info.world, task);
     }, this._page._timeoutSettings.timeout(options), this._apiName('textContent'));
   }
 
   async innerText(selector: string, options: types.TimeoutOptions = {}): Promise<string> {
-    return await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle.innerText(), this._apiName('innerText'));
+    const info = selectors._parseSelector(selector);
+    const task = dom.innerTextTask(info);
+    return this._page._runAbortableTask(async progress => {
+      progress.logger.info(`  retrieving innerText from "${selector}"`);
+      const result = dom.throwFatalDOMError(await this._scheduleRerunnableTask(progress, info.world, task));
+      return result.innerText;
+    }, this._page._timeoutSettings.timeout(options), this._apiName('innerText'));
   }
 
   async innerHTML(selector: string, options: types.TimeoutOptions = {}): Promise<string> {
-    return await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle.innerHTML(), this._apiName('innerHTML'));
+    const info = selectors._parseSelector(selector);
+    const task = dom.innerHTMLTask(info);
+    return this._page._runAbortableTask(async progress => {
+      progress.logger.info(`  retrieving innerHTML from "${selector}"`);
+      return this._scheduleRerunnableTask(progress, info.world, task);
+    }, this._page._timeoutSettings.timeout(options), this._apiName('innerHTML'));
   }
 
   async getAttribute(selector: string, name: string, options: types.TimeoutOptions = {}): Promise<string | null> {
-    return await this._retryWithSelectorIfNotConnected(selector, options, (progress, handle) => handle.getAttribute(name), this._apiName('getAttribute'));
+    const info = selectors._parseSelector(selector);
+    const task = dom.getAttributeTask(info, name);
+    return this._page._runAbortableTask(async progress => {
+      progress.logger.info(`  retrieving attribute "${name}" from "${selector}"`);
+      return this._scheduleRerunnableTask(progress, info.world, task);
+    }, this._page._timeoutSettings.timeout(options), this._apiName('getAttribute'));
   }
 
   async hover(selector: string, options: types.PointerActionOptions & types.PointerActionWaitOptions = {}) {

test/elementhandle.spec.js

@@ -470,6 +470,14 @@ describe('ElementHandle convenience API', function() {
     expect(await handle.innerText()).toBe('Text, more text');
     expect(await page.innerText('#inner')).toBe('Text, more text');
   });
+  it('innerText should throw', async({page, server}) => {
+    await page.setContent(`<svg>text</svg>`);
+    const error1 = await page.innerText('svg').catch(e => e);
+    expect(error1.message).toContain('Not an HTMLElement');
+    const handle = await page.$('svg');
+    const error2 = await handle.innerText().catch(e => e);
+    expect(error2.message).toContain('Not an HTMLElement');
+  });
   it('textContent should work', async({page, server}) => {
     await page.goto(`${server.PREFIX}/dom.html`);
     const handle = await page.$('#inner');
@@ -498,6 +506,72 @@ describe('ElementHandle convenience API', function() {
     expect(tc).toBe('Hello');
     expect(await page.evaluate(() => document.querySelector('div').textContent)).toBe('modified');
   });
+  it('innerText should be atomic', async({page}) => {
+    const createDummySelector = () => ({
+      create(root, target) {},
+      query(root, selector) {
+        const result = root.querySelector(selector);
+        if (result)
+          Promise.resolve().then(() => result.textContent = 'modified');
+        return result;
+      },
+      queryAll(root, selector) {
+        const result = Array.from(root.querySelectorAll(selector));
+        for (const e of result)
+          Promise.resolve().then(() => result.textContent = 'modified');
+        return result;
+      }
+    });
+    await utils.registerEngine('innerText', createDummySelector);
+    await page.setContent(`<div>Hello</div>`);
+    const tc = await page.innerText('innerText=div');
+    expect(tc).toBe('Hello');
+    expect(await page.evaluate(() => document.querySelector('div').innerText)).toBe('modified');
+  });
+  it('innerHTML should be atomic', async({page}) => {
+    const createDummySelector = () => ({
+      create(root, target) {},
+      query(root, selector) {
+        const result = root.querySelector(selector);
+        if (result)
+          Promise.resolve().then(() => result.textContent = 'modified');
+        return result;
+      },
+      queryAll(root, selector) {
+        const result = Array.from(root.querySelectorAll(selector));
+        for (const e of result)
+          Promise.resolve().then(() => result.textContent = 'modified');
+        return result;
+      }
+    });
+    await utils.registerEngine('innerHTML', createDummySelector);
+    await page.setContent(`<div>Hello<span>world</span></div>`);
+    const tc = await page.innerHTML('innerHTML=div');
+    expect(tc).toBe('Hello<span>world</span>');
+    expect(await page.evaluate(() => document.querySelector('div').innerHTML)).toBe('modified');
+  });
+  it('getAttribute should be atomic', async({page}) => {
+    const createDummySelector = () => ({
+      create(root, target) {},
+      query(root, selector) {
+        const result = root.querySelector(selector);
+        if (result)
+          Promise.resolve().then(() => result.setAttribute('foo', 'modified'));
+        return result;
+      },
+      queryAll(root, selector) {
+        const result = Array.from(root.querySelectorAll(selector));
+        for (const e of result)
+          Promise.resolve().then(() => result.setAttribute('foo', 'modified'));
+        return result;
+      }
+    });
+    await utils.registerEngine('getAttribute', createDummySelector);
+    await page.setContent(`<div foo=hello></div>`);
+    const tc = await page.getAttribute('getAttribute=div', 'foo');
+    expect(tc).toBe('hello');
+    expect(await page.evaluate(() => document.querySelector('div').getAttribute('foo'))).toBe('modified');
+  });
 });
 
 describe('ElementHandle.check', () => {