From 091b84faa38fe5a9a501f85e6f607e4e0472ff6d Mon Sep 17 00:00:00 2001
From: Max Schmitt <max@schmitt.mx>
Date: Thu, 6 Jun 2024 23:27:13 +0200
Subject: [PATCH] feedback 1

---
 docs/src/api/class-elementhandle.md           |  2 +-
 docs/src/api/class-locator.md                 |  2 +-
 docs/src/api/class-page.md                    |  2 +-
 .../src/client/elementHandle.ts               |  6 ++--
 packages/playwright-core/src/server/dom.ts    |  4 +--
 packages/playwright-core/types/types.d.ts     | 12 +++----
 tests/page/page-set-input-files.spec.ts       | 35 +++++++++++++------
 7 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/docs/src/api/class-elementhandle.md b/docs/src/api/class-elementhandle.md
index faeedb4a13305a..24b8bb2b6f6d01 100644
--- a/docs/src/api/class-elementhandle.md
+++ b/docs/src/api/class-elementhandle.md
@@ -953,7 +953,7 @@ When all steps combined have not finished during the specified [`option: timeout
 
 Sets the value of the file input to these file paths or files. If some of the `filePaths` are relative paths, then they
 are resolved relative to the current working directory. For empty array, clears the selected files.
-In order to upload a directory (`[webkitdirectory]`) pass the path to the directory.
+For inputs with a `[webkitdirectory]` attribute, only a single directory path is supported.
 
 This method expects [ElementHandle] to point to an
 [input element](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input). 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), targets the control instead.
diff --git a/docs/src/api/class-locator.md b/docs/src/api/class-locator.md
index 80c57a9a3d362f..463b47849bd131 100644
--- a/docs/src/api/class-locator.md
+++ b/docs/src/api/class-locator.md
@@ -2164,7 +2164,7 @@ When all steps combined have not finished during the specified [`option: timeout
 * since: v1.14
 
 Upload file or multiple files into `<input type=file>`.
-In order to upload a directory (`[webkitdirectory]`) pass the path to the directory.
+For inputs with a `[webkitdirectory]` attribute, only a single directory path is supported.
 
 **Usage**
 
diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md
index 0b722dd07a913e..f0876f67cd7389 100644
--- a/docs/src/api/class-page.md
+++ b/docs/src/api/class-page.md
@@ -3927,7 +3927,7 @@ An object containing additional HTTP headers to be sent with every request. All
 
 Sets the value of the file input to these file paths or files. If some of the `filePaths` are relative paths, then they
 are resolved relative to the current working directory. For empty array, clears the selected files.
-In order to upload a directory (`[webkitdirectory]`) pass the path to the directory.
+For inputs with a `[webkitdirectory]` attribute, only a single directory path is supported.
 
 This method expects [`param: selector`] to point to an
 [input element](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input). 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), targets the control instead.
diff --git a/packages/playwright-core/src/client/elementHandle.ts b/packages/playwright-core/src/client/elementHandle.ts
index d2fd73e770ff10..1e6cdda53a7a04 100644
--- a/packages/playwright-core/src/client/elementHandle.ts
+++ b/packages/playwright-core/src/client/elementHandle.ts
@@ -262,9 +262,9 @@ export async function convertInputFiles(files: string | FilePayload | string[] |
   if (items.some(item => typeof item === 'string')) {
     if (!items.every(item => typeof item === 'string'))
       throw new Error('File paths cannot be mixed with buffers');
-    const directoryCount = (await Promise.all(items.map(async item => (await fs.promises.stat(item as string)).isDirectory()))).filter(Boolean).length;
-    if (directoryCount > 1)
-      throw new Error('Only one directory can be uploaded at a time');
+    const itemFileType = (await Promise.all(items.map(async item => (await fs.promises.stat(item as string)).isDirectory() ? 'directory' : 'file')));
+    if (new Set(itemFileType).size > 1 || itemFileType.filter(type => type === 'directory').length > 1)
+      throw new Error('File paths must be all files or a single directory');
 
     if (context._connection.isRemote()) {
       let streams: channels.WritableStreamChannel[] | undefined;
diff --git a/packages/playwright-core/src/server/dom.ts b/packages/playwright-core/src/server/dom.ts
index a9942df66a39c0..852beba31984df 100644
--- a/packages/playwright-core/src/server/dom.ts
+++ b/packages/playwright-core/src/server/dom.ts
@@ -648,8 +648,8 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
           fs.promises.access(localPath, fs.constants.F_OK)
         )));
         const isDirectoryUpload = localPaths.length === 1 ? (await fs.promises.stat(localPaths[0])).isDirectory() : false;
-        const waitForChangeEvent = isDirectoryUpload ? this.evaluateInUtility(([_, node]) => new Promise<any>(fulfil => {
-          node.addEventListener('change', fulfil, { once: true });
+        const waitForChangeEvent = isDirectoryUpload ? this.evaluateInUtility(([_, node]) => new Promise<any>(fulfill => {
+          node.addEventListener('change', fulfill, { once: true });
         }), undefined) : Promise.resolve();
         await this._page._delegate.setInputFilePaths(retargeted, localPaths);
         await waitForChangeEvent;
diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts
index 827bcc2649dace..7f540cbd4f0db8 100644
--- a/packages/playwright-core/types/types.d.ts
+++ b/packages/playwright-core/types/types.d.ts
@@ -4055,8 +4055,8 @@ export interface Page {
    * instead. Read more about [locators](https://playwright.dev/docs/locators).
    *
    * Sets the value of the file input to these file paths or files. If some of the `filePaths` are relative paths, then
-   * they are resolved relative to the current working directory. For empty array, clears the selected files. In order
-   * to upload a directory (`[webkitdirectory]`) pass the path to the directory.
+   * they are resolved relative to the current working directory. For empty array, clears the selected files. For inputs
+   * with a `[webkitdirectory]` attribute, only a single directory path is supported.
    *
    * This method expects `selector` to point to an
    * [input element](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input). However, if the element is inside
@@ -10580,8 +10580,8 @@ export interface ElementHandle<T=Node> extends JSHandle<T> {
    * instead. Read more about [locators](https://playwright.dev/docs/locators).
    *
    * Sets the value of the file input to these file paths or files. If some of the `filePaths` are relative paths, then
-   * they are resolved relative to the current working directory. For empty array, clears the selected files. In order
-   * to upload a directory (`[webkitdirectory]`) pass the path to the directory.
+   * they are resolved relative to the current working directory. For empty array, clears the selected files. For inputs
+   * with a `[webkitdirectory]` attribute, only a single directory path is supported.
    *
    * This method expects {@link ElementHandle} to point to an
    * [input element](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input). However, if the element is inside
@@ -12788,8 +12788,8 @@ export interface Locator {
   }): Promise<void>;
 
   /**
-   * Upload file or multiple files into `<input type=file>`. In order to upload a directory (`[webkitdirectory]`) pass
-   * the path to the directory.
+   * Upload file or multiple files into `<input type=file>`. For inputs with a `[webkitdirectory]` attribute, only a
+   * single directory path is supported.
    *
    * **Usage**
    *
diff --git a/tests/page/page-set-input-files.spec.ts b/tests/page/page-set-input-files.spec.ts
index 21f1c253116b96..ee73dfa63cd650 100644
--- a/tests/page/page-set-input-files.spec.ts
+++ b/tests/page/page-set-input-files.spec.ts
@@ -37,20 +37,17 @@ it('should upload the file', async ({ page, server, asset }) => {
   }, input)).toBe('contents of the file');
 });
 
-async function createTestDirectoryStructure() {
-  const baseDir = path.join(it.info().outputDir, 'file-upload-test');
-  await fs.promises.mkdir(baseDir, { recursive: true });
-  await fs.promises.writeFile(path.join(baseDir, 'file1.txt'), 'file1 content');
-  await fs.promises.writeFile(path.join(baseDir, 'file2'), 'file2 content');
-  await fs.promises.mkdir(path.join(baseDir, 'sub-dir'));
-  await fs.promises.writeFile(path.join(baseDir, 'sub-dir', 'really.txt'), 'sub-dir file content');
-  return baseDir;
-}
-
 it('should upload a folder', async ({ page, server, browserName, headless, browserMajorVersion }) => {
   await page.goto(server.PREFIX + '/input/folderupload.html');
   const input = await page.$('input');
-  const dir = await createTestDirectoryStructure();
+  const dir = path.join(it.info().outputDir, 'file-upload-test');
+  {
+    await fs.promises.mkdir(dir, { recursive: true });
+    await fs.promises.writeFile(path.join(dir, 'file1.txt'), 'file1 content');
+    await fs.promises.writeFile(path.join(dir, 'file2'), 'file2 content');
+    await fs.promises.mkdir(path.join(dir, 'sub-dir'));
+    await fs.promises.writeFile(path.join(dir, 'sub-dir', 'really.txt'), 'sub-dir file content');
+  }
   await input.setInputFiles(dir);
   expect(new Set(await page.evaluate(e => [...e.files].map(f => f.webkitRelativePath), input))).toEqual(new Set([
     // https://issues.chromium.org/issues/345393164
@@ -70,6 +67,22 @@ it('should upload a folder', async ({ page, server, browserName, headless, brows
   }
 });
 
+it('should upload a folder and throw for multiple directories', async ({ page, server, browserName, headless, browserMajorVersion }) => {
+  await page.goto(server.PREFIX + '/input/folderupload.html');
+  const input = await page.$('input');
+  const dir = path.join(it.info().outputDir, 'file-upload-test');
+  {
+    await fs.promises.mkdir(path.join(dir, 'folder1'), { recursive: true });
+    await fs.promises.writeFile(path.join(dir, 'folder1', 'file1.txt'), 'file1 content');
+    await fs.promises.mkdir(path.join(dir, 'folder2'), { recursive: true });
+    await fs.promises.writeFile(path.join(dir, 'folder2', 'file2.txt'), 'file2 content');
+  }
+  await expect(input.setInputFiles([
+    path.join(dir, 'folder1'),
+    path.join(dir, 'folder2'),
+  ])).rejects.toThrow('File paths must be all files or a single directory');
+});
+
 it('should upload a file after popup', async ({ page, server, asset }) => {
   it.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/29923' });
   await page.goto(server.PREFIX + '/input/fileupload.html');