fixes a few minor issues with playback in xtermjs (#142172)

Co-authored-by: Karl Godard <[email protected]>
elastic · Sep 29, 2022 · 07495a4 · 07495a4
1 parent 92b686a
commit 07495a4
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 11 deletions.
diff --git a/x-pack/plugins/session_view/public/components/tty_player/hooks.test.tsx b/x-pack/plugins/session_view/public/components/tty_player/hooks.test.tsx
@@ -167,6 +167,21 @@ describe('TTYPlayer/hooks', () => {
       expect(result.current.currentLine).toBe(initialProps.lines.length - 1);
     });
 
+    it('should not print the first line twice after playback starts', async () => {
+      const { result, rerender } = renderHook((props) => useXtermPlayer(props), {
+        initialProps,
+      });
+
+      rerender({ ...initialProps, isPlaying: true });
+      act(() => {
+        // advance render loop
+        jest.advanceTimersByTime(DEFAULT_TTY_PLAYSPEED_MS);
+      });
+      rerender({ ...initialProps, isPlaying: false });
+
+      expect(result.current.terminal.buffer.active.getLine(0)?.translateToString(true)).toBe('256');
+    });
+
     it('will allow a plain text search highlight on the last line printed', async () => {
       const { result: xTermResult } = renderHook((props) => useXtermPlayer(props), {
         initialProps,

diff --git a/x-pack/plugins/session_view/public/components/tty_player/hooks.ts b/x-pack/plugins/session_view/public/components/tty_player/hooks.ts
@@ -281,13 +281,12 @@ export const useXtermPlayer = ({
   useEffect(() => {
     if (isPlaying) {
       const timer = setTimeout(() => {
-        render(currentLine, false);
-
-        if (currentLine === lines.length - 1) {
+        if (!hasNextPage && currentLine === lines.length - 1) {
           setIsPlaying(false);
         } else {
           const nextLine = Math.min(lines.length - 1, currentLine + TTY_LINES_PER_FRAME);
           setCurrentLine(nextLine);
+          render(nextLine, false);
         }
       }, playSpeed);
 

diff --git a/x-pack/plugins/session_view/public/components/tty_player/index.tsx b/x-pack/plugins/session_view/public/components/tty_player/index.tsx
@@ -134,7 +134,12 @@ export const TTYPlayer = ({
             <EuiBetaBadge label={BETA} size="s" css={styles.betaBadge} />
           </EuiFlexItem>
           <EuiFlexItem data-test-subj="sessionView:TTYSearch">
-            <TTYSearchBar lines={lines} seekToLine={seekToLine} xTermSearchFn={search} />
+            <TTYSearchBar
+              lines={lines}
+              seekToLine={seekToLine}
+              xTermSearchFn={search}
+              setIsPlaying={setIsPlaying}
+            />
           </EuiFlexItem>
 
           <EuiFlexItem grow={false}>

diff --git a/x-pack/plugins/session_view/public/components/tty_search_bar/index.test.tsx b/x-pack/plugins/session_view/public/components/tty_search_bar/index.test.tsx
@@ -34,6 +34,7 @@ describe('TTYSearchBar component', () => {
       lines,
       seekToLine: jest.fn(),
       xTermSearchFn: jest.fn(),
+      setIsPlaying: jest.fn(),
     };
   });
 
@@ -59,6 +60,7 @@ describe('TTYSearchBar component', () => {
     expect(props.xTermSearchFn).toHaveBeenCalledTimes(2);
     expect(props.xTermSearchFn).toHaveBeenNthCalledWith(1, '', 0);
     expect(props.xTermSearchFn).toHaveBeenNthCalledWith(2, '-h', 6);
+    expect(props.setIsPlaying).toHaveBeenCalledWith(false);
   });
 
   it('calls seekToline and xTermSearchFn when currentMatch changes', async () => {
@@ -85,6 +87,7 @@ describe('TTYSearchBar component', () => {
     expect(props.xTermSearchFn).toHaveBeenNthCalledWith(1, '', 0);
     expect(props.xTermSearchFn).toHaveBeenNthCalledWith(2, '-h', 6);
     expect(props.xTermSearchFn).toHaveBeenNthCalledWith(3, '-h', 13);
+    expect(props.setIsPlaying).toHaveBeenCalledTimes(3);
   });
 
   it('calls xTermSearchFn with empty query when search is cleared', async () => {
@@ -101,5 +104,6 @@ describe('TTYSearchBar component', () => {
     await new Promise((r) => setTimeout(r, 100));
 
     expect(props.xTermSearchFn).toHaveBeenNthCalledWith(3, '', 0);
+    expect(props.setIsPlaying).toHaveBeenCalledWith(false);
   });
 });
diff --git a/x-pack/plugins/session_view/public/components/tty_search_bar/index.tsx b/x-pack/plugins/session_view/public/components/tty_search_bar/index.tsx
@@ -19,15 +19,24 @@ export interface TTYSearchBarDeps {
   lines: IOLine[];
   seekToLine(index: number): void;
   xTermSearchFn(query: string, index: number): void;
+  setIsPlaying(value: boolean): void;
 }
 
-export const TTYSearchBar = ({ lines, seekToLine, xTermSearchFn }: TTYSearchBarDeps) => {
+const STRIP_NEWLINES_REGEX = /^(\r\n|\r|\n|\n\r)/;
+
+export const TTYSearchBar = ({
+  lines,
+  seekToLine,
+  xTermSearchFn,
+  setIsPlaying,
+}: TTYSearchBarDeps) => {
   const [currentMatch, setCurrentMatch] = useState<SearchResult | null>(null);
   const [searchQuery, setSearchQuery] = useState('');
 
   const jumpToMatch = useCallback(
     (match) => {
       if (match) {
+        setIsPlaying(false);
         const goToLine = lines.indexOf(match.line);
         seekToLine(goToLine);
       }
@@ -40,7 +49,7 @@ export const TTYSearchBar = ({ lines, seekToLine, xTermSearchFn }: TTYSearchBarD
         clearTimeout(timeout);
       };
     },
-    [lines, seekToLine, xTermSearchFn, searchQuery]
+    [setIsPlaying, lines, seekToLine, xTermSearchFn, searchQuery]
   );
 
   const searchResults = useMemo(() => {
@@ -53,7 +62,7 @@ export const TTYSearchBar = ({ lines, seekToLine, xTermSearchFn }: TTYSearchBarD
           const cursorMovement = current.value.match(/^\x1b\[\d+;(\d+)(H|d)/);
           const regex = new RegExp(searchQuery.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'ig');
           const lineMatches = stripAnsi(current.value)
-            .replace(/^\r|\r?\n/, '')
+            .replace(STRIP_NEWLINES_REGEX, '')
             .matchAll(regex);
 
           if (lineMatches) {
@@ -90,10 +99,14 @@ export const TTYSearchBar = ({ lines, seekToLine, xTermSearchFn }: TTYSearchBarD
     return matches;
   }, [searchQuery, lines, jumpToMatch, xTermSearchFn]);
 
-  const onSearch = useCallback((query) => {
-    setSearchQuery(query);
-    setCurrentMatch(null);
-  }, []);
+  const onSearch = useCallback(
+    (query) => {
+      setIsPlaying(false);
+      setSearchQuery(query);
+      setCurrentMatch(null);
+    },
+    [setIsPlaying]
+  );
 
   const onSetCurrentMatch = useCallback(
     (index) => {