From e9527464dba4e3e3543dfcfc3abe52327cbc2e44 Mon Sep 17 00:00:00 2001 From: kminkovich Date: Fri, 18 Oct 2024 10:39:47 -0700 Subject: [PATCH] fix(docs): Remove gzip encoding query param Box will automatically use accept-encoding header This fixes a bug where gzip encoding wasn't available in accept-encoding list --- src/lib/Preview.js | 6 ++--- src/lib/__tests__/Preview-test.js | 10 +------ src/lib/constants.js | 6 ----- src/lib/viewers/doc/DocBaseViewer.js | 25 ++--------------- .../doc/__tests__/DocBaseViewer-test.js | 27 ------------------- 5 files changed, 5 insertions(+), 69 deletions(-) diff --git a/src/lib/Preview.js b/src/lib/Preview.js index 58f2eae3e..c12a07ad8 100644 --- a/src/lib/Preview.js +++ b/src/lib/Preview.js @@ -1313,7 +1313,7 @@ class Preview extends EventEmitter { } // Log now that loading is finished - this.emitLoadMetrics(data); + this.emitLoadMetrics(); if (canDownload(this.file, this.options)) { this.ui.showDownloadButton(this.download); @@ -1597,10 +1597,9 @@ class Preview extends EventEmitter { * A value of 0 means that the load milestone was never reached. * * @private - * @param {string} [encoding] - Type of encoding applied to the downloaded content. ie) GZIP * @return {void} */ - emitLoadMetrics({ encoding } = {}) { + emitLoadMetrics() { if (!this.file || !this.file.id) { return; } @@ -1620,7 +1619,6 @@ class Preview extends EventEmitter { const previewLoadTime = Timer.get(previewLoadTag) || {}; this.emitLogEvent(PREVIEW_METRIC, { - encoding, event_name: LOAD_METRIC.previewLoadEvent, value: previewLoadTime.elapsed || 0, [LOAD_METRIC.fileInfoTime]: infoTime.elapsed || 0, diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js index 08e229dd2..20ddd96a6 100644 --- a/src/lib/__tests__/Preview-test.js +++ b/src/lib/__tests__/Preview-test.js @@ -10,7 +10,7 @@ import PreviewError from '../PreviewError'; import PreviewPerf from '../PreviewPerf'; import Timer from '../Timer'; import loaders from '../loaders'; -import { API_HOST, CLASS_NAVIGATION_VISIBILITY, ENCODING_TYPES } from '../constants'; +import { API_HOST, CLASS_NAVIGATION_VISIBILITY } from '../constants'; import { VIEWER_EVENT, ERROR_CODE, LOAD_METRIC, PREVIEW_METRIC } from '../events'; import PageTracker from '../PageTracker'; import { isFeatureEnabled } from '../featureChecking'; @@ -2525,14 +2525,6 @@ describe('lib/Preview', () => { expect(Timer.reset).toBeCalled(); expect(preview.emit).toBeCalled(); }); - - test('should append encoding field to load metric, when provided', done => { - preview.once(PREVIEW_METRIC, metric => { - expect(metric.encoding).toBe(ENCODING_TYPES.GZIP); - done(); - }); - preview.emitLoadMetrics({ encoding: ENCODING_TYPES.GZIP }); - }); }); describe('getRequestHeaders()', () => { diff --git a/src/lib/constants.js b/src/lib/constants.js index 7d59a266e..4c5227cec 100644 --- a/src/lib/constants.js +++ b/src/lib/constants.js @@ -112,12 +112,6 @@ export const PREVIEW_SCRIPT_NAME = 'preview.js'; export const FILE_OPTION_FILE_VERSION_ID = 'fileVersionId'; export const FILE_OPTION_START = 'startAt'; -// Query parameter for requesting compressed representations -export const QUERY_PARAM_ENCODING = 'encoding'; -export const ENCODING_TYPES = { - GZIP: 'gzip', -}; - export const ANNOTATOR_EVENT = { modeEnter: 'annotationmodeenter', modeExit: 'annotationmodeexit', diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js index 41b707f39..06dbf752e 100644 --- a/src/lib/viewers/doc/DocBaseViewer.js +++ b/src/lib/viewers/doc/DocBaseViewer.js @@ -26,20 +26,11 @@ import { CLASS_IS_SCROLLABLE, DISCOVERABILITY_ATTRIBUTE, DOCUMENT_FTUX_CURSOR_SEEN_KEY, - ENCODING_TYPES, PERMISSION_DOWNLOAD, PRELOAD_REP_NAME, - QUERY_PARAM_ENCODING, STATUS_SUCCESS, } from '../../constants'; -import { - appendQueryParams, - createAssetUrlCreator, - decodeKeydown, - getClosestPageToPinch, - getDistance, - getMidpoint, -} from '../../util'; +import { createAssetUrlCreator, decodeKeydown, getClosestPageToPinch, getDistance, getMidpoint } from '../../util'; import { checkPermission, getRepresentation } from '../../file'; import { ICON_PRINT_CHECKMARK } from '../../icons'; import { CMAP, CSS, IMAGES, JS, PRELOAD_JS, WORKER } from './docAssets'; @@ -101,9 +92,6 @@ class DocBaseViewer extends BaseViewer { /** @property {Thumbnail} - Thumbnail reference */ advancedInsightsThumbs; - /** @property {string} - Tracks the type of encoding, if applicable, that was requested for the viewable content */ - encoding; - /** @property {PageTracker} - PageTracker instance */ pageTracker; @@ -702,7 +690,6 @@ class DocBaseViewer extends BaseViewer { const { file, location } = this.options; const { size, watermark_info: watermarkInfo } = file; const assetUrlCreator = createAssetUrlCreator(location); - const queryParams = {}; // Do not disable create object URL in IE11 or iOS Chrome - pdf.js issues #3977 and #8081 are // not applicable to Box's use case and disabling causes performance issues @@ -723,13 +710,6 @@ class DocBaseViewer extends BaseViewer { // Disable streaming by default unless it is explicitly enabled via options const disableStream = this.getViewerOption('disableStream') !== false; - // If range requests and streaming are disabled, request the gzip compressed version of the representation - this.encoding = disableRange && disableStream ? ENCODING_TYPES.GZIP : undefined; - - if (this.encoding) { - queryParams[QUERY_PARAM_ENCODING] = this.encoding; - } - // Load PDF from representation URL and set as document for pdf.js. Cache task for destruction this.pdfLoadingTask = this.pdfjsLib.getDocument({ cMapPacked: true, @@ -740,7 +720,7 @@ class DocBaseViewer extends BaseViewer { disableStream, isEvalSupported: false, rangeChunkSize, - url: appendQueryParams(pdfUrl, queryParams), + url: pdfUrl, }); if (this.pageTracker) { @@ -1218,7 +1198,6 @@ class DocBaseViewer extends BaseViewer { } } this.emit(VIEWER_EVENT.load, { - encoding: this.encoding, numPages: pagesCount, scale: currentScale, currentPage: startPage, diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js index 8f2a24550..7f2bd0696 100644 --- a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js +++ b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js @@ -22,8 +22,6 @@ import { STATUS_ERROR, STATUS_PENDING, STATUS_SUCCESS, - QUERY_PARAM_ENCODING, - ENCODING_TYPES, SELECTOR_BOX_PREVIEW_CONTENT, CLASS_ANNOTATIONS_DOCUMENT_FTUX_CURSOR_SEEN, CLASS_BOX_PREVIEW_THUMBNAILS_CONTAINER, @@ -1359,29 +1357,6 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { return docBase.initViewer(''); }); - test('should append encoding query parameter for gzip content when range requests are disabled', () => { - const defaultChunkSize = 524288; // Taken from RANGE_CHUNK_SIZE_NON_US - const url = 'www.myTestPDF.com/123456'; - const paramsList = `${QUERY_PARAM_ENCODING}=${ENCODING_TYPES.GZIP}`; - - docBase.options.location = { - locale: 'ja-JP', // Disables range requests - }; - - docBase.options.file = { - size: 1048576, // 1MB < RANGE_REQUEST_MINIMUM_SIZE (25MB) - }; - - return docBase.initViewer(url).then(() => { - expect(stubs.getDocument).toBeCalledWith( - expect.objectContaining({ - rangeChunkSize: defaultChunkSize, - url: `${url}?${paramsList}`, - }), - ); - }); - }); - test('should resolve the loading task and set the document/viewer', () => { const doc = { url: 'url', @@ -1990,12 +1965,10 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => { }; docBase.loaded = false; docBase.pdfViewer.pagesCount = 5; - docBase.encoding = 'gzip'; docBase.startPageNum = 1; docBase.pagesinitHandler(); expect(stubs.emit).toBeCalledWith(VIEWER_EVENT.load, { - encoding: docBase.encoding, numPages: 5, scale: 'unknown', currentPage: 1,