From 93a3c22f63767c6b22d06ec0e5133e8d55f012df Mon Sep 17 00:00:00 2001 From: Artur Riabtsun Date: Tue, 1 Aug 2023 15:36:34 +0200 Subject: [PATCH 1/3] fix(a11y): screen reader support for view only file preview --- src/lib/viewers/doc/DocBaseViewer.js | 14 +++++- .../doc/__tests__/DocBaseViewer-test.js | 49 +++++++++++++++---- src/lib/viewers/doc/_docBase.scss | 11 +++++ 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index d2bb33b1f..027e62f14 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -177,6 +177,12 @@ class DocBaseViewer extends BaseViewer { this.viewerEl = this.docEl.appendChild(document.createElement('div')); this.viewerEl.classList.add('pdfViewer'); + // Text layer should be rendered for a11y reasons thats why we will block user from selecting content when no download permissions was granted + const isViewOnly = !checkPermission(this.options.file, PERMISSION_DOWNLOAD); + if (isViewOnly) { + this.viewerEl.classList.add('viewOnly'); + } + this.loadTimeout = LOAD_TIMEOUT_MS; this.startPageNum = this.getStartPage(this.startAt); @@ -795,7 +801,9 @@ class DocBaseViewer extends BaseViewer { const { AnnotationMode: PDFAnnotationMode = {} } = this.pdfjsLib; const assetUrlCreator = createAssetUrlCreator(location); const hasDownload = checkPermission(file, PERMISSION_DOWNLOAD); - const hasTextLayer = hasDownload && !this.getViewerOption('disableTextLayer'); + const enabledTextLayerMode = hasDownload + ? PDFJS_TEXT_LAYER_MODE.ENABLE + : PDFJS_TEXT_LAYER_MODE.ENABLE_PERMISSIONS; // This mode will prevent default behavior for copy events in the TextLayerBuilder return new PdfViewerClass({ annotationMode: PDFAnnotationMode.ENABLE, // Show annotations, but not forms @@ -806,7 +814,9 @@ class DocBaseViewer extends BaseViewer { linkService: this.pdfLinkService, maxCanvasPixels: this.isMobile ? MOBILE_MAX_CANVAS_SIZE : -1, renderInteractiveForms: false, // Enabling prevents unverified signatures from being displayed - textLayerMode: hasTextLayer ? PDFJS_TEXT_LAYER_MODE.ENABLE : PDFJS_TEXT_LAYER_MODE.DISABLE, + textLayerMode: this.getViewerOption('disableTextLayer') + ? PDFJS_TEXT_LAYER_MODE.DISABLE + : enabledTextLayerMode, }); } diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 08b629028..f7be87bc7 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -276,6 +276,35 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { docBase.setup(); expect(docBase.pageTracker).toBeInstanceOf(PageTracker); }); + test('should add view only class if user does not have download permissions', () => { + docBase = new DocBaseViewer({ + file: { + id: '0', + }, + }); + docBase.containerEl = containerEl; + + jest.spyOn(file, 'checkPermission').mockReturnValue(false); + docBase.setup(); + + expect(file.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); + expect(docBase.viewerEl).toHaveClass('viewOnly'); + }); + + test('should not add view only class if user has download permissions', () => { + docBase = new DocBaseViewer({ + file: { + id: '0', + }, + }); + docBase.containerEl = containerEl; + + jest.spyOn(file, 'checkPermission').mockReturnValue(true); + docBase.setup(); + + expect(file.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); + expect(docBase.viewerEl).not.toHaveClass('viewOnly'); + }); }); describe('Non setup methods', () => { @@ -1140,43 +1169,45 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }); }); - test('should enable the text layer based on download permissions', () => { + test('should enable the text layer if the user is on mobile', () => { + docBase.isMobile = true; stubs.checkPermission.mockReturnValueOnce(true); return docBase.initViewer('').then(() => { - expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); + expect(stubs.getViewerOption).toBeCalledWith('disableTextLayer'); // Text Layer mode 1 = enabled expect(stubs.pdfViewerClass).toBeCalledWith(expect.objectContaining({ textLayerMode: 1 })); }); }); - test('should enable the text layer if the user is on mobile', () => { - docBase.isMobile = true; + test('should use proper text layer mode if user has download permissions and disableTextLayer viewer option is not set', () => { + stubs.getViewerOption.mockReturnValue(false); stubs.checkPermission.mockReturnValueOnce(true); return docBase.initViewer('').then(() => { expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); + expect(stubs.getViewerOption).toBeCalledWith('disableTextLayer'); // Text Layer mode 1 = enabled expect(stubs.pdfViewerClass).toBeCalledWith(expect.objectContaining({ textLayerMode: 1 })); }); }); - test('should disable the text layer based on download permissions', () => { + test('should use proper text layer mode if user does not have download permissions and disableTextLayer viewer option is not set', () => { + stubs.getViewerOption.mockReturnValue(false); stubs.checkPermission.mockReturnValueOnce(false); return docBase.initViewer('').then(() => { expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); - // Text Layer mode 0 = disabled - expect(stubs.pdfViewerClass).toBeCalledWith(expect.objectContaining({ textLayerMode: 0 })); + expect(stubs.getViewerOption).toBeCalledWith('disableTextLayer'); + // Text Layer mode 2 = enabled permissions + expect(stubs.pdfViewerClass).toBeCalledWith(expect.objectContaining({ textLayerMode: 2 })); }); }); test('should disable the text layer if disableTextLayer viewer option is set', () => { - stubs.checkPermission.mockReturnValueOnce(true); stubs.getViewerOption.mockReturnValue(true); return docBase.initViewer('').then(() => { - expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); expect(stubs.getViewerOption).toBeCalledWith('disableTextLayer'); // Text Layer mode 0 = disabled expect(stubs.pdfViewerClass).toBeCalledWith(expect.objectContaining({ textLayerMode: 0 })); diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index bb1d28c20..e5a545f0f 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -304,6 +304,17 @@ $thumbnail-sidebar-width: 191px; // Extra pixel to account for sidebar border } } + .pdfViewer { + &.viewOnly { + .textLayer { + span { + cursor: not-allowed; + user-select: none; + } + } + } + } + .textLayer { caret-color: black; // Required for caret navigation on transparent text top: $pdfjs-page-padding; From e8cb1ab3aac3799d0ed4a7aac76333bebfc16f0f Mon Sep 17 00:00:00 2001 From: Artur Riabtsun Date: Wed, 2 Aug 2023 12:13:36 +0200 Subject: [PATCH 2/3] fix(a11y): resolve comments --- src/lib/viewers/doc/DocBaseViewer.js | 15 +++--- .../doc/__tests__/DocBaseViewer-test.js | 49 ++++++++----------- src/lib/viewers/doc/_docBase.scss | 4 +- 3 files changed, 29 insertions(+), 39 deletions(-) diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 027e62f14..3d20e9ce5 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -177,11 +177,6 @@ class DocBaseViewer extends BaseViewer { this.viewerEl = this.docEl.appendChild(document.createElement('div')); this.viewerEl.classList.add('pdfViewer'); - // Text layer should be rendered for a11y reasons thats why we will block user from selecting content when no download permissions was granted - const isViewOnly = !checkPermission(this.options.file, PERMISSION_DOWNLOAD); - if (isViewOnly) { - this.viewerEl.classList.add('viewOnly'); - } this.loadTimeout = LOAD_TIMEOUT_MS; @@ -801,10 +796,16 @@ class DocBaseViewer extends BaseViewer { const { AnnotationMode: PDFAnnotationMode = {} } = this.pdfjsLib; const assetUrlCreator = createAssetUrlCreator(location); const hasDownload = checkPermission(file, PERMISSION_DOWNLOAD); + const hasTextLayer = !this.getViewerOption('disableTextLayer'); const enabledTextLayerMode = hasDownload ? PDFJS_TEXT_LAYER_MODE.ENABLE : PDFJS_TEXT_LAYER_MODE.ENABLE_PERMISSIONS; // This mode will prevent default behavior for copy events in the TextLayerBuilder + // Text layer should be rendered for a11y reasons thats why we will block user from selecting content when no download permissions was granted + if (!hasDownload) { + this.viewerEl.classList.add('pdfViewer--viewOnly'); + } + return new PdfViewerClass({ annotationMode: PDFAnnotationMode.ENABLE, // Show annotations, but not forms container: this.docEl, @@ -814,9 +815,7 @@ class DocBaseViewer extends BaseViewer { linkService: this.pdfLinkService, maxCanvasPixels: this.isMobile ? MOBILE_MAX_CANVAS_SIZE : -1, renderInteractiveForms: false, // Enabling prevents unverified signatures from being displayed - textLayerMode: this.getViewerOption('disableTextLayer') - ? PDFJS_TEXT_LAYER_MODE.DISABLE - : enabledTextLayerMode, + textLayerMode: hasTextLayer ? enabledTextLayerMode : PDFJS_TEXT_LAYER_MODE.DISABLE, }); } diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index f7be87bc7..815f18ac2 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -276,35 +276,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { docBase.setup(); expect(docBase.pageTracker).toBeInstanceOf(PageTracker); }); - test('should add view only class if user does not have download permissions', () => { - docBase = new DocBaseViewer({ - file: { - id: '0', - }, - }); - docBase.containerEl = containerEl; - - jest.spyOn(file, 'checkPermission').mockReturnValue(false); - docBase.setup(); - - expect(file.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); - expect(docBase.viewerEl).toHaveClass('viewOnly'); - }); - - test('should not add view only class if user has download permissions', () => { - docBase = new DocBaseViewer({ - file: { - id: '0', - }, - }); - docBase.containerEl = containerEl; - - jest.spyOn(file, 'checkPermission').mockReturnValue(true); - docBase.setup(); - - expect(file.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); - expect(docBase.viewerEl).not.toHaveClass('viewOnly'); - }); }); describe('Non setup methods', () => { @@ -1180,6 +1151,26 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }); }); + test('should add view only class if user does not have download permissions', () => { + docBase.containerEl = containerEl; + stubs.checkPermission.mockReturnValueOnce(false); + + return docBase.initViewer('').then(() => { + expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); + expect(docBase.viewerEl).toHaveClass('pdfViewer--viewOnly'); + }); + }); + + test('should not add view only class if user has download permissions', () => { + docBase.containerEl = containerEl; + stubs.checkPermission.mockReturnValueOnce(true); + + return docBase.initViewer('').then(() => { + expect(stubs.checkPermission).toBeCalledWith(docBase.options.file, PERMISSION_DOWNLOAD); + expect(docBase.viewerEl).not.toHaveClass('pdfViewer--viewOnly'); + }); + }); + test('should use proper text layer mode if user has download permissions and disableTextLayer viewer option is not set', () => { stubs.getViewerOption.mockReturnValue(false); stubs.checkPermission.mockReturnValueOnce(true); diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index e5a545f0f..1cbc70991 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -305,10 +305,10 @@ $thumbnail-sidebar-width: 191px; // Extra pixel to account for sidebar border } .pdfViewer { - &.viewOnly { + &--viewOnly { .textLayer { span { - cursor: not-allowed; + cursor: unset; user-select: none; } } From d5082ecff0ebe90d1de78370b47bbcd62eee33f6 Mon Sep 17 00:00:00 2001 From: Artur Riabtsun Date: Wed, 2 Aug 2023 18:55:00 +0200 Subject: [PATCH 3/3] fix(a11y): resolve comments --- src/lib/viewers/doc/_docBase.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/viewers/doc/_docBase.scss b/src/lib/viewers/doc/_docBase.scss index 1cbc70991..ddbc5d80d 100644 --- a/src/lib/viewers/doc/_docBase.scss +++ b/src/lib/viewers/doc/_docBase.scss @@ -305,7 +305,7 @@ $thumbnail-sidebar-width: 191px; // Extra pixel to account for sidebar border } .pdfViewer { - &--viewOnly { + &.pdfViewer--viewOnly { .textLayer { span { cursor: unset;