Commit 3771ff0e authored by Robert Knight's avatar Robert Knight

Remove `beforeAnnotationCreated` event handler in `DocumentMeta`

This handler sets the `document` property of new annotations to metadata
extracted from `<meta>` and `<link>` tags on the page. It is unnecessary
however because the `Guest` class sets this property using either
`DocumentMeta.metadata` or `PDF.getMetadata`, depending on the document
type, before it emits the `beforeAnnotationCreated` event.

In PDFs this code is a serious hazard because if the
`beforeAnnotationCreated` event subscriber in `DocumentMeta` runs before
`AnnotationSync`'s corresponding event handler then DocumentMeta will
overwrite the PDF document-specific annotation metadata with generic
HTML document metadata - losing the critical `documentFingerprint`
property. Fortunately this problem did not occur because the `Guest`
constructor instantiates the `AnnotationSync` class before the
`DocumentMeta` class. Consequently AnnotationSync's
`beforeAnnotationCreated` event handler runs before DocumentMeta's and
pushes new annotations to the sidebar with the correct document metadata
before `DocumentMeta` overwrites it.
parent 39d6e181
...@@ -72,21 +72,9 @@ export default class DocumentMeta extends Delegator { ...@@ -72,21 +72,9 @@ export default class DocumentMeta extends Delegator {
this.document = options.document || document; this.document = options.document || document;
this.normalizeURI = options.normalizeURI || normalizeURI; this.normalizeURI = options.normalizeURI || normalizeURI;
/**
* Event handler that adds document metadata to new annotations.
*/
this.beforeAnnotationCreated = annotation => {
annotation.document = this.metadata;
};
this.subscribe('beforeAnnotationCreated', this.beforeAnnotationCreated);
this.getDocumentMetadata(); this.getDocumentMetadata();
} }
destroy() {
this.unsubscribe('beforeAnnotationCreated', this.beforeAnnotationCreated);
}
/** /**
* Returns the primary URI for the document being annotated * Returns the primary URI for the document being annotated
* *
......
...@@ -43,12 +43,6 @@ describe('DocumentMeta', function () { ...@@ -43,12 +43,6 @@ describe('DocumentMeta', function () {
testDocument.destroy(); testDocument.destroy();
}); });
it('attaches document metadata to new annotations', () => {
const annotation = {};
testDocument.publish('beforeAnnotationCreated', [annotation]);
assert.equal(annotation.document, testDocument.metadata);
});
describe('annotation should have some metadata', function () { describe('annotation should have some metadata', function () {
let metadata = null; let metadata = null;
......
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