Skip to content

Commit

Permalink
Remove the special handling for nameddests that look like standard …
Browse files Browse the repository at this point in the history
…pageNumbers

PR 7341 added special handling for `nameddest`s that look like pageNumbers, to prevent issues since we previously *incorrectly* supported specifying a pageNumber directly in the hash; i.e. `mozilla#10` versus the correct `#page=10` format.

Since this behaviour wasn't correct, PR 7757 fixed and deprecated the old format, which means that we no longer need to maintain the `nameddest` hack in multiple files.
  • Loading branch information
Snuffleupagus committed May 20, 2017
1 parent 756e095 commit bfac100
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 27 deletions.
8 changes: 2 additions & 6 deletions src/core/obj.js
Original file line number Diff line number Diff line change
Expand Up @@ -683,13 +683,9 @@ var Catalog = (function CatalogClosure() {
remoteDest = remoteDest.name;
}
if (isString(url)) {
var baseUrl = url.split('#')[0];
let baseUrl = url.split('#')[0];
if (isString(remoteDest)) {
// In practice, a named destination may contain only a number.
// If that happens, use the '#nameddest=' form to avoid the link
// redirecting to a page, instead of the correct destination.
url = baseUrl + '#' +
(/^\d+$/.test(remoteDest) ? 'nameddest=' : '') + remoteDest;
url = baseUrl + '#' + remoteDest;
} else if (isArray(remoteDest)) {
url = baseUrl + '#' + JSON.stringify(remoteDest);
}
Expand Down
5 changes: 2 additions & 3 deletions test/unit/annotation_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,8 @@ describe('annotation', function() {
var data = annotation.data;
expect(data.annotationType).toEqual(AnnotationType.LINK);

expect(data.url).toEqual('http://www.example.com/test.pdf#nameddest=15');
expect(data.unsafeUrl).toEqual(
'http://www.example.com/test.pdf#nameddest=15');
expect(data.url).toEqual('http://www.example.com/test.pdf#15');
expect(data.unsafeUrl).toEqual('http://www.example.com/test.pdf#15');
expect(data.dest).toBeUndefined();
expect(data.newWindow).toBeFalsy();
});
Expand Down
8 changes: 4 additions & 4 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -944,16 +944,16 @@ describe('api', function() {

expect(defaultAnnotations[0].url).toBeUndefined();
expect(defaultAnnotations[0].unsafeUrl).toEqual(
'../../0021/002156/215675E.pdf#nameddest=15');
'../../0021/002156/215675E.pdf#15');

expect(docBaseUrlAnnotations[0].url).toEqual(
'http://www.example.com/0021/002156/215675E.pdf#nameddest=15');
'http://www.example.com/0021/002156/215675E.pdf#15');
expect(docBaseUrlAnnotations[0].unsafeUrl).toEqual(
'../../0021/002156/215675E.pdf#nameddest=15');
'../../0021/002156/215675E.pdf#15');

expect(invalidDocBaseUrlAnnotations[0].url).toBeUndefined();
expect(invalidDocBaseUrlAnnotations[0].unsafeUrl).toEqual(
'../../0021/002156/215675E.pdf#nameddest=15');
'../../0021/002156/215675E.pdf#15');

Promise.all([
defaultLoadingTask.destroy(),
Expand Down
19 changes: 5 additions & 14 deletions web/pdf_link_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
import { getGlobalEventBus } from './dom_events';
import { parseQueryString } from './ui_utils';

var PageNumberRegExp = /^\d+$/;
function isPageNumber(str) {
return PageNumberRegExp.test(str);
}

/**
* @typedef {Object} PDFLinkServiceOptions
* @property {EventBus} eventBus - The application event bus.
Expand Down Expand Up @@ -154,19 +149,15 @@ var PDFLinkService = (function PDFLinkServiceClosure() {
},

/**
* @param dest - The PDF destination object.
* @param {string|Array} dest - The PDF destination object.
* @returns {string} The hyperlink to the PDF object.
*/
getDestinationHash: function PDFLinkService_getDestinationHash(dest) {
getDestinationHash(dest) {
if (typeof dest === 'string') {
// In practice, a named destination may contain only a number.
// If that happens, use the '#nameddest=' form to avoid the link
// redirecting to a page, instead of the correct destination.
return this.getAnchorUrl(
'#' + (isPageNumber(dest) ? 'nameddest=' : '') + escape(dest));
return this.getAnchorUrl('#' + escape(dest));
}
if (dest instanceof Array) {
var str = JSON.stringify(dest);
let str = JSON.stringify(dest);
return this.getAnchorUrl('#' + escape(str));
}
return this.getAnchorUrl('');
Expand Down Expand Up @@ -259,7 +250,7 @@ var PDFLinkService = (function PDFLinkServiceClosure() {
}
} else { // Named (or explicit) destination.
if ((typeof PDFJSDev === 'undefined' || PDFJSDev.test('GENERIC')) &&
isPageNumber(hash) && hash <= this.pagesCount) {
/^\d+$/.test(hash) && hash <= this.pagesCount) {
console.warn('PDFLinkService_setHash: specifying a page number ' +
'directly after the hash symbol (#) is deprecated, ' +
'please use the "#page=' + hash + '" form instead.');
Expand Down

0 comments on commit bfac100

Please sign in to comment.