feedback 3

packages/playwright-core/src/client/elementHandle.ts

@@ -267,28 +267,26 @@ export async function convertInputFiles(files: string | FilePayload | string[] |
       throw new Error('File paths must be all files or a single directory');
 
     if (context._connection.isRemote()) {
-      let streams: channels.WritableStreamChannel[] | undefined;
-      let localPaths: string[] | undefined;
-      await Promise.all((items as string[]).map(async item => {
+      const streams = (await Promise.all((items as string[]).map(async item => {
         const isDirectory = (await fs.promises.stat(item)).isDirectory();
-        const files = isDirectory ? (await fs.promises.readdir(item, { withFileTypes: true, recursive: true })).filter(f => f.isFile()).map(f => path.join(item, f.name)) : [item];
-        const { writableStreams, remoteDir } = await context._wrapApiCall(async () => context._channel.createTempFiles({
-          rootDirName: isDirectory ? item : undefined,
-          items: await Promise.all(files.map(f => fileToTempFileParams(f))),
+        const files = isDirectory ? (await fs.promises.readdir(item, { withFileTypes: true, recursive: true })).filter(f => f.isFile()).map(f => path.join(f.path, f.name)) : [item];
+        const { writableStreams } = await context._wrapApiCall(async () => context._channel.createTempFiles({
+          rootDirName: isDirectory ? path.basename(item) : undefined,
+          items: await Promise.all(files.map(async file => {
+            const lastModifiedMs = (await fs.promises.stat(file)).mtimeMs;
+            return {
+              name: isDirectory ? path.relative(item, file) : path.basename(file),
+              lastModifiedMs
+            };
+          })),
         }), true);
         for (let i = 0; i < files.length; i++) {
           const writable = WritableStream.from(writableStreams[i]);
           await pipelineAsync(fs.createReadStream(files[i]), writable.stream());
         }
-        if (isDirectory) {
-          localPaths ??= [];
-          localPaths.push(remoteDir);
-        } else {
-          streams ??= [];
-          streams.push(...writableStreams);
-        }
-      }));
-      return { streams, localPaths };
+        return writableStreams;
+      }))).flat();
+      return { streams };
     }
     return { localPaths: items.map(f => path.resolve(f as string)) as string[] };
   }
@@ -299,11 +297,6 @@ export async function convertInputFiles(files: string | FilePayload | string[] |
   return { payloads };
 }
 
-async function fileToTempFileParams(file: string): Promise<channels.BrowserContextCreateTempFilesParams['items'][0]> {
-  const lastModifiedMs = (await fs.promises.stat(file)).mtimeMs;
-  return { name: path.basename(file), lastModifiedMs };
-}
-
 export function determineScreenshotType(options: { path?: string, type?: 'png' | 'jpeg' }): 'png' | 'jpeg' | undefined {
   if (options.path) {
     const mimeType = mime.getType(options.path);

packages/playwright-core/src/protocol/validator.ts

@@ -959,7 +959,6 @@ scheme.BrowserContextCreateTempFilesParams = tObject({
   })),
 });
 scheme.BrowserContextCreateTempFilesResult = tObject({
-  remoteDir: tString,
   writableStreams: tArray(tChannel(['WritableStream'])),
 });
 scheme.BrowserContextUpdateSubscriptionParams = tObject({

packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts

@@ -181,14 +181,14 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
   async createTempFiles(params: channels.BrowserContextCreateTempFilesParams): Promise<channels.BrowserContextCreateTempFilesResult> {
     const dir = this._context._browser.options.artifactsDir;
     const tmpDir = path.join(dir, 'upload-' + createGuid());
-    const tempDirWithRootName = path.join(tmpDir, params.rootDirName ? path.basename(params.rootDirName) : '');
+    const tempDirWithRootName = params.rootDirName ? path.join(tmpDir, path.basename(params.rootDirName)) : tmpDir;
     await fs.promises.mkdir(tempDirWithRootName, { recursive: true });
+    this._context._tempDirs.push(tmpDir);
     return {
-      remoteDir: tempDirWithRootName,
       writableStreams: await Promise.all(params.items.map(async item => {
         await fs.promises.mkdir(path.dirname(path.join(tempDirWithRootName, item.name)), { recursive: true });
         const file = fs.createWriteStream(path.join(tempDirWithRootName, item.name));
-        return new WritableStreamDispatcher(this, file, item.lastModifiedMs);
+        return new WritableStreamDispatcher(this, file, item.lastModifiedMs, params.rootDirName ? tempDirWithRootName : undefined);
       }))
     };
   }

packages/playwright-core/src/server/dispatchers/writableStreamDispatcher.ts

@@ -23,10 +23,12 @@ import type { BrowserContextDispatcher } from './browserContextDispatcher';
 export class WritableStreamDispatcher extends Dispatcher<{ guid: string, stream: fs.WriteStream }, channels.WritableStreamChannel, BrowserContextDispatcher> implements channels.WritableStreamChannel {
   _type_WritableStream = true;
   private _lastModifiedMs: number | undefined;
+  private _rootDir: string | undefined;
 
-  constructor(scope: BrowserContextDispatcher, stream: fs.WriteStream, lastModifiedMs?: number) {
+  constructor(scope: BrowserContextDispatcher, stream: fs.WriteStream, lastModifiedMs?: number, rootDir?: string) {
     super(scope, { guid: 'writableStream@' + createGuid(), stream }, 'WritableStream', {});
     this._lastModifiedMs = lastModifiedMs;
+    this._rootDir = rootDir;
   }
 
   async write(params: channels.WritableStreamWriteParams): Promise<channels.WritableStreamWriteResult> {
@@ -51,4 +53,8 @@ export class WritableStreamDispatcher extends Dispatcher<{ guid: string, stream:
   path(): string {
     return this._object.stream.path as string;
   }
+
+  rootDir(): string | undefined {
+    return this._rootDir;
+  }
 }

packages/playwright-core/src/server/dom.ts

@@ -626,17 +626,24 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
 
   async _setInputFiles(progress: Progress, items: InputFilesItems, options: types.NavigatingActionWaitOptions): Promise<'error:notconnected' | 'done'> {
     const { filePayloads, localPaths } = items;
+    const localPathsFileTypes = (await Promise.all((localPaths ?? []).map(async item => (await fs.promises.stat(item as string)).isDirectory() ? 'directory' : 'file')));
+    if (new Set(localPathsFileTypes).size > 1 || localPathsFileTypes.filter(type => type === 'directory').length > 1)
+      throw new Error('File paths must be all files or a single directory');
+    const doesLocalPathsIncludeDirectory = localPathsFileTypes.includes('directory');
     const multiple = filePayloads && filePayloads.length > 1 || localPaths && localPaths.length > 1;
-    const result = await this.evaluateHandleInUtility(([injected, node, multiple]): Element | undefined => {
+    const result = await this.evaluateHandleInUtility(([injected, node, { multiple, doesLocalPathsIncludeDirectory }]): Element | undefined => {
       const element = injected.retarget(node, 'follow-label');
       if (!element)
         return;
       if (element.tagName !== 'INPUT')
         throw injected.createStacklessError('Node is not an HTMLInputElement');
-      if (multiple && !(element as HTMLInputElement).multiple && !(element as HTMLInputElement).webkitdirectory)
+      const inputElement = element as HTMLInputElement;
+      if (multiple && !inputElement.multiple && !inputElement.webkitdirectory)
         throw injected.createStacklessError('Non-multiple file input can only accept single file');
-      return element;
-    }, multiple);
+      if (doesLocalPathsIncludeDirectory && !inputElement.webkitdirectory)
+        throw injected.createStacklessError('File input does not support directories, pass individual files instead');
+      return inputElement;
+    }, { multiple, doesLocalPathsIncludeDirectory });
     if (result === 'error:notconnected' || !result.asElement())
       return 'error:notconnected';
     const retargeted = result.asElement() as ElementHandle<HTMLInputElement>;
@@ -647,14 +654,11 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
         await Promise.all(localPaths.map(localPath => (
           fs.promises.access(localPath, fs.constants.F_OK)
         )));
-        const itemFileTypes = (await Promise.all(localPaths.map(async item => (await fs.promises.stat(item as string)).isDirectory() ? 'directory' : 'file')));
-        if (new Set(itemFileTypes).size > 1 || itemFileTypes.filter(type => type === 'directory').length > 1)
-          throw new Error('File paths must be all files or a single directory');
-        const waitForChangeEvent = itemFileTypes.includes('directory') ? this.evaluateInUtility(([_, node]) => new Promise<any>(fulfill => {
-          node.addEventListener('change', fulfill, { once: true });
-        }), undefined) : Promise.resolve();
+        const waitForInputEvent = doesLocalPathsIncludeDirectory ? this.evaluate(node => new Promise<any>(fulfill => {
+          node.addEventListener('input', fulfill, { once: true });
+        })).catch(() => {}) : Promise.resolve();
         await this._page._delegate.setInputFilePaths(retargeted, localPaths);
-        await waitForChangeEvent;
+        await waitForInputEvent;
       } else {
         await this._page._delegate.setInputFiles(retargeted, filePayloads!);
       }

packages/playwright-core/src/server/fileUploadUtils.ts

@@ -36,8 +36,13 @@ export async function prepareFilesForUpload(frame: Frame, params: channels.Eleme
   if ([payloads, localPaths, streams].filter(Boolean).length !== 1)
     throw new Error('Exactly one of payloads, localPaths and streams must be provided');
 
-  if (streams)
-    localPaths = streams.map(c => (c as WritableStreamDispatcher).path());
+  if (streams) {
+    const directoryMode = streams.every(c => (c as WritableStreamDispatcher).rootDir());
+    if (directoryMode)
+      localPaths = Array.from(new Set(streams.map(c => (c as WritableStreamDispatcher).rootDir()!)));
+    else
+      localPaths = streams.map(c => (c as WritableStreamDispatcher).path());
+  }
   if (localPaths) {
     for (const p of localPaths)
       assert(path.isAbsolute(p) && path.resolve(p) === p, 'Paths provided to localPaths must be absolute and fully resolved.');

packages/protocol/src/channels.ts

@@ -1748,7 +1748,6 @@ export type BrowserContextCreateTempFilesOptions = {
   rootDirName?: string,
 };
 export type BrowserContextCreateTempFilesResult = {
-  remoteDir: string,
   writableStreams: WritableStreamChannel[],
 };
 export type BrowserContextUpdateSubscriptionParams = {

packages/protocol/src/protocol.yml

@@ -1195,7 +1195,6 @@ BrowserContext:
               name: string
               lastModifiedMs: number?
       returns:
-        remoteDir: string
         writableStreams:
           type: array
           items: WritableStream

tests/page/page-set-input-files.spec.ts

@@ -83,6 +83,17 @@ it('should upload a folder and throw for multiple directories', async ({ page, s
   ])).rejects.toThrow('File paths must be all files or a single directory');
 });
 
+it('should throw when uploading a folder in a normal file upload input', async ({ page, server, browserName, headless, browserMajorVersion }) => {
+  await page.goto(server.PREFIX + '/input/fileupload.html');
+  const input = await page.$('input');
+  const dir = path.join(it.info().outputDir, 'file-upload-test');
+  {
+    await fs.promises.mkdir(path.join(dir), { recursive: true });
+    await fs.promises.writeFile(path.join(dir, 'file1.txt'), 'file1 content');
+  }
+  await expect(input.setInputFiles(dir)).rejects.toThrow('File input does not support directories, pass individual files instead');
+});
+
 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');