Commit 2381e2c9 authored by Robert Knight's avatar Robert Knight

Fix anchoring using quote selector alone failing

Looking at the `anchor` function, it is clearly intended that the
position selector is optional for anchoring, as is the case for HTML
anchoring. However, anchoring using only a quote selector did not
actually work. This was in part due to the bug fixed in the previous
commit but also because of a missing check for a null position selector
when populating the (quote,position) cache.

In order for the test to pass using only a quote, it was necessary to
change the search string to a unique quote which only occurs once in the
document. Otherwise the quote anchoring found the occurrence of
'Netherfield Park' on the non-rendered page 1.
parent 81aa2372
......@@ -8,8 +8,11 @@ RenderingStates = require('../pdfjs-rendering-states')
# Caches for performance
# Map of page index to Promise<string>
# Map of page index to page text content as a `Promise<string>`
pageTextCache = {}
# Two-dimensional map from `[quote][position]` to `{page, anchor}` intended to
# optimize re-anchoring of a pair of quote and position selectors if the
# position selector fails to anchor on its own.
quotePositionCache = {}
......@@ -132,7 +135,7 @@ anchorByPosition = (page, anchor, options) ->
# Returns a `Promise<Range>` for the location of the quote.
findInPages = ([pageIndex, rest...], quote, position) ->
unless pageIndex?
return Promise.reject('quote not found')
return Promise.reject(new Error('Quote not found'))
attempt = (info) ->
# Try to find the quote in the current page.
......@@ -151,8 +154,9 @@ findInPages = ([pageIndex, rest...], quote, position) ->
return findInPages(rest, quote, position)
cacheAndFinish = (anchor) ->
quotePositionCache[quote.exact] ?= {}
quotePositionCache[quote.exact][position.start] = {page, anchor}
if position
quotePositionCache[quote.exact] ?= {}
quotePositionCache[quote.exact][position.start] = {page, anchor}
return anchorByPosition(page, anchor)
page = getPage(pageIndex)
......
......@@ -139,7 +139,7 @@ describe('PDF anchoring', function () {
describe('#anchor', function () {
it('anchors previously created selectors if the page is rendered', function () {
viewer.setCurrentPage(2);
var range = findText(container, 'Netherfield Park');
var range = findText(container, 'My dear Mr. Bennet');
return pdfAnchoring.describe(container, range).then(function (selectors) {
var position = selectors[0];
var quote = selectors[1];
......@@ -149,8 +149,7 @@ describe('PDF anchoring', function () {
var subsets = [
[position, quote],
[position],
// FIXME - Anchoring a quote on its own does not currently work
// [quote],
[quote],
];
var subsetsAnchored = subsets.map(function (subset) {
var types = subset.map(function (s) { return s.type; });
......
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