Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(docs): Remove gzip encoding query param #1537

Merged
merged 1 commit into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand All @@ -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,
Expand Down
10 changes: 1 addition & 9 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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()', () => {
Expand Down
6 changes: 0 additions & 6 deletions src/lib/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
25 changes: 2 additions & 23 deletions src/lib/viewers/doc/DocBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can delete queryParams and remove the appendQueryParams call now, since it's a noop.

}

// Load PDF from representation URL and set as document for pdf.js. Cache task for destruction
this.pdfLoadingTask = this.pdfjsLib.getDocument({
cMapPacked: true,
Expand All @@ -740,7 +720,7 @@ class DocBaseViewer extends BaseViewer {
disableStream,
isEvalSupported: false,
rangeChunkSize,
url: appendQueryParams(pdfUrl, queryParams),
url: pdfUrl,
});

if (this.pageTracker) {
Expand Down Expand Up @@ -1218,7 +1198,6 @@ class DocBaseViewer extends BaseViewer {
}
}
this.emit(VIEWER_EVENT.load, {
encoding: this.encoding,
numPages: pagesCount,
scale: currentScale,
currentPage: startPage,
Expand Down
27 changes: 0 additions & 27 deletions src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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,
Expand Down