fix(reader): resolve KOReader sync conflict against non-KOReader servers (#4205)
The sync-conflict dialog had two issues with servers other than KOReader (e.g. Kavita's KOReader-compatible sync endpoint): - "This device" preview rendered a bare "undefined" because reflowable books built the string from `sectionLabel`, which is empty for spine items with no matching TOC entry. It now falls back to the page count. - Choosing "use remote" closed the dialog but never moved the reader: `applyRemoteProgress` only knew how to navigate via CREngine XPointers, so non-XPointer progress strings were silently ignored. It now falls back to `view.goToFraction` using the reported percentage. Also fixes the section-title indentation in the dialog (SectionTitle bakes in `ps-4`, which misaligned the labels against their values). Closes #4200 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,73 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { BookProgress } from '@/types/book';
|
||||
import { getLocalProgressPreview, getProgressPercentage } from '@/app/reader/hooks/kosyncPreview';
|
||||
|
||||
const makeProgress = (overrides: Partial<BookProgress> = {}): BookProgress =>
|
||||
({
|
||||
location: '',
|
||||
sectionHref: '',
|
||||
sectionLabel: '',
|
||||
section: { current: 0, total: 0 },
|
||||
pageinfo: { current: 0, total: 100 },
|
||||
timeinfo: { section: 0, total: 0 },
|
||||
index: 0,
|
||||
range: {} as Range,
|
||||
page: 1,
|
||||
...overrides,
|
||||
}) as BookProgress;
|
||||
|
||||
// Minimal interpolating stand-in for the i18n `_()` function.
|
||||
const t = (key: string, opts: Record<string, number | string> = {}) =>
|
||||
key.replace(/\{\{(\w+)\}\}/g, (_m, name) => String(opts[name]));
|
||||
|
||||
describe('getProgressPercentage', () => {
|
||||
it('returns 0 when page info is missing', () => {
|
||||
expect(getProgressPercentage(undefined)).toBe(0);
|
||||
});
|
||||
|
||||
it('returns 0 when the total is zero', () => {
|
||||
expect(getProgressPercentage({ current: 0, total: 0 })).toBe(0);
|
||||
});
|
||||
|
||||
it('treats the page index as 1-based', () => {
|
||||
expect(getProgressPercentage({ current: 49, total: 100 })).toBe(0.5);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getLocalProgressPreview', () => {
|
||||
it('uses the section label for reflowable books', () => {
|
||||
const progress = makeProgress({
|
||||
sectionLabel: 'Chapter 3',
|
||||
pageinfo: { current: 49, total: 100 },
|
||||
});
|
||||
expect(getLocalProgressPreview(progress, false, t)).toBe('Chapter 3 (50%)');
|
||||
});
|
||||
|
||||
it('falls back to page info when the section label is undefined', () => {
|
||||
const progress = makeProgress({
|
||||
sectionLabel: undefined as unknown as string,
|
||||
pageinfo: { current: 0, total: 100 },
|
||||
});
|
||||
const result = getLocalProgressPreview(progress, false, t);
|
||||
expect(result).not.toContain('undefined');
|
||||
expect(result).toBe('Page 1 of 100 (1%)');
|
||||
});
|
||||
|
||||
it('falls back to page info when the section label is blank', () => {
|
||||
const progress = makeProgress({
|
||||
sectionLabel: ' ',
|
||||
pageinfo: { current: 9, total: 50 },
|
||||
});
|
||||
const result = getLocalProgressPreview(progress, false, t);
|
||||
expect(result).not.toContain('undefined');
|
||||
expect(result).toBe('Page 10 of 50 (20%)');
|
||||
});
|
||||
|
||||
it('uses page info for fixed-layout books regardless of section label', () => {
|
||||
const progress = makeProgress({
|
||||
sectionLabel: 'ignored',
|
||||
section: { current: 4, total: 20 },
|
||||
});
|
||||
expect(getLocalProgressPreview(progress, true, t)).toBe('Page 5 of 20 (25%)');
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,41 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { isXPointerProgress, getRemoteFraction } from '@/app/reader/hooks/kosyncProgress';
|
||||
|
||||
describe('isXPointerProgress', () => {
|
||||
it('accepts CREngine XPointers', () => {
|
||||
expect(isXPointerProgress('/body/DocFragment[11]/body/div/p[3]/text().0')).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects non-XPointer progress strings', () => {
|
||||
expect(isXPointerProgress('epubcfi(/6/14!/4/2/2)')).toBe(false);
|
||||
expect(isXPointerProgress('50')).toBe(false);
|
||||
expect(isXPointerProgress('')).toBe(false);
|
||||
expect(isXPointerProgress(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getRemoteFraction', () => {
|
||||
it('returns the percentage as a 0–1 fraction', () => {
|
||||
expect(getRemoteFraction({ percentage: 0.5 })).toBe(0.5);
|
||||
});
|
||||
|
||||
it('clamps fractions above 1', () => {
|
||||
expect(getRemoteFraction({ percentage: 1.5 })).toBe(1);
|
||||
});
|
||||
|
||||
it('returns undefined when there is no usable percentage', () => {
|
||||
expect(getRemoteFraction({})).toBeUndefined();
|
||||
expect(getRemoteFraction({ percentage: 0 })).toBeUndefined();
|
||||
expect(getRemoteFraction({ percentage: -0.2 })).toBeUndefined();
|
||||
expect(getRemoteFraction({ percentage: NaN })).toBeUndefined();
|
||||
expect(getRemoteFraction({ percentage: Infinity })).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back to the percentage for non-XPointer progress (e.g. Kavita)', () => {
|
||||
// Kavita's KOReader-compatible endpoint reports progress Readest cannot
|
||||
// resolve positionally, but still sends a percentage.
|
||||
const remote = { progress: 'page-42', percentage: 0.5 };
|
||||
expect(isXPointerProgress(remote.progress)).toBe(false);
|
||||
expect(getRemoteFraction(remote)).toBe(0.5);
|
||||
});
|
||||
});
|
||||
@@ -50,7 +50,7 @@ const KOSyncConflictResolver: React.FC<KOSyncConflictResolverProps> = ({
|
||||
`as='span'` because this lives inside a button, not as a
|
||||
document heading; opacity override expresses the
|
||||
"secondary on this surface" relationship. */}
|
||||
<SectionTitle as='span' className='!text-base-content/55'>
|
||||
<SectionTitle as='span' className='!text-base-content/55 !ps-0'>
|
||||
{_('This device')}
|
||||
</SectionTitle>
|
||||
<span className='line-clamp-2 text-sm font-medium leading-snug'>
|
||||
@@ -72,7 +72,7 @@ const KOSyncConflictResolver: React.FC<KOSyncConflictResolverProps> = ({
|
||||
{/* On the primary-color button background the default
|
||||
/65 token would clash; opacity-75 inherits the button's
|
||||
contrast color and dims it uniformly. */}
|
||||
<SectionTitle as='span' className='!text-current opacity-75'>
|
||||
<SectionTitle as='span' className='!ps-0 !text-current opacity-75'>
|
||||
{remoteDeviceName}
|
||||
</SectionTitle>
|
||||
<span className='line-clamp-2 text-sm font-medium leading-snug'>
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import { BookProgress, PageInfo } from '@/types/book';
|
||||
import { TranslationFunc } from '@/hooks/useTranslation';
|
||||
|
||||
/** Reading progress as a 0–1 fraction; 0 when page info is unavailable. */
|
||||
export const getProgressPercentage = (pageInfo?: PageInfo): number =>
|
||||
pageInfo && pageInfo.total > 0 ? (pageInfo.current + 1) / pageInfo.total : 0;
|
||||
|
||||
/**
|
||||
* Human-readable summary of the local reading position shown in the KOReader
|
||||
* sync-conflict dialog.
|
||||
*
|
||||
* Reflowable books prefer the current section label, but some TOCs leave spine
|
||||
* items unlabeled, so `sectionLabel` can be empty or `undefined` at runtime
|
||||
* despite its non-optional type. When it is missing we fall back to the page
|
||||
* count instead of rendering a bare "undefined".
|
||||
*/
|
||||
export const getLocalProgressPreview = (
|
||||
local: BookProgress,
|
||||
isFixedLayout: boolean,
|
||||
_: TranslationFunc,
|
||||
): string => {
|
||||
const pageInfo = isFixedLayout ? local.section : local.pageinfo;
|
||||
const percentage = Math.round(getProgressPercentage(pageInfo) * 100);
|
||||
const sectionLabel = local.sectionLabel?.trim();
|
||||
|
||||
if (!isFixedLayout && sectionLabel) {
|
||||
return `${sectionLabel} (${percentage}%)`;
|
||||
}
|
||||
if (pageInfo) {
|
||||
return _('Page {{page}} of {{total}} ({{percentage}}%)', {
|
||||
page: pageInfo.current + 1,
|
||||
total: pageInfo.total,
|
||||
percentage,
|
||||
});
|
||||
}
|
||||
return _('Current position');
|
||||
};
|
||||
@@ -0,0 +1,25 @@
|
||||
import { KoSyncProgress } from '@/services/sync/KOSyncClient';
|
||||
|
||||
/**
|
||||
* True when a KOSync `progress` string is a CREngine XPointer — KOReader's
|
||||
* native position format, e.g. `/body/DocFragment[11]/body/div/p[3]/text().0`.
|
||||
*
|
||||
* Servers other than KOReader — notably Kavita's KOReader-compatible sync
|
||||
* endpoint — report `progress` in formats Readest cannot resolve to a CFI.
|
||||
* For those, callers should fall back to the percentage (getRemoteFraction).
|
||||
*/
|
||||
export const isXPointerProgress = (progress?: string): boolean =>
|
||||
!!progress && progress.startsWith('/body');
|
||||
|
||||
/**
|
||||
* Remote reading completion as a 0–1 fraction suitable for
|
||||
* `view.goToFraction`, or `undefined` when the server reported no usable
|
||||
* percentage (missing, non-finite, or out of range).
|
||||
*/
|
||||
export const getRemoteFraction = (remote: KoSyncProgress): number | undefined => {
|
||||
const { percentage } = remote;
|
||||
if (typeof percentage !== 'number' || !Number.isFinite(percentage) || percentage <= 0) {
|
||||
return undefined;
|
||||
}
|
||||
return Math.min(percentage, 1);
|
||||
};
|
||||
@@ -10,6 +10,8 @@ import { BookDoc } from '@/libs/document';
|
||||
import { debounce } from '@/utils/debounce';
|
||||
import { eventDispatcher } from '@/utils/event';
|
||||
import { getCFIFromXPointer, XCFI } from '@/utils/xcfi';
|
||||
import { getLocalProgressPreview, getProgressPercentage } from './kosyncPreview';
|
||||
import { getRemoteFraction, isXPointerProgress } from './kosyncProgress';
|
||||
import { useWindowActiveChanged } from './useWindowActiveChanged';
|
||||
|
||||
type SyncState = 'idle' | 'checking' | 'conflict' | 'synced' | 'error';
|
||||
@@ -101,22 +103,42 @@ export const useKOSync = (bookKey: string) => {
|
||||
if (FIXED_LAYOUT_FORMATS.has(book.format)) {
|
||||
const pageToGo = parseInt(remote.progress!, 10);
|
||||
if (isNaN(pageToGo)) return;
|
||||
view?.select(pageToGo - 1);
|
||||
view.select(pageToGo - 1);
|
||||
} else {
|
||||
if (!remote.progress?.startsWith('/body')) return;
|
||||
try {
|
||||
const content = view?.renderer
|
||||
.getContents()
|
||||
.find((x) => x.index === view?.renderer.primaryIndex);
|
||||
const koProgress = remote.progress;
|
||||
const cfi = await getCFIFromXPointer(koProgress, content?.doc, content?.index, bookDoc);
|
||||
view?.goTo(cfi);
|
||||
} catch (error) {
|
||||
console.error('Failed to convert XPointer to CFI', error);
|
||||
return;
|
||||
let navigated = false;
|
||||
// KOReader stores positions as CREngine XPointers; convert and jump
|
||||
// precisely when we have one.
|
||||
if (isXPointerProgress(remote.progress)) {
|
||||
try {
|
||||
const content = view.renderer
|
||||
.getContents()
|
||||
.find((x) => x.index === view.renderer.primaryIndex);
|
||||
const cfi = await getCFIFromXPointer(
|
||||
remote.progress!,
|
||||
content?.doc,
|
||||
content?.index,
|
||||
bookDoc,
|
||||
);
|
||||
view.goTo(cfi);
|
||||
navigated = true;
|
||||
} catch (error) {
|
||||
console.error('Failed to convert XPointer to CFI', error);
|
||||
}
|
||||
}
|
||||
// Other KOSync-compatible servers (e.g. Kavita) report progress in
|
||||
// formats Readest can't resolve positionally — approximate with the
|
||||
// reported percentage so "use remote" still moves the reader.
|
||||
if (!navigated) {
|
||||
const remoteFraction = getRemoteFraction(remote);
|
||||
if (remoteFraction === undefined) return;
|
||||
view.goToFraction(remoteFraction);
|
||||
}
|
||||
}
|
||||
eventDispatcher.dispatch('toast', { message: _('Reading Progress Synced'), type: 'info' });
|
||||
eventDispatcher.dispatch('toast', {
|
||||
message: _('Reading Progress Synced'),
|
||||
type: 'info',
|
||||
timeout: 2000,
|
||||
});
|
||||
};
|
||||
|
||||
const promptedSync = async (
|
||||
@@ -125,26 +147,17 @@ export const useKOSync = (bookKey: string) => {
|
||||
local: BookProgress,
|
||||
remote: KoSyncProgress,
|
||||
) => {
|
||||
let localPreview = '';
|
||||
let remotePreview = '';
|
||||
const remotePercentage = remote.percentage || 0;
|
||||
const conflictProgressDiffThreshold = 0.0001;
|
||||
let showConflictDetails = false;
|
||||
const isFixedLayout = FIXED_LAYOUT_FORMATS.has(book.format);
|
||||
|
||||
if (FIXED_LAYOUT_FORMATS.has(book.format)) {
|
||||
const localPreview = getLocalProgressPreview(local, isFixedLayout, _);
|
||||
const localPercentage = getProgressPercentage(isFixedLayout ? local.section : local.pageinfo);
|
||||
|
||||
if (isFixedLayout) {
|
||||
const localPageInfo = local.section;
|
||||
const localPercentage =
|
||||
localPageInfo && localPageInfo.total > 0
|
||||
? (localPageInfo.current + 1) / localPageInfo.total
|
||||
: 0;
|
||||
localPreview = localPageInfo
|
||||
? _('Page {{page}} of {{total}} ({{percentage}}%)', {
|
||||
page: localPageInfo.current + 1,
|
||||
total: localPageInfo.total,
|
||||
percentage: Math.round(localPercentage * 100),
|
||||
})
|
||||
: _('Current position');
|
||||
|
||||
const remotePage = parseInt(remote.progress!, 10);
|
||||
if (!isNaN(remotePage) && remotePercentage > 0) {
|
||||
const localTotalPages = localPageInfo?.total ?? 0;
|
||||
@@ -172,13 +185,6 @@ export const useKOSync = (bookKey: string) => {
|
||||
});
|
||||
}
|
||||
} else {
|
||||
const localPageInfo = local.pageinfo;
|
||||
const localPercentage =
|
||||
localPageInfo && localPageInfo.total > 0
|
||||
? (localPageInfo.current + 1) / localPageInfo.total
|
||||
: 0;
|
||||
localPreview = `${local.sectionLabel} (${Math.round(localPercentage * 100)}%)`;
|
||||
|
||||
remotePreview = _('Approximately {{percentage}}%', {
|
||||
percentage: Math.round(remotePercentage * 100),
|
||||
});
|
||||
@@ -343,7 +349,8 @@ export const useKOSync = (bookKey: string) => {
|
||||
const book = conflictDetails?.book;
|
||||
const bookDoc = conflictDetails?.bookDoc;
|
||||
|
||||
if (!book || !bookDoc || !remote || !remote.progress || !view) return;
|
||||
if (!book || !bookDoc || !remote || !view) return;
|
||||
if (!remote.progress && getRemoteFraction(remote) === undefined) return;
|
||||
|
||||
applyRemoteProgress(book, bookDoc, remote);
|
||||
setSyncState('synced');
|
||||
|
||||
Reference in New Issue
Block a user