Commit 81aa2372 authored by Robert Knight's avatar Robert Knight

Fix PDF quote anchoring only testing a single page

The `findInPages` function was supposed to search for a quote looking
at each of the page indexes in the input array, in order.

However because of the way that `next` was used, this function ended up
only searching the first page in the list. The `then(attempt, next)`
code was intended to try the current page and then move to the next in
the list if no match was found. However the `next` function is not
called if `attempt` throws in this context.

The `pdfContent` var in the test was renamed to `pdfPages` for clarity.
parent cffcd255
...@@ -160,8 +160,9 @@ findInPages = ([pageIndex, rest...], quote, position) -> ...@@ -160,8 +160,9 @@ findInPages = ([pageIndex, rest...], quote, position) ->
offset = getPageOffset(pageIndex) offset = getPageOffset(pageIndex)
return Promise.all([page, content, offset]) return Promise.all([page, content, offset])
.then(attempt, next) .then(attempt)
.then(cacheAndFinish) .then(cacheAndFinish)
.catch(next)
# When a position anchor is available, quote search can prioritize pages by # When a position anchor is available, quote search can prioritize pages by
......
...@@ -18,7 +18,7 @@ function findText(container, text) { ...@@ -18,7 +18,7 @@ function findText(container, text) {
var fixtures = { var fixtures = {
// Each item in this list contains the text for one page of the "PDF" // Each item in this list contains the text for one page of the "PDF"
pdfContent: [ pdfPages: [
'Pride And Prejudice And Zombies\n' + 'Pride And Prejudice And Zombies\n' +
'By Jane Austin and Seth Grahame-Smith ', 'By Jane Austin and Seth Grahame-Smith ',
...@@ -49,7 +49,7 @@ describe('PDF anchoring', function () { ...@@ -49,7 +49,7 @@ describe('PDF anchoring', function () {
window.PDFViewerApplication = viewer = new FakePDFViewerApplication({ window.PDFViewerApplication = viewer = new FakePDFViewerApplication({
container: container, container: container,
content: fixtures.pdfContent, content: fixtures.pdfPages,
}); });
viewer.setCurrentPage(0); viewer.setCurrentPage(0);
}); });
...@@ -74,7 +74,7 @@ describe('PDF anchoring', function () { ...@@ -74,7 +74,7 @@ describe('PDF anchoring', function () {
viewer.setCurrentPage(2); viewer.setCurrentPage(2);
var quote = 'Netherfield Park'; var quote = 'Netherfield Park';
var range = findText(container, quote); var range = findText(container, quote);
var contentStr = fixtures.pdfContent.join(''); var contentStr = fixtures.pdfPages.join('');
var expectedPos = contentStr.replace(/\n/g,'').lastIndexOf(quote); var expectedPos = contentStr.replace(/\n/g,'').lastIndexOf(quote);
return pdfAnchoring.describe(container, range).then(function (selectors) { return pdfAnchoring.describe(container, range).then(function (selectors) {
...@@ -125,7 +125,7 @@ describe('PDF anchoring', function () { ...@@ -125,7 +125,7 @@ describe('PDF anchoring', function () {
commonAncestorContainer: staticRange.commonAncestorContainer, commonAncestorContainer: staticRange.commonAncestorContainer,
}; };
var contentStr = fixtures.pdfContent.join(''); var contentStr = fixtures.pdfPages.join('');
var expectedPos = contentStr.replace(/\n/g,'').lastIndexOf(quote); var expectedPos = contentStr.replace(/\n/g,'').lastIndexOf(quote);
return pdfAnchoring.describe(container, range).then(function (selectors) { return pdfAnchoring.describe(container, range).then(function (selectors) {
...@@ -185,6 +185,24 @@ describe('PDF anchoring', function () { ...@@ -185,6 +185,24 @@ describe('PDF anchoring', function () {
}); });
}); });
it('anchors using a quote if the position selector refers to the wrong page', function () {
viewer.setCurrentPage(0);
var range = findText(container, 'Pride And Prejudice');
return pdfAnchoring.describe(container, range).then(function (selectors) {
var position = selectors[0];
var quote = selectors[1];
// Manipulate the position selector so that it refers to a location
// a long way away, on a different page, than the quote.
position.start += fixtures.pdfPages[0].length + 10;
position.end += fixtures.pdfPages[0].length + 10;
return pdfAnchoring.anchor(container, [position, quote]);
}).then(function (range) {
assert.equal(range.toString(), 'Pride And Prejudice');
});
});
it('anchors to a placeholder element if the page is not rendered', function () { it('anchors to a placeholder element if the page is not rendered', function () {
viewer.setCurrentPage(2); viewer.setCurrentPage(2);
var range = findText(container, 'Netherfield Park'); var range = findText(container, 'Netherfield Park');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment