feat(actions): requery the element when it was detached during the action (#1853)

Author: dgozman

src/chromium/crPage.ts

@@ -36,6 +36,7 @@ import { CRPDF } from './crPdf';
 import { CRBrowserContext } from './crBrowser';
 import * as types from '../types';
 import { ConsoleMessage } from '../console';
+import { NotConnectedError } from '../errors';
 
 const UTILITY_WORLD_NAME = '__playwright_utility_world__';
 
@@ -765,6 +766,8 @@ class FrameSession {
       objectId: toRemoteObject(handle).objectId,
       rect,
     }).catch(e => {
+      if (e instanceof Error && e.message.includes('Node is detached from document'))
+        throw new NotConnectedError();
       if (e instanceof Error && e.message.includes('Node does not have a layout object'))
         e.message = 'Node is either not visible or not an HTMLElement';
       throw e;

src/dom.ts

@@ -14,19 +14,19 @@
  * limitations under the License.
  */
 
-import * as debug from 'debug';
 import * as fs from 'fs';
 import * as mime from 'mime';
 import * as path from 'path';
 import * as util from 'util';
 import * as frames from './frames';
-import { assert, debugError, helper } from './helper';
-import Injected from './injected/injected';
+import { assert, debugError, helper, debugInput } from './helper';
+import { Injected, InjectedResult } from './injected/injected';
 import * as input from './input';
 import * as js from './javascript';
 import { Page } from './page';
 import { selectors } from './selectors';
 import * as types from './types';
+import { NotConnectedError, TimeoutError } from './errors';
 
 export type PointerActionOptions = {
   modifiers?: input.Modifier[];
@@ -37,8 +37,6 @@ export type ClickOptions = PointerActionOptions & input.MouseClickOptions;
 
 export type MultiClickOptions = PointerActionOptions & input.MouseMultiClickOptions;
 
-const debugInput = debug('pw:input');
-
 export class FrameExecutionContext extends js.ExecutionContext {
   readonly frame: frames.Frame;
   private _injectedPromise?: Promise<js.JSHandle>;
@@ -220,10 +218,8 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     const position = options ? options.position : undefined;
     await this._scrollRectIntoViewIfNeeded(position ? { x: position.x, y: position.y, width: 0, height: 0 } : undefined);
     const point = position ? await this._offsetPoint(position) : await this._clickablePoint();
-
     point.x = (point.x * 100 | 0) / 100;
     point.y = (point.y * 100 | 0) / 100;
-
     if (!force)
       await this._waitForHitTargetAt(point, deadline);
 
@@ -270,18 +266,18 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
         assert(helper.isNumber(option.index), 'Indices must be numbers. Found index "' + option.index + '" of type "' + (typeof option.index) + '"');
     }
     return await this._page._frameManager.waitForSignalsCreatedBy<string[]>(async () => {
-      return this._evaluateInUtility(({ injected, node }, selectOptions) => injected.selectOptions(node, selectOptions), selectOptions);
+      const injectedResult = await this._evaluateInUtility(({ injected, node }, selectOptions) => injected.selectOptions(node, selectOptions), selectOptions);
+      return handleInjectedResult(injectedResult, '');
     }, deadline, options);
   }
 
   async fill(value: string, options?: types.NavigatingActionWaitOptions): Promise<void> {
     assert(helper.isString(value), 'Value must be string. Found value "' + value + '" of type "' + (typeof value) + '"');
     const deadline = this._page._timeoutSettings.computeDeadline(options);
     await this._page._frameManager.waitForSignalsCreatedBy(async () => {
-      const errorOrNeedsInput = await this._evaluateInUtility(({ injected, node }, value) => injected.fill(node, value), value);
-      if (typeof errorOrNeedsInput === 'string')
-        throw new Error(errorOrNeedsInput);
-      if (errorOrNeedsInput) {
+      const injectedResult = await this._evaluateInUtility(({ injected, node }, value) => injected.fill(node, value), value);
+      const needsInput = handleInjectedResult(injectedResult, '');
+      if (needsInput) {
         if (value)
           await this._page.keyboard.insertText(value);
         else
@@ -291,19 +287,21 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   }
 
   async selectText(): Promise<void> {
-    const error = await this._evaluateInUtility(({ injected, node }) => injected.selectText(node), {});
-    if (typeof error === 'string')
-      throw new Error(error);
+    const injectedResult = await this._evaluateInUtility(({ injected, node }) => injected.selectText(node), {});
+    handleInjectedResult(injectedResult, '');
   }
 
   async setInputFiles(files: string | types.FilePayload | string[] | types.FilePayload[], options?: types.NavigatingActionWaitOptions) {
     const deadline = this._page._timeoutSettings.computeDeadline(options);
-    const multiple = await this._evaluateInUtility(({ node }) => {
+    const injectedResult = await this._evaluateInUtility(({ node }): InjectedResult<boolean> => {
       if (node.nodeType !== Node.ELEMENT_NODE || (node as Node as Element).tagName !== 'INPUT')
-        throw new Error('Node is not an HTMLInputElement');
+        return { status: 'error', error: 'Node is not an HTMLInputElement' };
+      if (!node.isConnected)
+        return { status: 'notconnected' };
       const input = node as Node as HTMLInputElement;
-      return input.multiple;
+      return { status: 'success', value: input.multiple };
     }, {});
+    const multiple = handleInjectedResult(injectedResult, '');
     let ff: string[] | types.FilePayload[];
     if (!Array.isArray(files))
       ff = [ files ] as string[] | types.FilePayload[];
@@ -329,14 +327,8 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
   }
 
   async focus() {
-    const errorMessage = await this._evaluateInUtility(({ node }) => {
-      if (!(node as any)['focus'])
-        return 'Node is not an HTML or SVG element.';
-      (node as Node as HTMLElement | SVGElement).focus();
-      return false;
-    }, {});
-    if (errorMessage)
-      throw new Error(errorMessage);
+    const injectedResult = await this._evaluateInUtility(({ injected, node }) => injected.focusNode(node), {});
+    handleInjectedResult(injectedResult, '');
   }
 
   async type(text: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) {
@@ -416,7 +408,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     const stablePromise = this._evaluateInUtility(({ injected, node }, timeout) => {
       return injected.waitForDisplayedAtStablePosition(node, timeout);
     }, helper.timeUntilDeadline(deadline));
-    await helper.waitWithDeadline(stablePromise, 'element to be displayed and not moving', deadline);
+    const timeoutMessage = 'element to be displayed and not moving';
+    const injectedResult = await helper.waitWithDeadline(stablePromise, timeoutMessage, deadline);
+    handleInjectedResult(injectedResult, timeoutMessage);
     debugInput('...done');
   }
 
@@ -434,7 +428,9 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     const hitTargetPromise = this._evaluateInUtility(({ injected, node }, { timeout, point }) => {
       return injected.waitForHitTargetAt(node, timeout, point);
     }, { timeout: helper.timeUntilDeadline(deadline), point });
-    await helper.waitWithDeadline(hitTargetPromise, 'element to receive pointer events', deadline);
+    const timeoutMessage = 'element to receive pointer events';
+    const injectedResult = await helper.waitWithDeadline(hitTargetPromise, timeoutMessage, deadline);
+    handleInjectedResult(injectedResult, timeoutMessage);
     debugInput('...done');
   }
 }
@@ -446,3 +442,13 @@ export function toFileTransferPayload(files: types.FilePayload[]): types.FileTra
     data: file.buffer.toString('base64')
   }));
 }
+
+function handleInjectedResult<T = undefined>(injectedResult: InjectedResult<T>, timeoutMessage: string): T {
+  if (injectedResult.status === 'notconnected')
+    throw new NotConnectedError();
+  if (injectedResult.status === 'timeout')
+    throw new TimeoutError(`waiting for ${timeoutMessage} failed: timeout exceeded`);
+  if (injectedResult.status === 'error')
+    throw new Error(injectedResult.error);
+  return injectedResult.value as T;
+}

src/errors.ts

@@ -23,4 +23,10 @@ class CustomError extends Error {
   }
 }
 
+export class NotConnectedError extends CustomError {
+  constructor() {
+    super('Element is not attached to the DOM');
+  }
+}
+
 export class TimeoutError extends CustomError {}

src/firefox/ffPage.ts

@@ -31,6 +31,7 @@ import { RawKeyboardImpl, RawMouseImpl } from './ffInput';
 import { FFNetworkManager, headersArray } from './ffNetworkManager';
 import { Protocol } from './protocol';
 import { selectors } from '../selectors';
+import { NotConnectedError } from '../errors';
 
 const UTILITY_WORLD_NAME = '__playwright_utility_world__';
 
@@ -422,6 +423,10 @@ export class FFPage implements PageDelegate {
       frameId: handle._context.frame._id,
       objectId: toRemoteObject(handle).objectId!,
       rect,
+    }).catch(e => {
+      if (e instanceof Error && e.message.includes('Node is detached from document'))
+        throw new NotConnectedError();
+      throw e;
     });
   }
 

src/frames.ts

@@ -19,9 +19,9 @@ import * as fs from 'fs';
 import * as util from 'util';
 import { ConsoleMessage } from './console';
 import * as dom from './dom';
-import { TimeoutError } from './errors';
+import { TimeoutError, NotConnectedError } from './errors';
 import { Events } from './events';
-import { assert, helper, RegisteredListener } from './helper';
+import { assert, helper, RegisteredListener, debugInput } from './helper';
 import * as js from './javascript';
 import * as network from './network';
 import { Page } from './page';
@@ -693,72 +693,82 @@ export class Frame {
     return result!;
   }
 
+  private async _retryWithSelectorIfNotConnected<R>(
+    selector: string, options: types.TimeoutOptions,
+    action: (handle: dom.ElementHandle<Element>, deadline: number) => Promise<R>): Promise<R> {
+    const deadline = this._page._timeoutSettings.computeDeadline(options);
+    while (!helper.isPastDeadline(deadline)) {
+      try {
+        const { world, task } = selectors._waitForSelectorTask(selector, 'attached', deadline);
+        const handle = await this._scheduleRerunnableTask(task, world, deadline, `selector "${selector}"`);
+        const element = handle.asElement() as dom.ElementHandle<Element>;
+        try {
+          return await action(element, deadline);
+        } finally {
+          element.dispose();
+        }
+      } catch (e) {
+        if (!(e instanceof NotConnectedError))
+          throw e;
+        debugInput('Element was detached from the DOM, retrying');
+      }
+    }
+    throw new TimeoutError(`waiting for selector "${selector}" failed: timeout exceeded`);
+  }
+
   async click(selector: string, options: dom.ClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.click(helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.click(helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
   async dblclick(selector: string, options: dom.MultiClickOptions & types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.dblclick(helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.dblclick(helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
   async fill(selector: string, value: string, options: types.NavigatingActionWaitOptions = {}) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.fill(value, helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.fill(value, helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
-  async focus(selector: string, options?: types.TimeoutOptions) {
-    const { handle } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.focus();
-    handle.dispose();
+  async focus(selector: string, options: types.TimeoutOptions = {}) {
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.focus());
   }
 
-  async hover(selector: string, options?: dom.PointerActionOptions & types.PointerActionWaitOptions) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.hover(helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+  async hover(selector: string, options: dom.PointerActionOptions & types.PointerActionWaitOptions = {}) {
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.hover(helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
-  async selectOption(selector: string, values: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[], options?: types.NavigatingActionWaitOptions): Promise<string[]> {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    const result = await handle.selectOption(values, helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
-    return result;
+  async selectOption(selector: string, values: string | dom.ElementHandle | types.SelectOption | string[] | dom.ElementHandle[] | types.SelectOption[], options: types.NavigatingActionWaitOptions = {}): Promise<string[]> {
+    return await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.selectOption(values, helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
-  async setInputFiles(selector: string, files: string | types.FilePayload | string[] | types.FilePayload[], options?: types.NavigatingActionWaitOptions): Promise<void> {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    const result = await handle.setInputFiles(files, helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
-    return result;
+  async setInputFiles(selector: string, files: string | types.FilePayload | string[] | types.FilePayload[], options: types.NavigatingActionWaitOptions = {}): Promise<void> {
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.setInputFiles(files, helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
-  async type(selector: string, text: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.type(text, helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+  async type(selector: string, text: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.type(text, helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
-  async press(selector: string, key: string, options?: { delay?: number } & types.NavigatingActionWaitOptions) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.press(key, helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+  async press(selector: string, key: string, options: { delay?: number } & types.NavigatingActionWaitOptions = {}) {
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.press(key, helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
-  async check(selector: string, options?: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.check(helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+  async check(selector: string, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.check(helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
-  async uncheck(selector: string, options?: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions) {
-    const { handle, deadline } = await this._waitForSelectorInUtilityContext(selector, options);
-    await handle.uncheck(helper.optionsWithUpdatedTimeout(options, deadline));
-    handle.dispose();
+  async uncheck(selector: string, options: types.PointerActionWaitOptions & types.NavigatingActionWaitOptions = {}) {
+    await this._retryWithSelectorIfNotConnected(selector, options,
+        (handle, deadline) => handle.uncheck(helper.optionsWithUpdatedTimeout(options, deadline)));
   }
 
   async waitFor(selectorOrFunctionOrTimeout: (string | number | Function), options: types.WaitForFunctionOptions & types.WaitForElementOptions = {}, arg?: any): Promise<js.JSHandle | null> {
@@ -773,14 +783,6 @@ export class Frame {
     return Promise.reject(new Error('Unsupported target type: ' + (typeof selectorOrFunctionOrTimeout)));
   }
 
-  private async _waitForSelectorInUtilityContext(selector: string, options?: types.WaitForElementOptions): Promise<{ handle: dom.ElementHandle<Element>, deadline: number }> {
-    const { waitFor = 'attached' } = options || {};
-    const deadline = this._page._timeoutSettings.computeDeadline(options);
-    const { world, task } = selectors._waitForSelectorTask(selector, waitFor, deadline);
-    const result = await this._scheduleRerunnableTask(task, world, deadline, `selector "${selectorToString(selector, waitFor)}"`);
-    return { handle: result.asElement() as dom.ElementHandle<Element>, deadline };
-  }
-
   async waitForFunction<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg, options?: types.WaitForFunctionOptions): Promise<types.SmartHandle<R>>;
   async waitForFunction<R>(pageFunction: types.Func1<void, R>, arg?: any, options?: types.WaitForFunctionOptions): Promise<types.SmartHandle<R>>;
   async waitForFunction<R, Arg>(pageFunction: types.Func1<Arg, R>, arg: Arg, options: types.WaitForFunctionOptions = {}): Promise<types.SmartHandle<R>> {

src/helper.ts

@@ -24,6 +24,7 @@ import { TimeoutError } from './errors';
 import * as types from './types';
 
 export const debugError = debug(`pw:error`);
+export const debugInput = debug('pw:input');
 
 export type RegisteredListener = {
   emitter: EventEmitter;
@@ -346,6 +347,10 @@ class Helper {
     return seconds * 1000 + (nanoseconds / 1000000 | 0);
   }
 
+  static isPastDeadline(deadline: number) {
+    return deadline !== Number.MAX_SAFE_INTEGER && this.monotonicTime() >= deadline;
+  }
+
   static timeUntilDeadline(deadline: number): number {
     return Math.min(deadline - this.monotonicTime(), 2147483647); // 2^31-1 safe setTimeout in Node.
   }