Merge pull request #5180 from overleaf/msm-pdf-js-viewer-tests

Unit Tests for `pdf-js-viewer.js`

GitOrigin-RevId: 4bd38cf98c598e2ea30791052b9c24568d92c6b8

services/web/frontend/js/features/pdf-preview/components/pdf-js-viewer.js

@@ -33,15 +33,19 @@ function PdfJsViewer({ url }) {
 
   // create the viewer when the container is mounted
   const handleContainer = useCallback(parent => {
-    setPdfJsWrapper(parent ? new PDFJSWrapper(parent.firstChild) : undefined)
+    if (parent) {
+      const viewer = new PDFJSWrapper(parent.firstChild)
+      setPdfJsWrapper(viewer)
+      return () => viewer.destroy()
+    }
   }, [])
 
   // listen for initialize event
   useEffect(() => {
     if (pdfJsWrapper) {
-      pdfJsWrapper.eventBus.on('pagesinit', () => {
-        setInitialised(true)
-      })
+      const handlePagesinit = () => setInitialised(true)
+      pdfJsWrapper.eventBus.on('pagesinit', handlePagesinit)
+      return () => pdfJsWrapper.eventBus.off('pagesinit', handlePagesinit)
     }
   }, [pdfJsWrapper])
 
@@ -53,6 +57,7 @@ function PdfJsViewer({ url }) {
       // TODO: anything else to be reset?
 
       pdfJsWrapper.loadDocument(url).catch(error => setError(error))
+      return () => pdfJsWrapper.abortDocumentLoading()
     }
   }, [pdfJsWrapper, url])
 
@@ -73,21 +78,22 @@ function PdfJsViewer({ url }) {
 
   // listen for scroll events
   useEffect(() => {
-    if (pdfJsWrapper) {
+    if (initialised && pdfJsWrapper) {
       // store the scroll position in localStorage, for the synctex button
       const storePosition = debounce(pdfViewer => {
         // set position for "sync to code" button
         try {
           setPosition(pdfViewer.currentPosition)
         } catch (error) {
-          // console.error(error) // TODO
+          // TODO
+          console.error(error)
         }
       }, 500)
 
       // store the scroll position in localStorage, for use when reloading
       const storeScrollTop = debounce(pdfViewer => {
         // set position for "sync to code" button
-        setScrollTop(pdfJsWrapper.container.scrollTop)
+        setScrollTop(pdfViewer.container.scrollTop)
       }, 500)
 
       storePosition(pdfJsWrapper)
@@ -100,15 +106,17 @@ function PdfJsViewer({ url }) {
       pdfJsWrapper.container.addEventListener('scroll', scrollListener)
 
       return () => {
+        storePosition.cancel()
+        storeScrollTop.cancel()
         pdfJsWrapper.container.removeEventListener('scroll', scrollListener)
       }
     }
-  }, [setPosition, setScrollTop, pdfJsWrapper])
+  }, [setPosition, setScrollTop, pdfJsWrapper, initialised])
 
   // listen for double-click events
   useEffect(() => {
     if (pdfJsWrapper) {
-      pdfJsWrapper.eventBus.on('textlayerrendered', textLayer => {
+      const handleTextlayerrendered = textLayer => {
         const pageElement = textLayer.source.textLayerDiv.closest('.page')
 
         const doubleClickListener = event => {
@@ -120,7 +128,11 @@ function PdfJsViewer({ url }) {
         }
 
         pageElement.addEventListener('dblclick', doubleClickListener)
-      })
+      }
+
+      pdfJsWrapper.eventBus.on('textlayerrendered', handleTextlayerrendered)
+      return () =>
+        pdfJsWrapper.eventBus.off('textlayerrendered', handleTextlayerrendered)
     }
   }, [pdfJsWrapper])
 

services/web/frontend/js/features/pdf-preview/util/pdf-js-wrapper.js

@@ -32,7 +32,7 @@ export default class PDFJSWrapper {
     })
 
     // create the localization
-    const l10n = new PDFJSViewer.GenericL10n('en-GB') // TODO: locale mapping?
+    // const l10n = new PDFJSViewer.GenericL10n('en-GB') // TODO: locale mapping?
 
     // create the viewer
     const viewer = new PDFJSViewer.PDFViewer({
@@ -40,7 +40,7 @@ export default class PDFJSWrapper {
       eventBus,
       imageResourcesPath,
       linkService,
-      l10n,
+      // l10n, // commented out since it currently breaks `aria-label` rendering in pdf pages
       enableScripting: false, // default is false, but set explicitly to be sure
       renderInteractiveForms: false,
     })
@@ -53,22 +53,48 @@ export default class PDFJSWrapper {
   }
 
   // load a document from a URL
-  async loadDocument(url) {
-    const doc = await PDFJS.getDocument({
-      url,
-      cMapUrl,
-      cMapPacked: true,
-      disableFontFace,
-      rangeChunkSize,
-      disableAutoFetch: true,
-      disableStream: true,
-      textLayerMode: 2, // PDFJSViewer.TextLayerMode.ENABLE,
-    }).promise
-
-    this.viewer.setDocument(doc)
-    this.linkService.setDocument(doc)
-
-    return doc
+  loadDocument(url) {
+    // prevents any previous loading task from populating the viewer
+    this.loadDocumentTask = undefined
+
+    return new Promise((resolve, reject) => {
+      this.loadDocumentTask = PDFJS.getDocument({
+        url,
+        cMapUrl,
+        cMapPacked: true,
+        disableFontFace,
+        rangeChunkSize,
+        disableAutoFetch: true,
+        disableStream: true,
+        textLayerMode: 2, // PDFJSViewer.TextLayerMode.ENABLE,
+      })
+
+      this.loadDocumentTask.promise
+        .then(doc => {
+          if (!this.loadDocumentTask) {
+            return // ignoring the response since loading task has been aborted
+          }
+
+          const previousDoc = this.viewer.pdfDocument
+
+          this.viewer.setDocument(doc)
+          this.linkService.setDocument(doc)
+          resolve(doc)
+
+          if (previousDoc) {
+            previousDoc.cleanup().catch(console.error)
+            previousDoc.destroy()
+          }
+        })
+        .catch(error => {
+          if (this.loadDocumentTask) {
+            reject(error)
+          }
+        })
+        .finally(() => {
+          this.loadDocumentTask = undefined
+        })
+    })
   }
 
   // update the current scale value if the container size changes
@@ -123,4 +149,16 @@ export default class PDFJSWrapper {
       pageSize: { height, width },
     }
   }
+
+  abortDocumentLoading() {
+    this.loadDocumentTask = undefined
+  }
+
+  destroy() {
+    if (this.loadDocumentTask) {
+      this.loadDocumentTask.destroy()
+      this.loadDocumentTask = undefined
+    }
+    this.viewer.destroy()
+  }
 }

services/web/test/frontend/features/pdf-preview/components/pdf-js-viewer.test.js

@@ -0,0 +1,83 @@
+import { expect } from 'chai'
+import { screen } from '@testing-library/react'
+import path from 'path'
+import { renderWithEditorContext } from '../../../helpers/render-with-context'
+import { pathToFileURL } from 'url'
+import PdfJsViewer from '../../../../../frontend/js/features/pdf-preview/components/pdf-js-viewer'
+
+const example = pathToFileURL(
+  path.join(__dirname, '../fixtures/test-example.pdf').toString()
+)
+
+const exampleCorrupt = pathToFileURL(
+  path.join(__dirname, '../fixtures/test-example-corrupt.pdf')
+).toString()
+
+const invalidURL = 'http://nonexisting.com/doc'
+
+describe('<PdfJSViewer/>', function () {
+  it('loads all PDF pages', async function () {
+    renderWithEditorContext(<PdfJsViewer url={example} />)
+
+    await screen.findByLabelText('Page 1')
+    await screen.findByLabelText('Page 2')
+    await screen.findByLabelText('Page 3')
+    expect(screen.queryByLabelText('Page 4')).to.not.exist
+  })
+
+  it('renders pages in a "loading" state', async function () {
+    renderWithEditorContext(<PdfJsViewer url={example} />)
+    await screen.findByLabelText('Loading…')
+  })
+
+  it('can be unmounted while loading a document', async function () {
+    const { unmount } = renderWithEditorContext(<PdfJsViewer url={example} />)
+    unmount()
+  })
+
+  it('can be unmounted after loading a document', async function () {
+    const { unmount } = renderWithEditorContext(<PdfJsViewer url={example} />)
+    await screen.findByLabelText('Page 1')
+    unmount()
+  })
+
+  describe('with an invalid URL', function () {
+    it('renders an error alert', async function () {
+      renderWithEditorContext(<PdfJsViewer url={invalidURL} />)
+      await screen.findByRole('alert')
+      expect(screen.queryByLabelText('Page 1')).to.not.exist
+    })
+
+    it('can load another document after the error', async function () {
+      const { rerender } = renderWithEditorContext(
+        <PdfJsViewer url={invalidURL} />
+      )
+      await screen.findByRole('alert')
+
+      rerender(<PdfJsViewer url={example} />)
+
+      await screen.findByLabelText('Page 1')
+      expect(screen.queryByRole('alert')).to.not.exist
+    })
+  })
+
+  describe('with an corrupted document', function () {
+    it('renders an error alert', async function () {
+      renderWithEditorContext(<PdfJsViewer url={exampleCorrupt} />)
+      await screen.findByRole('alert')
+      expect(screen.queryByLabelText('Page 1')).to.not.exist
+    })
+
+    it('can load another document after the error', async function () {
+      const { rerender } = renderWithEditorContext(
+        <PdfJsViewer url={exampleCorrupt} />
+      )
+      await screen.findByRole('alert')
+
+      rerender(<PdfJsViewer url={example} />)
+
+      await screen.findByLabelText('Page 1')
+      expect(screen.queryByRole('alert')).to.not.exist
+    })
+  })
+})