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

Re-write PDFHistory from scratch #8775

Merged
merged 6 commits into from
Sep 3, 2017
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
1 change: 1 addition & 0 deletions test/unit/clitests.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"murmurhash3_spec.js",
"node_stream_spec.js",
"parser_spec.js",
"pdf_history.js",
"primitives_spec.js",
"stream_spec.js",
"type1_parser_spec.js",
Expand Down
1 change: 1 addition & 0 deletions test/unit/jasmine-boot.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function initializePDFJS(callback) {
'pdfjs-test/unit/murmurhash3_spec',
'pdfjs-test/unit/network_spec',
'pdfjs-test/unit/parser_spec',
'pdfjs-test/unit/pdf_history_spec',
'pdfjs-test/unit/primitives_spec',
'pdfjs-test/unit/stream_spec',
'pdfjs-test/unit/type1_parser_spec',
Expand Down
45 changes: 45 additions & 0 deletions test/unit/pdf_history_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/* Copyright 2017 Mozilla Foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { isDestsEqual } from '../../web/pdf_history';

describe('pdf_history', function() {
describe('isDestsEqual', function() {
let firstDest = [{ num: 1, gen: 0, }, { name: 'XYZ', }, 0, 375, null];
let secondDest = [{ num: 5, gen: 0, }, { name: 'XYZ', }, 0, 375, null];
let thirdDest = [{ num: 1, gen: 0, }, { name: 'XYZ', }, 750, 0, null];
let fourthDest = [{ num: 1, gen: 0, }, { name: 'XYZ', }, 0, 375, 1.0];
let fifthDest = [{ gen: 0, num: 1, }, { name: 'XYZ', }, 0, 375, null];

it('should reject non-equal destination arrays', function() {
expect(isDestsEqual(firstDest, undefined)).toEqual(false);
expect(isDestsEqual(firstDest, [1, 2, 3, 4, 5])).toEqual(false);

expect(isDestsEqual(firstDest, secondDest)).toEqual(false);
expect(isDestsEqual(firstDest, thirdDest)).toEqual(false);
expect(isDestsEqual(firstDest, fourthDest)).toEqual(false);
});

it('should accept equal destination arrays', function() {
expect(isDestsEqual(firstDest, firstDest)).toEqual(true);
expect(isDestsEqual(firstDest, fifthDest)).toEqual(true);

let firstDestCopy = firstDest.slice();
expect(firstDest).not.toBe(firstDestCopy);

expect(isDestsEqual(firstDest, firstDestCopy)).toEqual(true);
});
});
});
117 changes: 116 additions & 1 deletion test/unit/ui_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
*/

import {
binarySearchFirstItem, EventBus, getPDFFileNameFromURL
binarySearchFirstItem, EventBus, getPDFFileNameFromURL, waitOnEventOrTimeout,
WaitOnType
} from '../../web/ui_utils';
import { createObjectURL, isNodeJS } from '../../src/shared/util';

Expand Down Expand Up @@ -259,4 +260,118 @@ describe('ui_utils', function() {
expect(count).toEqual(2);
});
});

describe('waitOnEventOrTimeout', function() {
let eventBus;

beforeAll(function(done) {
eventBus = new EventBus();
done();
});

afterAll(function() {
eventBus = null;
});

it('should reject invalid parameters', function(done) {
let invalidTarget = waitOnEventOrTimeout({
target: 'window',
name: 'DOMContentLoaded',
}).then(function() {
throw new Error('Should reject invalid parameters.');
}, function(reason) {
expect(reason instanceof Error).toEqual(true);
});

let invalidName = waitOnEventOrTimeout({
target: eventBus,
name: '',
}).then(function() {
throw new Error('Should reject invalid parameters.');
}, function(reason) {
expect(reason instanceof Error).toEqual(true);
});

let invalidDelay = waitOnEventOrTimeout({
target: eventBus,
name: 'pagerendered',
delay: -1000,
}).then(function() {
throw new Error('Should reject invalid parameters.');
}, function(reason) {
expect(reason instanceof Error).toEqual(true);
});

Promise.all([invalidTarget, invalidName, invalidDelay]).then(done,
done.fail);
});

it('should resolve on event, using the DOM', function(done) {
if (isNodeJS()) {
pending('Document in not supported in Node.js.');
}
let button = document.createElement('button');

let buttonClicked = waitOnEventOrTimeout({
target: button,
name: 'click',
delay: 10000,
});
// Immediately dispatch the expected event.
button.click();

buttonClicked.then(function(type) {
expect(type).toEqual(WaitOnType.EVENT);
done();
}, done.fail);
});

it('should resolve on timeout, using the DOM', function(done) {
if (isNodeJS()) {
pending('Document in not supported in Node.js.');
}
let button = document.createElement('button');

let buttonClicked = waitOnEventOrTimeout({
target: button,
name: 'click',
delay: 10,
});
// Do *not* dispatch the event, and wait for the timeout.

buttonClicked.then(function(type) {
expect(type).toEqual(WaitOnType.TIMEOUT);
done();
}, done.fail);
});

it('should resolve on event, using the EventBus', function(done) {
let pageRendered = waitOnEventOrTimeout({
target: eventBus,
name: 'pagerendered',
delay: 10000,
});
// Immediately dispatch the expected event.
eventBus.dispatch('pagerendered');

pageRendered.then(function(type) {
expect(type).toEqual(WaitOnType.EVENT);
done();
}, done.fail);
});

it('should resolve on timeout, using the EventBus', function(done) {
let pageRendered = waitOnEventOrTimeout({
target: eventBus,
name: 'pagerendered',
delay: 10,
});
// Do *not* dispatch the event, and wait for the timeout.

pageRendered.then(function(type) {
expect(type).toEqual(WaitOnType.TIMEOUT);
done();
}, done.fail);
});
});
});
67 changes: 17 additions & 50 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ const DefaultExternalServices = {

let PDFViewerApplication = {
initialBookmark: document.location.hash.substring(1),
initialDestination: null,
initialized: false,
fellback: false,
appConfig: null,
Expand Down Expand Up @@ -931,21 +930,16 @@ let PDFViewerApplication = {
if (!PDFJS.disableHistory && !this.isViewerEmbedded) {
// The browsing history is only enabled when the viewer is standalone,
// i.e. not when it is embedded in a web page.
if (!this.viewerPrefs['showPreviousViewOnLoad']) {
this.pdfHistory.clearHistoryState();
}
this.pdfHistory.initialize(this.documentFingerprint);
let resetHistory = !this.viewerPrefs['showPreviousViewOnLoad'];
this.pdfHistory.initialize(id, resetHistory);

if (this.pdfHistory.initialDestination) {
this.initialDestination = this.pdfHistory.initialDestination;
} else if (this.pdfHistory.initialBookmark) {
if (this.pdfHistory.initialBookmark) {
this.initialBookmark = this.pdfHistory.initialBookmark;
}
}

let initialParams = {
destination: this.initialDestination,
bookmark: this.initialBookmark,
bookmark: null,
hash: null,
};
let storePromise = store.getMultiple({
Expand Down Expand Up @@ -979,9 +973,11 @@ let PDFViewerApplication = {
sidebarView,
};
}).then(({ hash, sidebarView, }) => {
this.setInitialView(hash, { sidebarView, });
initialParams.bookmark = this.initialBookmark;
initialParams.hash = hash;

this.setInitialView(hash, { sidebarView, });

// Make all navigation keys work on document load,
// unless the viewer is embedded in a web page.
if (!this.isViewerEmbedded) {
Expand All @@ -991,14 +987,12 @@ let PDFViewerApplication = {
}).then(() => {
// For documents with different page sizes, once all pages are resolved,
// ensure that the correct location becomes visible on load.
if (!initialParams.destination && !initialParams.bookmark &&
!initialParams.hash) {
if (!initialParams.bookmark && !initialParams.hash) {
return;
}
if (pdfViewer.hasEqualPageSizes) {
return;
}
this.initialDestination = initialParams.destination;
this.initialBookmark = initialParams.bookmark;

pdfViewer.currentScaleValue = pdfViewer.currentScaleValue;
Expand Down Expand Up @@ -1141,12 +1135,8 @@ let PDFViewerApplication = {
this.isInitialViewSet = true;
this.pdfSidebar.setInitialView(sidebarView);

if (this.initialDestination) {
this.pdfLinkService.navigateTo(this.initialDestination);
this.initialDestination = null;
} else if (this.initialBookmark) {
if (this.initialBookmark) {
this.pdfLinkService.setHash(this.initialBookmark);
this.pdfHistory.push({ hash: this.initialBookmark, }, true);
this.initialBookmark = null;
} else if (storedHash) {
this.pdfLinkService.setHash(storedHash);
Expand Down Expand Up @@ -1787,10 +1777,6 @@ function webViewerUpdateViewarea(evt) {
PDFViewerApplication.appConfig.secondaryToolbar.viewBookmarkButton.href =
href;

// Update the current bookmark in the browsing history.
PDFViewerApplication.pdfHistory.updateCurrentBookmark(location.pdfOpenParams,
location.pageNumber);

// Show/hide the loading indicator in the page number input element.
let currentPage =
PDFViewerApplication.pdfViewer.getPageView(PDFViewerApplication.page - 1);
Expand All @@ -1814,16 +1800,14 @@ function webViewerResize() {
}

function webViewerHashchange(evt) {
if (PDFViewerApplication.pdfHistory.isHashChangeUnlocked) {
let hash = evt.hash;
if (!hash) {
return;
}
if (!PDFViewerApplication.isInitialViewSet) {
PDFViewerApplication.initialBookmark = hash;
} else {
PDFViewerApplication.pdfLinkService.setHash(hash);
}
let hash = evt.hash;
if (!hash) {
return;
}
if (!PDFViewerApplication.isInitialViewSet) {
PDFViewerApplication.initialBookmark = hash;
} else if (!PDFViewerApplication.pdfHistory.popStateInProgress) {
PDFViewerApplication.pdfLinkService.setHash(hash);
}
}

Expand Down Expand Up @@ -2277,23 +2261,6 @@ function webViewerKeyDown(evt) {
}
}

if (cmd === 2) { // alt-key
switch (evt.keyCode) {
case 37: // left arrow
if (isViewerInPresentationMode) {
PDFViewerApplication.pdfHistory.back();
handled = true;
}
break;
case 39: // right arrow
if (isViewerInPresentationMode) {
PDFViewerApplication.pdfHistory.forward();
handled = true;
}
break;
}
}

if (ensureViewerFocused && !pdfViewer.containsElement(curElement)) {
// The page container is not focused, but a page navigation key has been
// pressed. Change the focus to the viewer container to make sure that
Expand Down
18 changes: 15 additions & 3 deletions web/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,22 @@ class IPDFLinkService {
* @interface
*/
class IPDFHistory {
forward() {}
/**
* @param {string} fingerprint - The PDF document's unique fingerprint.
* @param {boolean} resetHistory - (optional) Reset the browsing history.
*/
initialize(fingerprint, resetHistory = false) {}

/**
* @param {Object} params
*/
push({ namedDest, explicitDest, pageNumber, }) {}

pushCurrentPosition() {}

back() {}
push(params) {}
updateNextHashParam(hash) {}

forward() {}
}

/**
Expand Down
Loading