From f123f7ac690f9184f0781c47950924e0094f605e Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Fri, 22 Nov 2024 11:40:43 +0000
Subject: [PATCH] fix: isEditable/toBeEditable throw for elements that cannot
 be editable/readonly (#33713)

---
 docs/src/actionability.md                         | 13 +++++++++++--
 docs/src/api/class-locator.md                     |  2 +-
 .../src/server/injected/injectedScript.ts         | 11 +++++++----
 .../src/server/injected/roleUtils.ts              | 15 +++++++++++++++
 packages/playwright-core/types/types.d.ts         |  4 +++-
 tests/page/expect-boolean.spec.ts                 |  7 +++++++
 tests/page/locator-convenience.spec.ts            | 15 ++++++++++++++-
 tests/page/page-fill.spec.ts                      |  4 ++--
 8 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/docs/src/actionability.md b/docs/src/actionability.md
index 4a84f5f5b2..3510171e5c 100644
--- a/docs/src/actionability.md
+++ b/docs/src/actionability.md
@@ -93,11 +93,20 @@ Element is considered stable when it has maintained the same bounding box for at
 
 ## Enabled
 
-Element is considered enabled unless it is a `<button>`, `<select>`, `<input>` or `<textarea>` with a `disabled` property.
+Element is considered enabled when it is **not disabled**.
+
+Element is **disabled** when:
+- it is a `<button>`, `<select>`, `<input>`, `<textarea>`, `<option>` or `<optgroup>` with a `[disabled]` attribute;
+- it is a `<button>`, `<select>`, `<input>`, `<textarea>`, `<option>` or `<optgroup>` that is a part of a `<fieldset>` with a `[disabled]` attribute;
+- it is a descendant of an element with `[aria-disabled=true]` attribute.
 
 ## Editable
 
-Element is considered editable when it is [enabled] and does not have `readonly` property set.
+Element is considered editable when it is [enabled] and is **not readonly**.
+
+Element is **readonly** when:
+- it is a `<select>`, `<input>` or `<textarea>` with a `[readonly]` attribute;
+- it has an `[aria-readonly=true]` attribute and an aria role that [supports it](https://w3c.github.io/aria/#aria-readonly).
 
 ## Receives Events
 
diff --git a/docs/src/api/class-locator.md b/docs/src/api/class-locator.md
index 64c5873710..e61b119c69 100644
--- a/docs/src/api/class-locator.md
+++ b/docs/src/api/class-locator.md
@@ -1483,7 +1483,7 @@ Boolean disabled = await page.GetByRole(AriaRole.Button).IsDisabledAsync();
 * since: v1.14
 - returns: <[boolean]>
 
-Returns whether the element is [editable](../actionability.md#editable).
+Returns whether the element is [editable](../actionability.md#editable). If the target element is not an `<input>`, `<textarea>`, `<select>`, `[contenteditable]` and does not have a role allowing `[aria-readonly]`, this method throws an error.
 
 :::warning[Asserting editable state]
 If you need to assert that an element is editable, prefer [`method: LocatorAssertions.toBeEditable`] to avoid flakiness. See [assertions guide](../test-assertions.md) for more details.
diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts
index aa0eb1004f..74ff799ff1 100644
--- a/packages/playwright-core/src/server/injected/injectedScript.ts
+++ b/packages/playwright-core/src/server/injected/injectedScript.ts
@@ -29,7 +29,7 @@ import type { CSSComplexSelectorList } from '../../utils/isomorphic/cssParser';
 import { generateSelector, type GenerateSelectorOptions } from './selectorGenerator';
 import type * as channels from '@protocol/channels';
 import { Highlight } from './highlight';
-import { getChecked, getAriaDisabled, getAriaRole, getElementAccessibleName, getElementAccessibleDescription } from './roleUtils';
+import { getChecked, getAriaDisabled, getAriaRole, getElementAccessibleName, getElementAccessibleDescription, getReadonly } from './roleUtils';
 import { kLayoutSelectorNames, type LayoutSelectorName, layoutSelectorScore } from './layoutSelectorUtils';
 import { asLocator } from '../../utils/isomorphic/locatorGenerators';
 import type { Language } from '../../utils/isomorphic/locatorGenerators';
@@ -626,9 +626,12 @@ export class InjectedScript {
     if (state === 'enabled')
       return !disabled;
 
-    const editable = !(['INPUT', 'TEXTAREA', 'SELECT'].includes(element.nodeName) && element.hasAttribute('readonly'));
-    if (state === 'editable')
-      return !disabled && editable;
+    if (state === 'editable') {
+      const readonly = getReadonly(element);
+      if (readonly === 'error')
+        throw this.createStacklessError('Element is not an <input>, <textarea>, <select> or [contenteditable] and does not have a role allowing [aria-readonly]');
+      return !disabled && !readonly;
+    }
 
     if (state === 'checked' || state === 'unchecked') {
       const need = state === 'checked';
diff --git a/packages/playwright-core/src/server/injected/roleUtils.ts b/packages/playwright-core/src/server/injected/roleUtils.ts
index 25035b6998..cc9e6a70d0 100644
--- a/packages/playwright-core/src/server/injected/roleUtils.ts
+++ b/packages/playwright-core/src/server/injected/roleUtils.ts
@@ -860,6 +860,21 @@ export function getChecked(element: Element, allowMixed: boolean): boolean | 'mi
   return 'error';
 }
 
+// https://w3c.github.io/aria/#aria-readonly
+const kAriaReadonlyRoles = ['checkbox', 'combobox', 'grid', 'gridcell', 'listbox', 'radiogroup', 'slider', 'spinbutton', 'textbox', 'columnheader', 'rowheader', 'searchbox', 'switch', 'treegrid'];
+export function getReadonly(element: Element): boolean | 'error' {
+  const tagName = elementSafeTagName(element);
+  // https://www.w3.org/TR/wai-aria-1.2/#aria-checked
+  // https://www.w3.org/TR/html-aam-1.0/#html-attribute-state-and-property-mappings
+  if (['INPUT', 'TEXTAREA', 'SELECT'].includes(tagName))
+    return element.hasAttribute('readonly');
+  if (kAriaReadonlyRoles.includes(getAriaRole(element) || ''))
+    return element.getAttribute('aria-readonly') === 'true';
+  if ((element as HTMLElement).isContentEditable)
+    return false;
+  return 'error';
+}
+
 export const kAriaPressedRoles = ['button'];
 export function getAriaPressed(element: Element): boolean | 'mixed' {
   // https://www.w3.org/TR/wai-aria-1.2/#aria-pressed
diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts
index d7e3a4f36a..7086d87f2e 100644
--- a/packages/playwright-core/types/types.d.ts
+++ b/packages/playwright-core/types/types.d.ts
@@ -13680,7 +13680,9 @@ export interface Locator {
   }): Promise<boolean>;
 
   /**
-   * Returns whether the element is [editable](https://playwright.dev/docs/actionability#editable).
+   * Returns whether the element is [editable](https://playwright.dev/docs/actionability#editable). If the target element is not an `<input>`,
+   * `<textarea>`, `<select>`, `[contenteditable]` and does not have a role allowing `[aria-readonly]`, this method
+   * throws an error.
    *
    * **NOTE** If you need to assert that an element is editable, prefer
    * [expect(locator).toBeEditable([options])](https://playwright.dev/docs/api/class-locatorassertions#locator-assertions-to-be-editable)
diff --git a/tests/page/expect-boolean.spec.ts b/tests/page/expect-boolean.spec.ts
index a22657bda6..1890c608b9 100644
--- a/tests/page/expect-boolean.spec.ts
+++ b/tests/page/expect-boolean.spec.ts
@@ -138,6 +138,13 @@ test.describe('toBeEditable', () => {
     const locator = page.locator('input');
     await expect(locator).not.toBeEditable({ editable: false });
   });
+
+  test('throws', async ({ page }) => {
+    await page.setContent('<button>');
+    const locator = page.locator('button');
+    const error = await expect(locator).toBeEditable().catch(e => e);
+    expect(error.message).toContain('Element is not an <input>, <textarea>, <select> or [contenteditable] and does not have a role allowing [aria-readonly]');
+  });
 });
 
 test.describe('toBeEnabled', () => {
diff --git a/tests/page/locator-convenience.spec.ts b/tests/page/locator-convenience.spec.ts
index 143bd526d2..d63b890fd9 100644
--- a/tests/page/locator-convenience.spec.ts
+++ b/tests/page/locator-convenience.spec.ts
@@ -119,7 +119,15 @@ it('isEnabled and isDisabled should work', async ({ page }) => {
 });
 
 it('isEditable should work', async ({ page }) => {
-  await page.setContent(`<input id=input1 disabled><textarea></textarea><input id=input2>`);
+  await page.setContent(`
+    <input id=input1 disabled>
+    <textarea></textarea>
+    <input id=input2>
+    <div contenteditable="true"></div>
+    <span id=span1 role=textbox aria-readonly=true></span>
+    <span id=span2 role=textbox></span>
+    <button>button</button>
+  `);
   await page.$eval('textarea', t => t.readOnly = true);
   const input1 = page.locator('#input1');
   expect(await input1.isEditable()).toBe(false);
@@ -130,6 +138,11 @@ it('isEditable should work', async ({ page }) => {
   const textarea = page.locator('textarea');
   expect(await textarea.isEditable()).toBe(false);
   expect(await page.isEditable('textarea')).toBe(false);
+  expect(await page.locator('div').isEditable()).toBe(true);
+  expect(await page.locator('#span1').isEditable()).toBe(false);
+  expect(await page.locator('#span2').isEditable()).toBe(true);
+  const error = await page.locator('button').isEditable().catch(e => e);
+  expect(error.message).toContain('Element is not an <input>, <textarea>, <select> or [contenteditable] and does not have a role allowing [aria-readonly]');
 });
 
 it('isChecked should work', async ({ page }) => {
diff --git a/tests/page/page-fill.spec.ts b/tests/page/page-fill.spec.ts
index 88cd33d407..d42dd046dc 100644
--- a/tests/page/page-fill.spec.ts
+++ b/tests/page/page-fill.spec.ts
@@ -238,8 +238,8 @@ it('should fill elements with existing value and selection', async ({ page, serv
 
 it('should throw nice error without injected script stack when element is not an <input>', async ({ page, server }) => {
   let error = null;
-  await page.goto(server.PREFIX + '/input/textarea.html');
-  await page.fill('body', '').catch(e => error = e);
+  await page.setContent(`<select><option>value1</option></select>`);
+  await page.fill('select', '').catch(e => error = e);
   expect(error.message).toContain('page.fill: Error: Element is not an <input>, <textarea> or [contenteditable] element\nCall log:');
 });