From 7aa3935dcce55a892f9f2b5982f2db048c1c889f Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Wed, 7 Dec 2022 09:04:32 -0800
Subject: [PATCH] chore: match selected options by both value and label
 (#19316)

---
 docs/src/api/class-locator.md                 | 39 +++++++++++--------
 docs/src/api/params.md                        |  2 +-
 .../src/client/elementHandle.ts               |  2 +-
 packages/playwright-core/src/client/types.ts  |  2 +-
 .../playwright-core/src/protocol/validator.ts |  2 +
 .../src/server/injected/injectedScript.ts     |  7 +++-
 packages/playwright-core/types/types.d.ts     | 32 ++++++++++-----
 packages/protocol/src/channels.ts             |  4 ++
 packages/protocol/src/protocol.yml            |  2 +
 tests/page/page-select-option.spec.ts         |  7 ++++
 10 files changed, 68 insertions(+), 31 deletions(-)

diff --git a/docs/src/api/class-locator.md b/docs/src/api/class-locator.md
index 7e82d548dd..6a0fb6af80 100644
--- a/docs/src/api/class-locator.md
+++ b/docs/src/api/class-locator.md
@@ -997,6 +997,10 @@ completely visible as defined by
 * since: v1.14
 - returns: <[Array]<[string]>>
 
+Selects option or options in `<select>`.
+
+**Details**
+
 This method waits for [actionability](../actionability.md) checks, waits until all specified options are present in the `<select>` element and selects these options.
 
 If the target element is not a `<select>` element, this method throws an error. However, if the element is inside the `<label>` element that has an associated [control](https://developer.mozilla.org/en-US/docs/Web/API/HTMLLabelElement/control), the control will be used instead.
@@ -1007,56 +1011,59 @@ Triggers a `change` and `input` event once all the provided options have been se
 
 **Usage**
 
+```html
+<select multiple>
+  <option value="red">Red</div>
+  <option value="green">Green</div>
+  <option value="blue">Blue</div>
+</select>
+```
+
 ```js
-// single selection matching the value
+// single selection matching the value or label
 element.selectOption('blue');
 
 // single selection matching the label
 element.selectOption({ label: 'Blue' });
 
-// multiple selection
+// multiple selection for red, green and blue options
 element.selectOption(['red', 'green', 'blue']);
 ```
 
 ```java
-// single selection matching the value
+// single selection matching the value or label
 element.selectOption("blue");
 // single selection matching the label
 element.selectOption(new SelectOption().setLabel("Blue"));
-// multiple selection
+// multiple selection for blue, red and second option
 element.selectOption(new String[] {"red", "green", "blue"});
 ```
 
 ```python async
-# single selection matching the value
+# single selection matching the value or label
 await element.select_option("blue")
 # single selection matching the label
 await element.select_option(label="blue")
-# multiple selection
+# multiple selection for blue, red and second option
 await element.select_option(value=["red", "green", "blue"])
 ```
 
 ```python sync
-# single selection matching the value
+# single selection matching the value or label
 element.select_option("blue")
-# single selection matching both the label
+# single selection matching the label
 element.select_option(label="blue")
-# multiple selection
+# multiple selection for blue, red and second option
 element.select_option(value=["red", "green", "blue"])
 ```
 
 ```csharp
-// single selection matching the value
+// single selection matching the value or label
 await element.SelectOptionAsync(new[] { "blue" });
 // single selection matching the label
 await element.SelectOptionAsync(new[] { new SelectOptionValue() { Label = "blue" } });
-// multiple selection
-await element.SelectOptionAsync(new[] { "red", "green", "blue" });
 // multiple selection for blue, red and second option
-await element.SelectOptionAsync(new[] {
-    new SelectOptionValue() { Label = "blue" },
-    new SelectOptionValue() { Index = 2 },
-    new SelectOptionValue() { Value = "red" }});
+await element.SelectOptionAsync(new[] { "red", "green", "blue" });
 ```
 
 ### param: Locator.selectOption.values = %%-select-options-values-%%
diff --git a/docs/src/api/params.md b/docs/src/api/params.md
index c027f9bfe0..50d38efa12 100644
--- a/docs/src/api/params.md
+++ b/docs/src/api/params.md
@@ -697,7 +697,7 @@ Whether to allow sites to register Service workers. Defaults to `'allow'`.
   - `index` ?<[int]> Matches by the index. Optional.
 
 Options to select. If the `<select>` has the `multiple` attribute, all matching options are selected, otherwise only the
-first option matching one of the passed options is selected. String values are equivalent to `{value:'string'}`. Option
+first option matching one of the passed options is selected. String values are matching both values and labels. Option
 is considered matching if all specified properties match.
 
 ## wait-for-navigation-url
diff --git a/packages/playwright-core/src/client/elementHandle.ts b/packages/playwright-core/src/client/elementHandle.ts
index 3cd6efaac9..891410c58a 100644
--- a/packages/playwright-core/src/client/elementHandle.ts
+++ b/packages/playwright-core/src/client/elementHandle.ts
@@ -252,7 +252,7 @@ export function convertSelectOptionValues(values: string | api.ElementHandle | S
   if (values[0] instanceof ElementHandle)
     return { elements: (values as ElementHandle[]).map((v: ElementHandle) => v._elementChannel) };
   if (isString(values[0]))
-    return { options: (values as string[]).map(value => ({ value })) };
+    return { options: (values as string[]).map(valueOrLabel => ({ valueOrLabel })) };
   return { options: values as SelectOption[] };
 }
 
diff --git a/packages/playwright-core/src/client/types.ts b/packages/playwright-core/src/client/types.ts
index 3852d00c7f..cfdcfa3030 100644
--- a/packages/playwright-core/src/client/types.ts
+++ b/packages/playwright-core/src/client/types.ts
@@ -32,7 +32,7 @@ export type Env = { [key: string]: string | number | boolean | undefined };
 export type WaitForEventOptions = Function | { predicate?: Function, timeout?: number };
 export type WaitForFunctionOptions = { timeout?: number, polling?: 'raf' | number };
 
-export type SelectOption = { value?: string, label?: string, index?: number };
+export type SelectOption = { value?: string, label?: string, index?: number, valueOrLabel?: string };
 export type SelectOptionOptions = { force?: boolean, timeout?: number, noWaitAfter?: boolean };
 export type FilePayload = { name: string, mimeType: string, buffer: Buffer };
 export type StorageState = {
diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts
index 4422903c9e..1781af6ae5 100644
--- a/packages/playwright-core/src/protocol/validator.ts
+++ b/packages/playwright-core/src/protocol/validator.ts
@@ -1465,6 +1465,7 @@ scheme.FrameSelectOptionParams = tObject({
   strict: tOptional(tBoolean),
   elements: tOptional(tArray(tChannel(['ElementHandle']))),
   options: tOptional(tArray(tObject({
+    valueOrLabel: tOptional(tString),
     value: tOptional(tString),
     label: tOptional(tString),
     index: tOptional(tNumber),
@@ -1833,6 +1834,7 @@ scheme.ElementHandleScrollIntoViewIfNeededResult = tOptional(tObject({}));
 scheme.ElementHandleSelectOptionParams = tObject({
   elements: tOptional(tArray(tChannel(['ElementHandle']))),
   options: tOptional(tArray(tObject({
+    valueOrLabel: tOptional(tString),
     value: tOptional(tString),
     label: tOptional(tString),
     index: tOptional(tNumber),
diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts
index c83bc5bd64..999bb965d7 100644
--- a/packages/playwright-core/src/server/injected/injectedScript.ts
+++ b/packages/playwright-core/src/server/injected/injectedScript.ts
@@ -621,8 +621,9 @@ export class InjectedScript {
     throw this.createStacklessError(`Unexpected element state "${state}"`);
   }
 
-  selectOptions(optionsToSelect: (Node | { value?: string, label?: string, index?: number })[],
+  selectOptions(optionsToSelect: (Node | { valueOrLabel?: string, value?: string, label?: string, index?: number })[],
     node: Node, progress: InjectedScriptProgress): string[] | 'error:notconnected' | symbol {
+
     const element = this.retarget(node, 'follow-label');
     if (!element)
       return 'error:notconnected';
@@ -634,10 +635,12 @@ export class InjectedScript {
     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 }) => {
+      const filter = (optionToSelect: Node | { valueOrLabel?: string, value?: string, label?: string, index?: number }) => {
         if (optionToSelect instanceof Node)
           return option === optionToSelect;
         let matches = true;
+        if (optionToSelect.valueOrLabel !== undefined)
+          matches = matches && (optionToSelect.valueOrLabel === option.value || optionToSelect.valueOrLabel === option.label);
         if (optionToSelect.value !== undefined)
           matches = matches && optionToSelect.value === option.value;
         if (optionToSelect.label !== undefined)
diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts
index 50245746cc..acce927a64 100644
--- a/packages/playwright-core/types/types.d.ts
+++ b/packages/playwright-core/types/types.d.ts
@@ -3517,8 +3517,8 @@ export interface Page {
    * @param selector A selector to search for an element. If there are multiple elements satisfying the selector, the first will be
    * used.
    * @param values Options to select. If the `<select>` has the `multiple` attribute, all matching options are selected, otherwise
-   * only the first option matching one of the passed options is selected. String values are equivalent to
-   * `{value:'string'}`. Option is considered matching if all specified properties match.
+   * only the first option matching one of the passed options is selected. String values are matching both values and
+   * labels. Option is considered matching if all specified properties match.
    * @param options
    */
   selectOption(selector: string, values: null|string|ElementHandle|Array<string>|{
@@ -6389,8 +6389,8 @@ export interface Frame {
    *
    * @param selector A selector to query for.
    * @param values Options to select. If the `<select>` has the `multiple` attribute, all matching options are selected, otherwise
-   * only the first option matching one of the passed options is selected. String values are equivalent to
-   * `{value:'string'}`. Option is considered matching if all specified properties match.
+   * only the first option matching one of the passed options is selected. String values are matching both values and
+   * labels. Option is considered matching if all specified properties match.
    * @param options
    */
   selectOption(selector: string, values: null|string|ElementHandle|Array<string>|{
@@ -9316,8 +9316,8 @@ export interface ElementHandle<T=Node> extends JSHandle<T> {
    * ```
    *
    * @param values Options to select. If the `<select>` has the `multiple` attribute, all matching options are selected, otherwise
-   * only the first option matching one of the passed options is selected. String values are equivalent to
-   * `{value:'string'}`. Option is considered matching if all specified properties match.
+   * only the first option matching one of the passed options is selected. String values are matching both values and
+   * labels. Option is considered matching if all specified properties match.
    * @param options
    */
   selectOption(values: null|string|ElementHandle|Array<string>|{
@@ -10923,6 +10923,10 @@ export interface Locator {
   }): Promise<void>;
 
   /**
+   * Selects option or options in `<select>`.
+   *
+   * **Details**
+   *
    * This method waits for [actionability](https://playwright.dev/docs/actionability) checks, waits until all specified options are present in
    * the `<select>` element and selects these options.
    *
@@ -10937,20 +10941,28 @@ export interface Locator {
    *
    * **Usage**
    *
+   * ```html
+   * <select multiple>
+   *   <option value="red">Red</div>
+   *   <option value="green">Green</div>
+   *   <option value="blue">Blue</div>
+   * </select>
+   * ```
+   *
    * ```js
-   * // single selection matching the value
+   * // single selection matching the value or label
    * element.selectOption('blue');
    *
    * // single selection matching the label
    * element.selectOption({ label: 'Blue' });
    *
-   * // multiple selection
+   * // multiple selection for blue, red and second option
    * element.selectOption(['red', 'green', 'blue']);
    * ```
    *
    * @param values Options to select. If the `<select>` has the `multiple` attribute, all matching options are selected, otherwise
-   * only the first option matching one of the passed options is selected. String values are equivalent to
-   * `{value:'string'}`. Option is considered matching if all specified properties match.
+   * only the first option matching one of the passed options is selected. String values are matching both values and
+   * labels. Option is considered matching if all specified properties match.
    * @param options
    */
   selectOption(values: null|string|ElementHandle|Array<string>|{
diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts
index 985c8c405c..d5b555c68a 100644
--- a/packages/protocol/src/channels.ts
+++ b/packages/protocol/src/channels.ts
@@ -2651,6 +2651,7 @@ export type FrameSelectOptionParams = {
   strict?: boolean,
   elements?: ElementHandleChannel[],
   options?: {
+    valueOrLabel?: string,
     value?: string,
     label?: string,
     index?: number,
@@ -2663,6 +2664,7 @@ export type FrameSelectOptionOptions = {
   strict?: boolean,
   elements?: ElementHandleChannel[],
   options?: {
+    valueOrLabel?: string,
     value?: string,
     label?: string,
     index?: number,
@@ -3275,6 +3277,7 @@ export type ElementHandleScrollIntoViewIfNeededResult = void;
 export type ElementHandleSelectOptionParams = {
   elements?: ElementHandleChannel[],
   options?: {
+    valueOrLabel?: string,
     value?: string,
     label?: string,
     index?: number,
@@ -3286,6 +3289,7 @@ export type ElementHandleSelectOptionParams = {
 export type ElementHandleSelectOptionOptions = {
   elements?: ElementHandleChannel[],
   options?: {
+    valueOrLabel?: string,
     value?: string,
     label?: string,
     index?: number,
diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml
index eb1ee8a668..0c9e5daf47 100644
--- a/packages/protocol/src/protocol.yml
+++ b/packages/protocol/src/protocol.yml
@@ -1962,6 +1962,7 @@ Frame:
           items:
             type: object
             properties:
+              valueOrLabel: string?
               value: string?
               label: string?
               index: number?
@@ -2512,6 +2513,7 @@ ElementHandle:
           items:
             type: object
             properties:
+              valueOrLabel: string?
               value: string?
               label: string?
               index: number?
diff --git a/tests/page/page-select-option.spec.ts b/tests/page/page-select-option.spec.ts
index 2046f2cb29..7cae8a2540 100644
--- a/tests/page/page-select-option.spec.ts
+++ b/tests/page/page-select-option.spec.ts
@@ -36,6 +36,13 @@ it('should select single option by value', async ({ page, server }) => {
   expect(await page.evaluate(() => window['result'].onChange)).toEqual(['blue']);
 });
 
+it('should fall back to selecting by label', async ({ page, server }) => {
+  await page.goto(server.PREFIX + '/input/select.html');
+  await page.selectOption('select', 'Blue');
+  expect(await page.evaluate(() => window['result'].onInput)).toEqual(['blue']);
+  expect(await page.evaluate(() => window['result'].onChange)).toEqual(['blue']);
+});
+
 it('should select single option by label', async ({ page, server }) => {
   await page.goto(server.PREFIX + '/input/select.html');
   await page.selectOption('select', { label: 'Indigo' });