fix(dom): make selectOption wait for options (#5036)

Author: yury-s

docs/src/api/class-elementhandle.md

@@ -545,6 +545,8 @@ Returns the array of option values that have been successfully selected.
 Triggers a `change` and `input` event once all the provided options have been selected. If element is not a `<select>`
 element, the method throws an error.
 
+Will wait until all specified options are present in the `<select>` element.
+
 ```js
 // single selection matching the value
 handle.selectOption('blue');

docs/src/api/class-frame.md

@@ -842,6 +842,8 @@ Returns the array of option values that have been successfully selected.
 Triggers a `change` and `input` event once all the provided options have been selected. If there's no `<select>` element
 matching [`param: selector`], the method throws an error.
 
+Will wait until all specified options are present in the `<select>` element.
+
 ```js
 // single selection matching the value
 frame.selectOption('select#colors', 'blue');

docs/src/api/class-page.md

@@ -1869,6 +1869,8 @@ Returns the array of option values that have been successfully selected.
 Triggers a `change` and `input` event once all the provided options have been selected. If there's no `<select>` element
 matching [`param: selector`], the method throws an error.
 
+Will wait until all specified options are present in the `<select>` element.
+
 ```js
 // single selection matching the value
 page.selectOption('select#colors', 'blue');

src/server/dom.ts

@@ -446,9 +446,12 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     const selectOptions = [...elements, ...values];
     return this._page._frameManager.waitForSignalsCreatedBy(progress, options.noWaitAfter, async () => {
       progress.throwIfAborted();  // Avoid action that has side-effects.
-      const value = await this._evaluateInUtility(([injected, node, selectOptions]) => injected.selectOptions(node, selectOptions), selectOptions);
+      progress.log('  selecting specified option(s)');
+      const poll = await this._evaluateHandleInUtility(([injected, node, selectOptions]) => injected.waitForOptionsAndSelect(node, selectOptions), selectOptions);
+      const pollHandler = new InjectedScriptPollHandler(progress, poll);
+      const result = throwFatalDOMError(await pollHandler.finish());
       await this._page._doSlowMo();
-      return throwFatalDOMError(value);
+      return result;
     });
   }
 
@@ -817,7 +820,7 @@ export class InjectedScriptPollHandler<T> {
 
   async finishHandle(): Promise<js.SmartHandle<T>> {
     try {
-      const result = await this._poll!.evaluateHandle(poll => poll.result);
+      const result = await this._poll!.evaluateHandle(poll => poll.run());
       await this._finishInternal();
       return result;
     } finally {
@@ -827,7 +830,7 @@ export class InjectedScriptPollHandler<T> {
 
   async finish(): Promise<T> {
     try {
-      const result = await this._poll!.evaluate(poll => poll.result);
+      const result = await this._poll!.evaluate(poll => poll.run());
       await this._finishInternal();
       return result;
     } finally {

src/server/injected/injectedScript.ts

@@ -31,7 +31,7 @@ export type InjectedScriptProgress = {
 };
 
 export type InjectedScriptPoll<T> = {
-  result: Promise<T>,
+  run: () => Promise<T>,
   // Takes more logs, waiting until at least one message is available.
   takeNextLogs: () => Promise<string[]>,
   // Takes all current logs without waiting.
@@ -242,19 +242,23 @@ export class InjectedScript {
       },
     };
 
-    const result = task(progress);
+    const run = () => {
+      const result = task(progress);
 
-    // After the task has finished, there should be no more logs.
-    // Release any pending `takeNextLogs` call, and do not block any future ones.
-    // This prevents non-finished protocol evaluation calls and memory leaks.
-    result.finally(() => {
-      taskFinished = true;
-      logReady();
-    });
+      // After the task has finished, there should be no more logs.
+      // Release any pending `takeNextLogs` call, and do not block any future ones.
+      // This prevents non-finished protocol evaluation calls and memory leaks.
+      result.finally(() => {
+        taskFinished = true;
+        logReady();
+      });
+
+      return result;
+    };
 
     return {
       takeNextLogs,
-      result,
+      run,
       cancel: () => { progress.aborted = true; },
       takeLastLogs: () => unsentLogs,
     };
@@ -267,35 +271,52 @@ export class InjectedScript {
     return { left: parseInt(style.borderLeftWidth || '', 10), top: parseInt(style.borderTopWidth || '', 10) };
   }
 
-  selectOptions(node: Node, optionsToSelect: (Node | { value?: string, label?: string, index?: number })[]): string[] | 'error:notconnected' | FatalDOMError {
-    const element = this.findLabelTarget(node as Element);
-    if (!element || !element.isConnected)
-      return 'error:notconnected';
-    if (element.nodeName.toLowerCase() !== 'select')
-      return 'error:notselect';
-    const select = element as HTMLSelectElement;
-    const options = Array.from(select.options);
-    select.value = undefined as any;
-    for (let index = 0; index < options.length; index++) {
-      const option = options[index];
-      option.selected = optionsToSelect.some(optionToSelect => {
-        if (optionToSelect instanceof Node)
-          return option === optionToSelect;
-        let matches = true;
-        if (optionToSelect.value !== undefined)
-          matches = matches && optionToSelect.value === option.value;
-        if (optionToSelect.label !== undefined)
-          matches = matches && optionToSelect.label === option.label;
-        if (optionToSelect.index !== undefined)
-          matches = matches && optionToSelect.index === index;
-        return matches;
-      });
-      if (option.selected && !select.multiple)
-        break;
-    }
-    select.dispatchEvent(new Event('input', { 'bubbles': true }));
-    select.dispatchEvent(new Event('change', { 'bubbles': true }));
-    return options.filter(option => option.selected).map(option => option.value);
+  waitForOptionsAndSelect(node: Node, optionsToSelect: (Node | { value?: string, label?: string, index?: number })[]): InjectedScriptPoll<string[] | 'error:notconnected' | FatalDOMError> {
+    return this.pollRaf((progress, continuePolling) => {
+      const element = this.findLabelTarget(node as Element);
+      if (!element || !element.isConnected)
+        return 'error:notconnected';
+      if (element.nodeName.toLowerCase() !== 'select')
+        return 'error:notselect';
+      const select = element as HTMLSelectElement;
+      const options = Array.from(select.options);
+      const selectedOptions = [];
+      let remainingOptionsToSelect = optionsToSelect.slice();
+      for (let index = 0; index < options.length; index++) {
+        const option = options[index];
+        const filter = (optionToSelect: Node | { value?: string, label?: string, index?: number }) => {
+          if (optionToSelect instanceof Node)
+            return option === optionToSelect;
+          let matches = true;
+          if (optionToSelect.value !== undefined)
+            matches = matches && optionToSelect.value === option.value;
+          if (optionToSelect.label !== undefined)
+            matches = matches && optionToSelect.label === option.label;
+          if (optionToSelect.index !== undefined)
+            matches = matches && optionToSelect.index === index;
+          return matches;
+        };
+        if (!remainingOptionsToSelect.some(filter))
+          continue;
+        selectedOptions.push(option);
+        if (select.multiple) {
+          remainingOptionsToSelect = remainingOptionsToSelect.filter(o => !filter(o));
+        } else {
+          remainingOptionsToSelect = [];
+          break;
+        }
+      }
+      if (remainingOptionsToSelect.length) {
+        progress.logRepeating('    did not find some options - waiting... ');
+        return continuePolling;
+      }
+      select.value = undefined as any;
+      selectedOptions.forEach(option => option.selected = true);
+      progress.log('    selected specified option(s)');
+      select.dispatchEvent(new Event('input', { 'bubbles': true }));
+      select.dispatchEvent(new Event('change', { 'bubbles': true }));
+      return selectedOptions.map(option => option.value);
+    });
   }
 
   waitForEnabledAndFill(node: Node, value: string): InjectedScriptPoll<FatalDOMError | 'error:notconnected' | 'needsinput' | 'done'> {

test/page-fill.spec.ts

@@ -288,3 +288,14 @@ it('should be able to clear', async ({page, server}) => {
   await page.fill('input', '');
   expect(await page.evaluate(() => window['result'])).toBe('');
 });
+
+it('should not throw when fill causes navigation', async ({page, server}) => {
+  await page.goto(server.PREFIX + '/input/textarea.html');
+  await page.setContent('<input type=date>');
+  await page.$eval('input', select => select.addEventListener('input', () => window.location.href = '/empty.html'));
+  await Promise.all([
+    page.fill('input', '2020-03-02'),
+    page.waitForNavigation(),
+  ]);
+  expect(page.url()).toContain('empty.html');
+});

test/page-select-option.spec.ts

@@ -17,6 +17,12 @@
 
 import { it, expect } from './fixtures';
 
+
+async function giveItAChanceToResolve(page) {
+  for (let i = 0; i < 5; i++)
+    await page.evaluate(() => new Promise(f => requestAnimationFrame(() => requestAnimationFrame(f))));
+}
+
 it('should select single option', async ({page, server}) => {
   await page.goto(server.PREFIX + '/input/select.html');
   await page.selectOption('select', 'blue');
@@ -61,7 +67,12 @@ it('should select single option by multiple attributes', async ({page, server})
 
 it('should not select single option when some attributes do not match', async ({page, server}) => {
   await page.goto(server.PREFIX + '/input/select.html');
-  await page.selectOption('select', { value: 'green', label: 'Brown' });
+  await page.$eval('select', s => s.value = undefined);
+  try {
+    await page.selectOption('select', { value: 'green', label: 'Brown' }, {timeout: 300});
+  } catch (e) {
+    expect(e.message).toContain('Timeout');
+  }
   expect(await page.evaluate(() => document.querySelector('select').value)).toEqual('');
 });
 
@@ -134,7 +145,7 @@ it('should throw when element is not a <select>', async ({page, server}) => {
 
 it('should return [] on no matched values', async ({page, server}) => {
   await page.goto(server.PREFIX + '/input/select.html');
-  const result = await page.selectOption('select', ['42','abc']);
+  const result = await page.selectOption('select', []);
   expect(result).toEqual([]);
 });
 
@@ -237,3 +248,56 @@ it('should work when re-defining top-level Event class', async ({page, server})
   expect(await page.evaluate(() => window['result'].onInput)).toEqual(['blue']);
   expect(await page.evaluate(() => window['result'].onChange)).toEqual(['blue']);
 });
+
+it('should wait for option to be present',async ({page, server}) => {
+  await page.goto(server.PREFIX + '/input/select.html');
+  const selectPromise  = page.selectOption('select', 'scarlet');
+  let didSelect = false;
+  selectPromise.then(() => didSelect = true);
+  await giveItAChanceToResolve(page);
+  expect(didSelect).toBe(false);
+  await page.$eval('select', select => {
+    const option = document.createElement('option');
+    option.value = 'scarlet';
+    option.textContent = 'Scarlet';
+    select.appendChild(option);
+  });
+  const items = await selectPromise;
+  expect(items).toStrictEqual(['scarlet']);
+});
+
+it('should wait for option index to be present',async ({page, server}) => {
+  await page.goto(server.PREFIX + '/input/select.html');
+  const len = await page.$eval('select', select => select.options.length);
+  const selectPromise  = page.selectOption('select', {index: len});
+  let didSelect = false;
+  selectPromise.then(() => didSelect = true);
+  await giveItAChanceToResolve(page);
+  expect(didSelect).toBe(false);
+  await page.$eval('select', select => {
+    const option = document.createElement('option');
+    option.value = 'scarlet';
+    option.textContent = 'Scarlet';
+    select.appendChild(option);
+  });
+  const items = await selectPromise;
+  expect(items).toStrictEqual(['scarlet']);
+});
+
+it('should wait for multiple options to be present',async ({page, server}) => {
+  await page.goto(server.PREFIX + '/input/select.html');
+  await page.evaluate(() => window['makeMultiple']());
+  const selectPromise  = page.selectOption('select', ['green', 'scarlet']);
+  let didSelect = false;
+  selectPromise.then(() => didSelect = true);
+  await giveItAChanceToResolve(page);
+  expect(didSelect).toBe(false);
+  await page.$eval('select', select => {
+    const option = document.createElement('option');
+    option.value = 'scarlet';
+    option.textContent = 'Scarlet';
+    select.appendChild(option);
+  });
+  const items = await selectPromise;
+  expect(items).toStrictEqual(['green', 'scarlet']);
+});

types/types.d.ts

@@ -2417,6 +2417,8 @@ export interface Page {
    * Triggers a `change` and `input` event once all the provided options have been selected. If there's no `<select>` element
    * matching `selector`, the method throws an error.
    * 
+   * Will wait until all specified options are present in the `<select>` element.
+   * 
    * ```js
    * // single selection matching the value
    * page.selectOption('select#colors', 'blue');
@@ -4090,6 +4092,8 @@ export interface Frame {
    * Triggers a `change` and `input` event once all the provided options have been selected. If there's no `<select>` element
    * matching `selector`, the method throws an error.
    * 
+   * Will wait until all specified options are present in the `<select>` element.
+   * 
    * ```js
    * // single selection matching the value
    * frame.selectOption('select#colors', 'blue');
@@ -5940,6 +5944,8 @@ export interface ElementHandle<T=Node> extends JSHandle<T> {
    * Triggers a `change` and `input` event once all the provided options have been selected. If element is not a `<select>`
    * element, the method throws an error.
    * 
+   * Will wait until all specified options are present in the `<select>` element.
+   * 
    * ```js
    * // single selection matching the value
    * handle.selectOption('blue');