Commit 56737601 authored by Robert Knight's avatar Robert Knight

Remove fallback document URL and metadata

Remove the fallback logic that uses `location.href` and `document.title`
as the document URL and metadata if using the integration-specific code
(HTML or PDF) to get the document URL and metadata fails. Instead just
let the async exception propagate.  Currently this will result in no
annotation/highlight being created and a console error. In future we
should improve this by showing an error notification to the user.

Recording the wrong document metadata with an annotation is worse than
not creating an annotation at all: The user will be unaware that there
is a problem yet the annotation will likely fail to show up in future or
be displayed with completely wrong metadata in the Notebook. Therefore
we want the failure here to be obvious so that the user notifies us of
the problem and doesn't create a bunch of annotations which later
"disappear".

Fixes #3204
parent f2a6fe75
......@@ -229,24 +229,17 @@ export default class Guest {
/**
* Retrieve metadata for the current document.
*/
getDocumentInfo() {
const uriPromise = this.integration
.uri()
.catch(() => decodeURIComponent(window.location.href));
const metadataPromise = this.integration.getMetadata().catch(() => ({
title: document.title,
link: [{ href: decodeURIComponent(window.location.href) }],
}));
return Promise.all([metadataPromise, uriPromise]).then(
([metadata, href]) => {
return {
uri: normalizeURI(href),
metadata,
frameIdentifier: this.frameIdentifier,
};
}
);
async getDocumentInfo() {
const [uri, metadata] = await Promise.all([
this.integration.uri(),
this.integration.getMetadata(),
]);
return {
uri: normalizeURI(uri),
metadata,
frameIdentifier: this.frameIdentifier,
};
}
/**
......
......@@ -399,35 +399,6 @@ describe('Guest', () => {
createCallback('https://example.com/test.pdf', metadata, done)
);
});
it('calls the callback with fallback URL if PDF URL cannot be determined', done => {
guest = createGuest({ documentType: 'pdf' });
fakePDFIntegration.getMetadata.resolves({});
fakePDFIntegration.uri.rejects(new Error('Not a PDF document'));
emitGuestEvent(
'getDocumentInfo',
createCallback(window.location.href, {}, done)
);
});
it('calls the callback with fallback metadata if PDF metadata extraction fails', done => {
guest = createGuest({ documentType: 'pdf' });
const metadata = {
title: document.title,
link: [{ href: window.location.href }],
};
fakePDFIntegration.getMetadata.rejects(
new Error('Not a PDF document')
);
emitGuestEvent(
'getDocumentInfo',
createCallback('https://example.com/test.pdf', metadata, done)
);
});
});
});
......@@ -642,6 +613,18 @@ describe('Guest', () => {
.getDocumentInfo()
.then(({ uri }) => assert.equal(uri, 'http://foobar.com/things?id=42'));
});
it('rejects if getting the URL fails', async () => {
const guest = createGuest();
fakeHTMLIntegration.uri.rejects(new Error('Failed to get URI'));
await assert.rejects(guest.getDocumentInfo(), 'Failed to get URI');
});
it('rejects if getting the document metadata fails', async () => {
const guest = createGuest();
fakeHTMLIntegration.getMetadata.rejects(new Error('Failed to get URI'));
await assert.rejects(guest.getDocumentInfo(), 'Failed to get URI');
});
});
describe('#createAnnotation', () => {
......
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