Commit 5cb7fb17 authored by Robert Knight's avatar Robert Knight

Only send annotations to corresponding chapters in EPUBs

When new annotations are loaded in the store, compare their segment information
(ie. which EPUB chapter they were made in) against that of the guest frames. If
there is no match, skip sending the annotation to the frame. This prevents
annotations made on chapters other than the current one from mis-anchoring
or being incorrectly marked as orphans.

Currently these "skipped" annotations will appear only in the sidebar after a
short delay. This is because logic elsewhere in the application expects all
loaded annotations to be marked as anchored or not and doesn't display them
until the anchoring status is set, or a 500ms timeout is reached. For these
skipped annotations, they don't appear until after the 500ms timeout expires.
This will be addressed separately.
parent 6e841c14
...@@ -9,6 +9,7 @@ import { ...@@ -9,6 +9,7 @@ import {
isMessageEqual, isMessageEqual,
} from '../../shared/messaging'; } from '../../shared/messaging';
import { isReply, isPublic } from '../helpers/annotation-metadata'; import { isReply, isPublic } from '../helpers/annotation-metadata';
import { annotationMatchesSegment } from '../helpers/annotation-segment';
import { watch } from '../util/watch'; import { watch } from '../util/watch';
import type { Message } from '../../shared/messaging'; import type { Message } from '../../shared/messaging';
...@@ -59,7 +60,7 @@ export function formatAnnot({ ...@@ -59,7 +60,7 @@ export function formatAnnot({
/** /**
* Return the frame which best matches an annotation. * Return the frame which best matches an annotation.
*/ */
function frameForAnnotation(frames: Frame[], ann: Annotation) { function frameForAnnotation(frames: Frame[], ann: Annotation): Frame | null {
// Choose frame with an exact URL match if possible. In the unlikely situation // Choose frame with an exact URL match if possible. In the unlikely situation
// where multiple frames have the same URL, we'll use whichever connected first. // where multiple frames have the same URL, we'll use whichever connected first.
const uriMatch = frames.find(f => f.uri === ann.uri); const uriMatch = frames.find(f => f.uri === ann.uri);
...@@ -75,7 +76,7 @@ function frameForAnnotation(frames: Frame[], ann: Annotation) { ...@@ -75,7 +76,7 @@ function frameForAnnotation(frames: Frame[], ann: Annotation) {
// If there is no main frame (eg. in VitalSource), fall back to whichever // If there is no main frame (eg. in VitalSource), fall back to whichever
// frame connected first. // frame connected first.
return frames[0]; return frames[0] ?? null;
} }
/** /**
...@@ -198,9 +199,14 @@ export class FrameSyncService { ...@@ -198,9 +199,14 @@ export class FrameSyncService {
// Send added annotations to matching frame. // Send added annotations to matching frame.
if (added.length > 0) { if (added.length > 0) {
const addedByFrame = new Map<string | null, Annotation[]>(); const addedByFrame = new Map<string | null, Annotation[]>();
for (const annotation of added) { for (const annotation of added) {
const frame = frameForAnnotation(frames, annotation); const frame = frameForAnnotation(frames, annotation);
if (!frame) { if (
!frame ||
(frame.segment &&
!annotationMatchesSegment(annotation, frame.segment))
) {
continue; continue;
} }
const anns = addedByFrame.get(frame.id) ?? []; const anns = addedByFrame.get(frame.id) ?? [];
......
...@@ -347,6 +347,57 @@ describe('FrameSyncService', () => { ...@@ -347,6 +347,57 @@ describe('FrameSyncService', () => {
); );
}); });
context('in EPUB documents', () => {
it('sends annotations to frame if current segment matches frame', async () => {
const bookURI = 'https://publisher.com/books/1234';
const chapter1ann = {
uri: bookURI,
target: [
{
selector: [
{
type: 'EPUBContentSelector',
cfi: '/2',
url: '/chapters/01.xhtml',
},
],
},
],
};
const chapter2ann = {
uri: bookURI,
target: [
{
selector: [
{
type: 'EPUBContentSelector',
cfi: '/4',
url: '/chapters/02.xhtml',
},
],
},
],
};
await connectGuest();
emitGuestEvent('documentInfoChanged', {
uri: bookURI,
segmentInfo: {
cfi: '/4',
url: '/chapters/02.xhtml',
},
});
fakeStore.setState({ annotations: [chapter1ann, chapter2ann] });
assert.calledWithMatch(
guestRPC().call,
'loadAnnotations',
sinon.match([formatAnnot(chapter2ann)])
);
});
});
it('sends a "loadAnnotations" message only for new annotations', async () => { it('sends a "loadAnnotations" message only for new annotations', async () => {
const frameInfo = fixtures.htmlDocumentInfo; const frameInfo = fixtures.htmlDocumentInfo;
await connectGuest(); await connectGuest();
......
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