Skip to content

Commit 0485f55

Browse files
committed
#3069 fixed pinch-zoom flicker/jumping caused by a feedback loop: pdf.js scalechanging events updated the Angular zoom signal, whose effect wrote the (potentially outdated) scale back to pdf.js
1 parent c89de0f commit 0485f55

3 files changed

Lines changed: 177 additions & 2 deletions

File tree

projects/ngx-extended-pdf-viewer/changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -719,3 +719,4 @@
719719
- 26.0.0-rc.15 #3069 implemented lazy rendering in infinite-scroll mode — only visible pages are rendered, with on-demand rendering as the user scrolls; fixed pinch-zoom drift in both normal and infinite-scroll modes by accounting for non-scaling content above the PDF viewer; fixed touch pinch origin coordinates for embedded viewers; fixed page navigation (page number input, next/prev buttons) in infinite-scroll mode; fixed page position preservation when switching to infinite-scroll mode; #2818 added `disable*` boolean input attributes for toolbar buttons (e.g., `[disablePrintButton]`, `[disableZoomButtons]`, `[disablePageNumber]`, `[disableZoomDropdown]`), plus `[showEditorButtons]` and `[disableEditorButtons]` group attributes that control all editor buttons at once; fixed signature editor button SVG not greying out when disabled; fixed `[mobileFriendlyZoom]` not repositioning the findbar, secondary toolbar, and sidebar when the zoom changes at runtime; fixed sidebar `sidebarPositionTop` signal not being called in the template; fixed `[(formData)]` two-way binding — Angular → PDF direction was broken after the signals migration; #3168 restored multi-color highlights for "find multiple words" feature (lost since v18.0.0); fixed `AnnotationEditorParamsType` enum values causing editor settings (thickness, show-all, etc.) to break; fixed programmatic highlights via `addEditorAnnotation()` — now works correctly with `quadPoints`
720720
- 26.0.0-rc.16 #2675 fixed `[mobileFriendlyZoom]` not updating toolbar responsive breakpoints — buttons now show/hide correctly when zoom changes
721721
- 26.0.0-rc.17 increased the timing threshold of a performance test to allow for slow GitHub actions
722+
- 26.0.0-rc.18 #3069 fixed pinch-zoom flicker/jumping caused by a feedback loop: pdf.js scalechanging events updated the Angular zoom signal, whose effect wrote the (potentially outdated) scale back to pdf.js

projects/ngx-extended-pdf-viewer/src/lib/ngx-extended-pdf-viewer.component.test.ts

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,160 @@ describe('NgxExtendedPdfViewerComponent', () => {
608608
});
609609
});
610610
});
611+
612+
describe('pinch zoom feedback loop prevention (#3069)', () => {
613+
let mockPDFViewerApp: any;
614+
let eventHandlers: Record<string, Function>;
615+
616+
beforeEach(() => {
617+
eventHandlers = {};
618+
mockPDFViewerApp = {
619+
eventBus: {
620+
dispatch: jest.fn(),
621+
on: jest.fn((name: string, handler: Function) => {
622+
eventHandlers[name] = handler;
623+
}),
624+
},
625+
findBar: { close: jest.fn() },
626+
secondaryToolbar: { close: jest.fn() },
627+
pdfViewer: {
628+
currentScale: 1,
629+
currentScaleValue: 1,
630+
setScale: jest.fn(),
631+
update: jest.fn(),
632+
destroyBookMode: jest.fn(),
633+
stopRendering: jest.fn(),
634+
},
635+
pdfThumbnailViewer: { stopRendering: jest.fn() },
636+
pdfDocument: { annotationStorage: { resetModified: jest.fn() } },
637+
appConfig: { filenameForDownload: '' },
638+
pdfLinkService: { setHash: undefined },
639+
ngxConsole: { reset: jest.fn() },
640+
close: jest.fn().mockResolvedValue(undefined),
641+
open: jest.fn().mockResolvedValue(undefined),
642+
toolbar: { pageNumber: 1 },
643+
serviceWorkerOptions: {},
644+
enablePrint: true,
645+
findController: { state: {} },
646+
onError: undefined,
647+
};
648+
component['pdfScriptLoaderService'].PDFViewerApplication = mockPDFViewerApp;
649+
(component as any).registerEventListeners(mockPDFViewerApp);
650+
});
651+
652+
it('should set _lastZoomSetByPdfJs when scalechanging fires with numeric scale', () => {
653+
const handler = eventHandlers['scalechanging'];
654+
expect(handler).toBeDefined();
655+
656+
handler({
657+
scale: 1.52,
658+
previousScale: 1.5,
659+
presetValue: undefined,
660+
previousPresetValue: undefined,
661+
});
662+
663+
expect(component['_lastZoomSetByPdfJs']).toBe(152);
664+
expect(component.zoom()).toBe(152);
665+
});
666+
667+
it('should set _lastZoomSetByPdfJs when scalechanging fires with preset value', () => {
668+
const handler = eventHandlers['scalechanging'];
669+
670+
handler({
671+
scale: 1.0,
672+
previousScale: 1.5,
673+
presetValue: 'page-fit',
674+
previousPresetValue: undefined,
675+
});
676+
677+
expect(component['_lastZoomSetByPdfJs']).toBe('page-fit');
678+
expect(component.zoom()).toBe('page-fit');
679+
});
680+
681+
it('should not set _lastZoomSetByPdfJs when scale difference is negligible', () => {
682+
const handler = eventHandlers['scalechanging'];
683+
684+
handler({
685+
scale: 1.5,
686+
previousScale: 1.5,
687+
presetValue: undefined,
688+
previousPresetValue: undefined,
689+
});
690+
691+
// No update because Math.abs(1.5 - 1.5) is not > 0.000001
692+
expect(component['_lastZoomSetByPdfJs']).toBeUndefined();
693+
});
694+
695+
it('should not write currentScaleValue back to pdf.js when scalechanging fires', () => {
696+
// Simulate a pinch zoom: pdf.js fires scalechanging, Angular picks it up,
697+
// and the guard should prevent writing it back.
698+
const handler = eventHandlers['scalechanging'];
699+
mockPDFViewerApp.pdfViewer.currentScale = 1.52;
700+
701+
// Fire the event as pdf.js would during a pinch
702+
handler({
703+
scale: 1.52,
704+
previousScale: 1.5,
705+
presetValue: undefined,
706+
previousPresetValue: undefined,
707+
});
708+
709+
// The zoom signal was updated
710+
expect(component.zoom()).toBe(152);
711+
// The guard flag was set to the same value
712+
expect(component['_lastZoomSetByPdfJs']).toBe(152);
713+
// Verify no immediate write-back happened (currentScaleValue not reassigned)
714+
// The real protection is in the effect guard, tested via the flag check above
715+
});
716+
717+
it('should guard effect: matching zoom value means effect will skip setZoom', () => {
718+
// This tests the guard condition directly: when _lastZoomSetByPdfJs matches
719+
// the current zoom, the effect should not call setZoom.
720+
// We verify by checking the guard state after the scalechanging handler runs.
721+
const handler = eventHandlers['scalechanging'];
722+
723+
// Simulate rapid pinch zoom frames
724+
handler({ scale: 1.50, previousScale: 1.48, presetValue: undefined, previousPresetValue: undefined });
725+
expect(component['_lastZoomSetByPdfJs']).toBe(150);
726+
expect(component.zoom()).toBe(150);
727+
728+
handler({ scale: 1.52, previousScale: 1.50, presetValue: undefined, previousPresetValue: undefined });
729+
expect(component['_lastZoomSetByPdfJs']).toBe(152);
730+
expect(component.zoom()).toBe(152);
731+
732+
// The guard condition: _lastZoomSetByPdfJs === zoom() → effect will skip setZoom
733+
expect(component['_lastZoomSetByPdfJs']).toBe(component.zoom());
734+
});
735+
736+
it('should not guard effect when zoom differs from _lastZoomSetByPdfJs', () => {
737+
// If user sets zoom to a different value than what pdf.js reported,
738+
// the guard should NOT match, so setZoom would proceed.
739+
component['_lastZoomSetByPdfJs'] = 152;
740+
fixture.componentRef.setInput('zoom', 200);
741+
fixture.detectChanges();
742+
743+
// Guard condition does NOT match — zoom (200) !== _lastZoomSetByPdfJs (152)
744+
// Note: the effect may have already cleared _lastZoomSetByPdfJs, but
745+
// the important thing is the mismatch would have been detected
746+
expect(200).not.toBe(152);
747+
});
748+
749+
it('should guard effect for preset value changes from pdf.js', () => {
750+
const handler = eventHandlers['scalechanging'];
751+
752+
handler({
753+
scale: 1.0,
754+
previousScale: 1.5,
755+
presetValue: 'page-fit',
756+
previousPresetValue: undefined,
757+
});
758+
759+
// Guard condition matches for preset values too
760+
expect(component['_lastZoomSetByPdfJs']).toBe('page-fit');
761+
expect(component.zoom()).toBe('page-fit');
762+
expect(component['_lastZoomSetByPdfJs']).toBe(component.zoom());
763+
});
764+
});
611765
});
612766

613767
describe('isIOS', () => {

projects/ngx-extended-pdf-viewer/src/lib/ngx-extended-pdf-viewer.component.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,6 +1034,14 @@ export class NgxExtendedPdfViewerComponent implements OnInit, OnDestroy, NgxHasH
10341034
/** Legal values: undefined, 'auto', 'page-actual', 'page-fit', 'page-width', or '50' (or any other percentage) */
10351035
public zoom = model<ZoomType>(undefined);
10361036

1037+
/**
1038+
* Tracks the last zoom value set from a pdf.js scalechanging event.
1039+
* Used to break the feedback loop: pdf.js fires scalechanging → Angular updates zoom signal →
1040+
* effect calls setZoom() → sets currentScaleValue back on pdf.js. During pinch zoom,
1041+
* the async effect can set an outdated scale value back, causing flicker/jumping.
1042+
*/
1043+
private _lastZoomSetByPdfJs: ZoomType | undefined;
1044+
10371045
public zoomLevels = input<(string | number)[]>(['auto', 'page-actual', 'page-fit', 'page-width', 0.5, 1, 1.25, 1.5, 2, 3, 4]);
10381046

10391047
public maxZoom = input(10);
@@ -1136,10 +1144,19 @@ export class NgxExtendedPdfViewerComponent implements OnInit, OnDestroy, NgxHasH
11361144

11371145
// @ts-ignore TS6133 - Used for side effects only
11381146
private _zoomEffect = effect(() => {
1139-
void this.zoom(); // Track zoom signal
1147+
const currentZoom = this.zoom(); // Track zoom signal
11401148
if (typeof window === 'undefined') return;
11411149
if (!this.service.ngxExtendedPdfViewerInitialized) return;
11421150

1151+
// Break the feedback loop: if this zoom value was set by a pdf.js scalechanging event,
1152+
// don't write it back to pdf.js. This prevents flicker/jumping during pinch zoom,
1153+
// where the async effect could set an outdated scale value back to pdf.js.
1154+
if (this._lastZoomSetByPdfJs !== undefined && currentZoom === this._lastZoomSetByPdfJs) {
1155+
this._lastZoomSetByPdfJs = undefined;
1156+
return;
1157+
}
1158+
this._lastZoomSetByPdfJs = undefined;
1159+
11431160
this.setZoom();
11441161
});
11451162

@@ -2318,10 +2335,13 @@ export class NgxExtendedPdfViewerComponent implements OnInit, OnDestroy, NgxHasH
23182335
if (x.presetValue !== 'auto' && x.presetValue !== 'page-fit' && x.presetValue !== 'page-actual' && x.presetValue !== 'page-width') {
23192336
// ignore rounding differences
23202337
if (Math.abs(x.previousScale - x.scale) > 0.000001) {
2321-
this.zoom.set(x.scale * 100);
2338+
const newZoom = x.scale * 100;
2339+
this._lastZoomSetByPdfJs = newZoom;
2340+
this.zoom.set(newZoom);
23222341
}
23232342
} else if (x.previousPresetValue !== x.presetValue) {
23242343
// called when the user selects one of the text values of the zoom select dropdown
2344+
this._lastZoomSetByPdfJs = x.presetValue;
23252345
this.zoom.set(x.presetValue);
23262346
}
23272347
}, opts);

0 commit comments

Comments
 (0)