EpubNavigator.load() only restores the initial locator by progression, ignoring text/cssSelector — go()'s own text/cssSelector search can silently land on the wrong page if called right after load() resolves

[oscarhn27]

Title

EpubNavigator.load() only restores the initial locator by progression, ignoring text/cssSelector — and go()'s own text/cssSelector search can silently land on the wrong page if called right after load() resolves

Summary

When constructing an EpubNavigator with an initialPosition Locator that carries text.highlight (and/or a cssSelector in locations.otherLocations), calling .load() alone never attempts a text- or selector-based match — it only ever applies locations.progression (a float relative to the current column/viewport width). Precise restoration only happens through a separate .go(initialPosition, ...) call, which internally hits loadLocator()'s go_text/go_id cascade.

In our app we call .go() right after .load() resolves (once we know the saved locator has text), expecting loadLocator()'s go_text step to refine the position precisely. In practice, this refinement intermittently lands on the page before the intended one — never the chapter start, always just one page early, and reproducible without any window/device change between save and restore. Adding an artificial multi-second delay between load() resolving and calling go() reliably fixes it. This points to go()/loadLocator()'s text/cssSelector search running against a layout that hasn't fully settled yet (fonts still loading, CSS columns not yet computed at their final width) at the moment load()'s promise resolves — getBoundingClientRect() on the matched range then measures against a transient layout, and ColumnSnapper.snapOffset()'s floor-to-page-width rounding turns a few stale pixels into a full page offset.

Environment

• @readium/navigator: 2.8.2
• @readium/navigator-html-injectables: 2.6.3
• @readium/shared: 2.4.0
• Reflowable EPUB, paginated (column) mode, 2 columns per screen.

Repro

1. Save a Locator mid-chapter while reading, with a real text.highlight/before/after captured from the currently visible paragraph (and, in our later attempt, a cssSelector obtained via the first_visible_locator comms message).
2. Construct a fresh EpubNavigator with that Locator as initialPosition, await navigator.load(), then immediately call navigator.go(initialPosition, false, cb).
3. Observe the resulting page: it's the page immediately before the one that was actually visible when the locator was saved — not chapter start, no window/device change, no diff in text.highlight/before/after between save and restore.
4. Insert an artificial delay (we tried a few seconds) between step 2's load() resolving and calling go(): the restore becomes exact.

Root cause analysis (source-level, version 2.8.2/2.6.3)

• EpubNavigator.load() (epub/EpubNavigator.ts): sets this.currentLocation = initialPosition (already done in the constructor) and calls apply() → framePool.update(pub, this.currentLocator, modules), which ends in newFrame.show(locator.locations.progression). This is purely progression-based — no text/cssSelector/fragment matching happens here at all.
• Text/cssSelector-based refinement only exists in go() → loadLocator():

  private async loadLocator(locator: Locator, cb: (ok: boolean) => void) {
      let done = false;
      let cssSelector = getCssSelector(locator.locations);
      if (locator.text?.highlight) {
          done = await new Promise<boolean>((res) => {
              this._cframes[0]!.msg!.send("go_text", cssSelector ? [locator.text?.serialize(), cssSelector] : locator.text?.serialize(), (ok) => res(ok));
          });
      } else if (cssSelector) { /* go_text with cssSelector only */ }
      if (done) { cb(done); return; }
      /* ...go_id, then go_progression as last resort... */
  }

  This is only reached via go(), which the app must call explicitly.
• go_text's handler in ColumnSnapper.ts resolves the target Range via rangeFromLocator() (Hypothesis-style TextQuoteAnchor, scoped to the cssSelector's element when present) and then does:

  this.doc().scrollLeft = this.snapOffset(r.getBoundingClientRect().left + wnd.scrollX);

  snapOffset() floors to the nearest multiple of wnd.innerWidth (the page/column-pair width):

  snapOffset(offset: number) {
      const value = offset + (this.rtl ? -1 : 1);
      return value - (value % this.wnd.innerWidth);
  }

  If getBoundingClientRect() is measured before column width/line metrics have settled (fonts still loading, ReadiumCSS's commitCSS/updateCSS not yet applied, ResizeObserver not yet fired once), the returned left can be off by a few pixels relative to the final layout — enough for snapOffset's floor to round down to the previous page instead of the correct one. Since go()'s call to loadLocator() happens synchronously right after apply() resolves (itself right after load()'s own apply()), and neither waits on document.fonts.ready or an actual settled-layout signal, this race is possible on real content/devices even though load()'s promise has already resolved.

Suggested directions (open to whatever fits your architecture — see also #24 re: go_text/go_id living in JumpToLocation)

1. Have load() itself attempt the same text/cssSelector refinement loadLocator() does for the initial locator, once its own layout is confirmed settled — so callers don't need a separate go() call (and don't need to guess when it's safe to call it).
2. Alternatively, expose an explicit "content stable" signal/promise (e.g. resolved after the frame's fonts.ready and one paint cycle) that callers can await before invoking go() for an initial restore, instead of leaving it to a race against load()'s promise.
3. At minimum, document that load() is progression-only and that callers needing text/cssSelector-accurate initial restoration must call go() afterward and that doing so immediately can race a settling layout — so it's not just our app that trips over this silently.

Happy to help test/validate a fix, or to turn this into a PR once there's agreement on which direction fits the codebase (e.g. per #24's plan to move go_text/go_id into a JumpToLocation module).

[chocolatkey]

This is being worked on in #198 , I will be making changes regarding it soon

[oscarhn27]

Thank you for your response. I wait to close the issue when test the release with the fix.