Skip to content

Commit 97270b7

Browse files
JayPanozJayPanoz
authored
Fix peripheral breaking on iframe loading (#222)
1 parent 607245e commit 97270b7

6 files changed

Lines changed: 71 additions & 20 deletions

File tree

‎navigator-html-injectables/src/helpers/dom.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ export function isInteractiveElement(element: Element | null): boolean {
7979
// Check for interactive roles
8080
if (element.role && interactiveRoles.includes(element.role)) return true;
8181

82+
// An iframe element is a separate browsing context — its focused content cannot be inspected
83+
// cross-origin, so the iframe container itself must not be treated as interactive.
84+
if (element.tagName.toLowerCase() === "iframe") return false;
85+
8286
if ((element as HTMLElement).tabIndex >= 0) return true;
8387

8488
// Use existing interactive tags logic

‎navigator-html-injectables/src/modules/Peripherals.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,15 @@ export class Peripherals extends Module {
537537
ack(true);
538538
});
539539

540+
// Disable keyboard handler when frame is unfocused so that stale focus
541+
// inside a hidden/off-screen iframe doesn't silently swallow key events
542+
// through a halted comms channel. The handler is re-established on the
543+
// next show() cycle via the keyboard_peripherals message below.
544+
this.comms?.register("unfocus", Peripherals.moduleName, (_, ack) => {
545+
this.disableKeyboardPeripherals();
546+
ack(true);
547+
});
548+
540549
// Separate handler for keyboard peripherals
541550
this.comms?.register("keyboard_peripherals", Peripherals.moduleName, (data: unknown, ack) => {
542551
const keyboardPeripherals = data as KeyboardPeripheral[];

‎navigator/src/epub/EpubNavigator.ts‎

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ export class EpubNavigator extends VisualNavigator implements Configurable<Confi
8181
private _css: ReadiumCSS;
8282
private _preferencesEditor: EpubPreferencesEditor | null = null;
8383
private _injector: Injector | null = null;
84+
private _isNavigating = false;
8485
private readonly _readiumRulesPromise: Promise<IInjectableRule[]>;
8586
private readonly _injectablesConfig: IInjectablesConfig;
8687
private readonly _contentProtection: IContentProtectionConfig;
@@ -758,33 +759,45 @@ export class EpubNavigator extends VisualNavigator implements Configurable<Confi
758759
}
759760

760761
public goBackward(_: boolean, cb: (ok: boolean) => void): void {
762+
if(this._isNavigating) { cb(false); return; }
763+
this._isNavigating = true;
761764
if(this._layout === Layout.fixed) {
762-
this.changeResource(-1);
763-
cb(true);
765+
this.changeResource(-1).then((ok) => {
766+
this._isNavigating = false;
767+
cb(ok);
768+
});
764769
} else {
765770
this._cframes[0]?.msg?.send("go_prev", undefined, async (ack) => {
766-
if(ack)
767-
// OK
771+
if(ack) {
772+
this._isNavigating = false;
768773
cb(true);
769-
else
770-
// Need to change resources because we're at the beginning of the current one
771-
cb(await this.changeResource(-1));
774+
} else {
775+
const ok = await this.changeResource(-1);
776+
this._isNavigating = false;
777+
cb(ok);
778+
}
772779
});
773780
}
774781
}
775782

776783
public goForward(_: boolean, cb: (ok: boolean) => void): void {
784+
if(this._isNavigating) { cb(false); return; }
785+
this._isNavigating = true;
777786
if(this._layout === Layout.fixed) {
778-
this.changeResource(1);
779-
cb(true);
787+
this.changeResource(1).then((ok) => {
788+
this._isNavigating = false;
789+
cb(ok);
790+
});
780791
} else {
781792
this._cframes[0]?.msg?.send("go_next", undefined, async (ack) => {
782-
if(ack)
783-
// OK
793+
if(ack) {
794+
this._isNavigating = false;
784795
cb(true);
785-
else
786-
// Need to change resources because we're at the end of the current one
787-
cb(await this.changeResource(1));
796+
} else {
797+
const ok = await this.changeResource(1);
798+
this._isNavigating = false;
799+
cb(ok);
800+
}
788801
});
789802
}
790803
}
@@ -907,8 +920,14 @@ export class EpubNavigator extends VisualNavigator implements Configurable<Confi
907920
return cb(this.listeners.handleLocator(locator));
908921
}
909922

923+
if(this._isNavigating) { cb(false); return; }
924+
this._isNavigating = true;
925+
910926
this.currentLocation = this.positions.find(p => p.href === link!.href)!;
911-
this.apply().then(() => this.loadLocator(locator, (ok) => cb(ok))).then(() => {
927+
this.apply().then(() => this.loadLocator(locator, (ok) => {
928+
this._isNavigating = false;
929+
cb(ok);
930+
})).then(() => {
912931
// Now that we've gone to the right locator, we can attach the listeners.
913932
// Doing this only at this stage reduces janky UI with multiple locator updates.
914933
this.attachListener();

‎navigator/src/epub/frame/FrameManager.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ export class FrameManager {
115115
this.frame.style.opacity = "0";
116116
this.frame.style.pointerEvents = "none";
117117
this.hidden = true;
118+
// Return focus to the parent document so keyboard events aren't silently
119+
// swallowed by a hidden iframe whose comms channel has been halted.
120+
this.frame.blur();
118121
if(this.frame.parentElement) {
119122
if(this.comms === undefined || !this.comms.ready) return;
120123
return new Promise((res, _) => {

‎navigator/src/epub/fxl/FXLFrameManager.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ export class FXLFrameManager {
199199
async unfocus(): Promise<void> {
200200
if(this.frame.parentElement) {
201201
if(this.comms === undefined) return;
202+
// Return focus to the parent document so keyboard events aren't silently
203+
// swallowed by an off-screen iframe whose comms channel has been halted.
204+
this.frame.blur();
202205
return new Promise((res, _) => {
203206
this.comms?.send("unfocus", undefined, (_: boolean) => {
204207
this.comms?.halt();

‎navigator/src/webpub/WebPubNavigator.ts‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export class WebPubNavigator extends VisualNavigator implements Configurable<Web
7272
private _css: WebPubCSS;
7373
private _preferencesEditor: WebPubPreferencesEditor | null = null;
7474
private readonly _injector: Injector | null = null;
75+
private _isNavigating = false;
7576
private readonly _contentProtection: IContentProtectionConfig;
7677
private readonly _keyboardPeripherals: IKeyboardPeripheralsConfig;
7778
private readonly _navigatorProtector: NavigatorProtector | null = null;
@@ -463,14 +464,20 @@ export class WebPubNavigator extends VisualNavigator implements Configurable<Web
463464
}
464465

465466
goBackward(_animated: boolean, cb: (ok: boolean) => void): void {
466-
this.changeResource(-1).then((success) => {
467-
cb(success);
467+
if(this._isNavigating) { cb(false); return; }
468+
this._isNavigating = true;
469+
this.changeResource(-1).then((ok) => {
470+
this._isNavigating = false;
471+
cb(ok);
468472
});
469473
}
470474

471475
goForward(_animated: boolean, cb: (ok: boolean) => void): void {
472-
this.changeResource(1).then((success) => {
473-
cb(success);
476+
if(this._isNavigating) { cb(false); return; }
477+
this._isNavigating = true;
478+
this.changeResource(1).then((ok) => {
479+
this._isNavigating = false;
480+
cb(ok);
474481
});
475482
}
476483

@@ -581,8 +588,14 @@ export class WebPubNavigator extends VisualNavigator implements Configurable<Web
581588
this.currentIndex = index;
582589
}
583590

591+
if(this._isNavigating) { cb(false); return; }
592+
this._isNavigating = true;
593+
584594
this.currentLocation = this.createCurrentLocator();
585-
this.apply().then(() => this.loadLocator(locator, (ok) => cb(ok))).then(() => {
595+
this.apply().then(() => this.loadLocator(locator, (ok) => {
596+
this._isNavigating = false;
597+
cb(ok);
598+
})).then(() => {
586599
// Now that we've gone to the right locator, we can attach the listeners.
587600
// Doing this only at this stage reduces janky UI with multiple locator updates.
588601
this.attachListener();

0 commit comments

Comments
 (0)