From 98982efa76deb909b0c1c4b507a76326f3b70c30 Mon Sep 17 00:00:00 2001
From: Pavel Feldman <pavel.feldman@gmail.com>
Date: Mon, 14 Oct 2024 11:07:52 -0700
Subject: [PATCH] chore: fix codegen selector while debugging

---
 .../playwright-core/src/server/recorder.ts    |  4 ++-
 .../src/server/recorder/recorderApp.ts        | 10 ++++----
 .../src/server/recorder/recorderFrontend.ts   |  2 +-
 .../server/recorder/recorderInTraceViewer.ts  |  4 +--
 packages/recorder/src/recorder.tsx            | 25 +++++++++++--------
 packages/recorder/src/recorderTypes.ts        |  2 +-
 packages/web/src/components/sourceChooser.tsx | 24 ++++++++++--------
 packages/web/src/components/toolbarButton.tsx |  3 +++
 tests/library/inspector/pause.spec.ts         | 16 ++++++++++++
 9 files changed, 60 insertions(+), 30 deletions(-)

diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts
index 4aab712b9bd642..293af11d6a3f04 100644
--- a/packages/playwright-core/src/server/recorder.ts
+++ b/packages/playwright-core/src/server/recorder.ts
@@ -140,6 +140,8 @@ export class Recorder implements InstrumentationListener, IRecorder {
     this._contextRecorder.on(ContextRecorder.Events.Change, (data: { sources: Source[], actions: actions.ActionInContext[] }) => {
       this._recorderSources = data.sources;
       recorderApp.setActions(data.actions, data.sources);
+      if (data.sources[0])
+        recorderApp.setFile(data.sources[0].id, 'codegen');
       this._pushAllSources();
     });
 
@@ -299,7 +301,7 @@ export class Recorder implements InstrumentationListener, IRecorder {
     }
     this._pushAllSources();
     if (fileToSelect)
-      this._recorderApp?.setFile(fileToSelect);
+      this._recorderApp?.setFile(fileToSelect, 'breakpoint');
   }
 
   private _pushAllSources() {
diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts
index 30149f9816ca2b..7baf91885e9353 100644
--- a/packages/playwright-core/src/server/recorder/recorderApp.ts
+++ b/packages/playwright-core/src/server/recorder/recorderApp.ts
@@ -34,7 +34,7 @@ export class EmptyRecorderApp extends EventEmitter implements IRecorderApp {
   async close(): Promise<void> {}
   async setPaused(paused: boolean): Promise<void> {}
   async setMode(mode: Mode): Promise<void> {}
-  async setFile(file: string): Promise<void> {}
+  async setFile(file: string, mode: 'breakpoint' | 'codegen'): Promise<void> {}
   async setSelector(selector: string, userGesture?: boolean): Promise<void> {}
   async updateCallLogs(callLogs: CallLog[]): Promise<void> {}
   async setSources(sources: Source[]): Promise<void> {}
@@ -131,10 +131,10 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
     }).toString(), { isFunction: true }, mode).catch(() => {});
   }
 
-  async setFile(file: string): Promise<void> {
-    await this._page.mainFrame().evaluateExpression(((file: string) => {
-      window.playwrightSetFile(file);
-    }).toString(), { isFunction: true }, file).catch(() => {});
+  async setFile(file: string, mode: 'breakpoint' | 'codegen'): Promise<void> {
+    await this._page.mainFrame().evaluateExpression(((params: { file: string, mode: 'breakpoint' | 'codegen' }) => {
+      window.playwrightSetFile(params.file, params.mode);
+    }).toString(), { isFunction: true }, { file, mode }).catch(() => {});
   }
 
   async setPaused(paused: boolean): Promise<void> {
diff --git a/packages/playwright-core/src/server/recorder/recorderFrontend.ts b/packages/playwright-core/src/server/recorder/recorderFrontend.ts
index 97df1d3ceb6c49..f856bd08d55f7b 100644
--- a/packages/playwright-core/src/server/recorder/recorderFrontend.ts
+++ b/packages/playwright-core/src/server/recorder/recorderFrontend.ts
@@ -28,7 +28,7 @@ export interface IRecorderApp extends EventEmitter {
   close(): Promise<void>;
   setPaused(paused: boolean): Promise<void>;
   setMode(mode: Mode): Promise<void>;
-  setFile(file: string): Promise<void>;
+  setFile(file: string, mode: 'breakpoint' | 'codegen'): Promise<void>;
   setSelector(selector: string, userGesture?: boolean): Promise<void>;
   updateCallLogs(callLogs: CallLog[]): Promise<void>;
   setSources(sources: Source[]): Promise<void>;
diff --git a/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts b/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts
index ab67fe562cf7f3..6ab381fc2f5b0b 100644
--- a/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts
+++ b/packages/playwright-core/src/server/recorder/recorderInTraceViewer.ts
@@ -66,8 +66,8 @@ export class RecorderInTraceViewer extends EventEmitter implements IRecorderApp
     this._transport.deliverEvent('setMode', { mode });
   }
 
-  async setFile(file: string): Promise<void> {
-    this._transport.deliverEvent('setFileIfNeeded', { file });
+  async setFile(file: string, mode: 'breakpoint' | 'codegen'): Promise<void> {
+    this._transport.deliverEvent('setFile', { file, mode });
   }
 
   async setSelector(selector: string, userGesture?: boolean): Promise<void> {
diff --git a/packages/recorder/src/recorder.tsx b/packages/recorder/src/recorder.tsx
index 9d5c0feebac946..11d0af176ced2a 100644
--- a/packages/recorder/src/recorder.tsx
+++ b/packages/recorder/src/recorder.tsx
@@ -41,13 +41,11 @@ export const Recorder: React.FC<RecorderProps> = ({
   log,
   mode,
 }) => {
-  const [fileId, setFileId] = React.useState<string | undefined>();
+  const [preferredLanguage, setPreferredLanguage] = React.useState<string | undefined>();
+  const [selectedFileId, setSelectedFileId] = React.useState<string | undefined>();
   const [selectedTab, setSelectedTab] = React.useState<string>('log');
 
-  React.useEffect(() => {
-    if (!fileId && sources.length > 0)
-      setFileId(sources[0].id);
-  }, [fileId, sources]);
+  const fileId = selectedFileId || preferredLanguage || sources[0]?.id;
 
   const source = React.useMemo(() => {
     if (fileId) {
@@ -66,7 +64,14 @@ export const Recorder: React.FC<RecorderProps> = ({
     setLocator(asLocator(language, selector));
   };
 
-  window.playwrightSetFile = setFileId;
+  window.playwrightSetFile = React.useCallback((fileId: string, mode: 'breakpoint' | 'codegen') => {
+    if (mode === 'breakpoint') {
+      setSelectedFileId(fileId);
+    } else {
+      setPreferredLanguage(fileId);
+      setSelectedFileId(undefined);
+    }
+  }, []);
 
   const messagesEndRef = React.useRef<HTMLDivElement>(null);
   React.useLayoutEffect(() => {
@@ -134,19 +139,19 @@ export const Recorder: React.FC<RecorderProps> = ({
       <ToolbarButton icon='files' title='Copy' disabled={!source || !source.text} onClick={() => {
         copy(source.text);
       }}></ToolbarButton>
-      <ToolbarButton icon='debug-continue' title='Resume (F8)' disabled={!paused} onClick={() => {
+      <ToolbarButton icon='debug-continue' title='Resume (F8)' ariaLabel='Resume' disabled={!paused} onClick={() => {
         window.dispatch({ event: 'resume' });
       }}></ToolbarButton>
-      <ToolbarButton icon='debug-pause' title='Pause (F8)' disabled={paused} onClick={() => {
+      <ToolbarButton icon='debug-pause' title='Pause (F8)' ariaLabel='Pause' disabled={paused} onClick={() => {
         window.dispatch({ event: 'pause' });
       }}></ToolbarButton>
-      <ToolbarButton icon='debug-step-over' title='Step over (F10)' disabled={!paused} onClick={() => {
+      <ToolbarButton icon='debug-step-over' title='Step over (F10)' ariaLabel='Step over' disabled={!paused} onClick={() => {
         window.dispatch({ event: 'step' });
       }}></ToolbarButton>
       <div style={{ flex: 'auto' }}></div>
       <div>Target:</div>
       <SourceChooser fileId={fileId} sources={sources} setFileId={fileId => {
-        setFileId(fileId);
+        setSelectedFileId(fileId);
         window.dispatch({ event: 'fileChanged', params: { file: fileId } });
       }} />
       <ToolbarButton icon='clear-all' title='Clear' disabled={!source || !source.text} onClick={() => {
diff --git a/packages/recorder/src/recorderTypes.ts b/packages/recorder/src/recorderTypes.ts
index a5791e2306522c..c65fe874862a96 100644
--- a/packages/recorder/src/recorderTypes.ts
+++ b/packages/recorder/src/recorderTypes.ts
@@ -96,7 +96,7 @@ declare global {
     playwrightSetSources: (sources: Source[]) => void;
     playwrightSetOverlayVisible: (visible: boolean) => void;
     playwrightUpdateLogs: (callLogs: CallLog[]) => void;
-    playwrightSetFile: (file: string) => void;
+    playwrightSetFile: (file: string, mode: 'breakpoint' | 'codegen') => void;
     playwrightSetSelector: (selector: string, focus?: boolean) => void;
     playwrightSourcesEchoForTest: Source[];
     dispatch(data: any): Promise<void>;
diff --git a/packages/web/src/components/sourceChooser.tsx b/packages/web/src/components/sourceChooser.tsx
index 0645480a03c65e..22b91c61a511b3 100644
--- a/packages/web/src/components/sourceChooser.tsx
+++ b/packages/web/src/components/sourceChooser.tsx
@@ -22,7 +22,7 @@ export const SourceChooser: React.FC<{
   fileId: string | undefined,
   setFileId: (fileId: string) => void,
 }> = ({ sources, fileId, setFileId }) => {
-  return <select className='source-chooser' hidden={!sources.length} value={fileId} onChange={event => {
+  return <select className='source-chooser' hidden={!sources.length} title='Source chooser' value={fileId} onChange={event => {
     setFileId(event.target.selectedOptions[0].value);
   }}>{renderSourceOptions(sources)}</select>;
 };
@@ -33,17 +33,21 @@ function renderSourceOptions(sources: Source[]): React.ReactNode {
     <option key={source.id} value={source.id}>{transformTitle(source.label)}</option>
   );
 
-  const hasGroup = sources.some(s => s.group);
-  if (hasGroup) {
-    const groups = new Set(sources.map(s => s.group));
-    return [...groups].filter(Boolean).map(group => (
-      <optgroup label={group} key={group}>
-        {sources.filter(s => s.group === group).map(source => renderOption(source))}
-      </optgroup>
-    ));
+  const sourcesByGroups = new Map<string, Source[]>();
+  for (const source of sources) {
+    let list = sourcesByGroups.get(source.group || 'Debugger');
+    if (!list) {
+      list = [];
+      sourcesByGroups.set(source.group || 'Debugger', list);
+    }
+    list.push(source);
   }
 
-  return sources.map(source => renderOption(source));
+  return [...sourcesByGroups.entries()].map(([group, sources]) => (
+    <optgroup label={group} key={group}>
+      {sources.filter(s => (s.group || 'Debugger') === group).map(source => renderOption(source))}
+    </optgroup>
+  ));
 }
 
 export function emptySource(): Source {
diff --git a/packages/web/src/components/toolbarButton.tsx b/packages/web/src/components/toolbarButton.tsx
index 00b9babd599401..184642b395e7f6 100644
--- a/packages/web/src/components/toolbarButton.tsx
+++ b/packages/web/src/components/toolbarButton.tsx
@@ -28,6 +28,7 @@ export interface ToolbarButtonProps {
   style?: React.CSSProperties,
   testId?: string,
   className?: string,
+  ariaLabel?: string,
 }
 
 export const ToolbarButton: React.FC<React.PropsWithChildren<ToolbarButtonProps>> = ({
@@ -40,6 +41,7 @@ export const ToolbarButton: React.FC<React.PropsWithChildren<ToolbarButtonProps>
   style,
   testId,
   className,
+  ariaLabel,
 }) => {
   return <button
     className={clsx(className, 'toolbar-button', icon, toggled && 'toggled')}
@@ -50,6 +52,7 @@ export const ToolbarButton: React.FC<React.PropsWithChildren<ToolbarButtonProps>
     disabled={!!disabled}
     style={style}
     data-testid={testId}
+    aria-label={ariaLabel}
   >
     {icon && <span className={`codicon codicon-${icon}`} style={children ? { marginRight: 5 } : {}}></span>}
     {children}
diff --git a/tests/library/inspector/pause.spec.ts b/tests/library/inspector/pause.spec.ts
index ac58fcf8bbf76b..405fffbe5b0c63 100644
--- a/tests/library/inspector/pause.spec.ts
+++ b/tests/library/inspector/pause.spec.ts
@@ -103,6 +103,7 @@ it.describe('pause', () => {
       await page.pause();
     })();
     const recorderPage = await recorderPageGetter();
+    await expect(recorderPage.getByRole('combobox', { name: 'Source chooser' })).toHaveValue(/pause\.spec\.ts/);
     const source = await recorderPage.textContent('.source-line-paused');
     expect(source).toContain('page.pause()');
     await recorderPage.click('[title="Resume (F8)"]');
@@ -480,6 +481,21 @@ it.describe('pause', () => {
     await recorderPage.click('[title="Resume (F8)"]');
     await scriptPromise;
   });
+
+  it('should record from debugger', async ({ page, recorderPageGetter }) => {
+    const scriptPromise = (async () => {
+      await page.pause();
+    })();
+    const recorderPage = await recorderPageGetter();
+    await expect(recorderPage.getByRole('combobox', { name: 'Source chooser' })).toHaveValue(/pause\.spec\.ts/);
+    await expect(recorderPage.locator('.source-line-paused')).toHaveText(/await page\.pause\(\)/);
+    await recorderPage.getByRole('button', { name: 'Record' }).click();
+    await page.locator('body').click();
+    await expect(recorderPage.getByRole('combobox', { name: 'Source chooser' })).toHaveValue('javascript');
+    await expect(recorderPage.locator('.cm-wrapper')).toContainText(`await page.locator('body').click();`);
+    await recorderPage.getByRole('button', { name: 'Resume' }).click();
+    await scriptPromise;
+  });
 });
 
 async function sanitizeLog(recorderPage: Page): Promise<string[]> {