[whosawhatsis]

Fixes #

Part 1 of 2. # is stacked on this branch and adds a ttsSubSupEnabled
preference (default on) as an opt-out for the behaviour change noted at the bottom.
This PR stands alone if you would rather not take the preference.

What

<sub> and <sup> text was never spoken by read-aloud. In scientific prose this
does not merely sound awkward, it changes the statement: πr<sup>2</sup> was read
as "pi r", v<sub>1</sub> as "v", SO<sub>4</sub><sup>2-</sup> as "SO".

Three changes in
src/r2-xxx-js/r2-navigator-js/electron/renderer/common/dom-text-utils.ts:

1. "sup"/"sub" removed from _doNotProcessDeepChildTagNames, with
   !isSubSup added to the deep-dive condition. The elements are now descended
   into like any other inline element, which keeps their text in the surrounding
   utterance — v<sub>1</sub> reads as one phrase, and the textNodes array stays
   populated so word-level highlighting keeps working. (Pushing a separate queue
   item, as img does, would have fragmented every sentence containing an index.)

2. An aria-label / title override, following the existing
   linkNeedsDeepDive idiom, so authors can supply better phrasing:
   <sup aria-label="squared">2</sup>. Consistent with the math branch a few
   lines below, which prefers alttext and falls back to textContent.

3. "noteref" added to _skippables. This is what makes (1) safe: footnote
   reference markers were previously suppressed only as a side effect of the tag
   blacklist. _skippables listed note bodies (footnote, endnote, note,
   rearnote) but never the reference. computeEpubTypes already resolves
   epub:type="noteref" and normalises role="doc-noteref" → noteref, so only
   the list entry was missing. Correctly-marked references stay silent, now via the
   mechanism intended for it and under the user's existing skippability control.

Why not just add an alt-text fallback and leave the blacklist alone

That would have left every existing book silent, since the fallback only fires when
the attribute is present — the fix would have required every publisher to re-mark
their content. The math branch already establishes the better pattern in this same
function: attribute if provided, content otherwise.

Verification

Confirmed by ear in a running build (this branch on develop), on a real EPUB, for
both fusion classes:

passage	before	after
…two atoms of oxygen (written O<sub>2</sub>), we have 6.0214076 * 10<sup>23</sup>…	"written O … times 10"	"written O 2 … times 10 23"
…relative sizes of r, R<sub>o</sub>, and R<sub>i</sub>… R<sub>o</sub> = 0.05 m, R<sub>i</sub> = 0.04 m	"r, R and R … R = 0.05 m, R = 0.04 m"	"r, R o, and R i … R o = 0.05, R i = 0.04"

A small test EPUB covering all seventeen cases is linked from # (with the
script that generates it). Running generateTtsQueue() over that file from develop
and from this branch gives:

test	develop	this branch
1 v₁ v₂	"v and v"	"v 1 and v 2"
2 R_o R_i	"R and R"	"R o and R i"
3 t_o	"t"	"t o"
4 10²³	"10"	"10 23"
6 H₂O	"HO"	"H 2O"
7 x₁²	"x"	"x 1 2"
8 aria-label="squared"	"x"	"x" + "squared"
10 epub:type="noteref"	silent	silent
11 role="doc-noteref"	silent	silent
12 bare <sup> marker	silent	"3"
13 display:none	silent	silent
14 plain prose	unchanged	unchanged

Tests 10, 11, 13 and 14 are the regression cases and are unaffected. Test 12 is the
single behaviour change, covered by the preference in #.

Plus:

• tsc --noEmit -p tsconfig.json — exit 0, zero diagnostics
• eslint on the changed file — clean, identical to baseline
• Behavioural diff: generateTtsQueue() bundled from this branch and from
  develop, run against jsdom fixtures, comparing combinedText. At the default
  setting (skippability on):

fixture	before	after
&#x3C0;<em>r</em><sup>2</sup>	"the area is πr exactly"	"the area is πr2 exactly"
<em>v</em><sub>1</sub> … <sub>2</sub>	"compare v with v"	"compare v1 with v2"
SO<sub>4</sub><sup>2-</sup>	"the SO ion"	"the SO42- ion"
&#x3BC;<sub>s</sub>	"use μ for static"	"use μs for static"
<sup aria-label="squared">2</sup>	"the area is r here"	"the area is r", "squared", "here"
<sup><a epub:type="noteref">1</a></sup>	"as shown earlier"	unchanged
<sup><a role="doc-noteref">1</a></sup>	"as shown earlier"	unchanged
<sup><a href="#fn1">1</a></sup>	"as shown earlier"	"as shown1 earlier"
<sup style="display:none">9</sup>	"visible text"	unchanged

I can contribute the fixture harness as a jest test if you want it in-tree; it
needs jest-environment-jsdom, which isn't currently a devDependency, so I left it
out of this PR rather than expand the dependency set unasked.

Why removing the tags from the list is not sufficient on its own (commit 3)

combineTextNodes() joins raw nodeValues with no separator, so a base and its
sub/superscript reach the speech engine as one token:

markup	on develop	naively spoken	with commit 3
10<sup>23</sup>	"ten"	"one thousand twenty-three"	"ten, twenty-three"
<em>R</em><sub>o</sub>	"R"	"Ro" (one syllable)	"R o"
<em>t</em><sub>o</sub>	"t"	"to" (the word)	"t o"
<em>x</em><sup>2</sup>	"x"	"x 2" ✔	"x 2" ✔

Measured on a 12-chapter physics textbook: of 172 prose sub/sup sites, 8 fused
into a false number and 40 into a false word. That is 28% of sites producing
confidently wrong output — which is very likely why sup/sub were put on
_doNotProcessDeepChildTagNames to begin with.

This is not platform-specific. Measured on all three desktop platforms, each
through the Web Speech API so it is that platform's real engine — macOS Samantha,
espeak-ng via speech-dispatcher on Linux, SAPI Speech_OneCore under Chrome on Windows:

utterance	macOS	espeak-ng	SAPI
we have 1023 atoms	2.198s	2.514s	3.298s
we have one thousand twenty three atoms	2.310s	2.546s	3.087s
we have 10 23 atoms	1.860s	2.108s	2.677s
compare Ro with Ri / compare R o with R i	1.303s / 1.689s	1.458s / 1.809s	2.086s / 2.216s
the time to matters / the time t o matters	1.306s / 1.540s	1.404s / 1.596s	1.996s / 2.266s

On every engine 1023 matches the spelled-out number rather than the split digits, and
the fused letters run shorter than the separated ones — the digits are parsed as one
number and the letters as one syllable everywhere, so the separator is needed on all
three. It reads as a blunt guard against
garbage rather than an oversight, so this PR would not be defensible without
addressing it.

So text inside sup/sub now contributes a leading SUBSUP_SEPARATOR when the
utterance string is assembled. The separator exists only in that string, never in
the DOM, so rendering, CFIs and locators are untouched.

Why a plain space, and not a zero-width character

U+FEFF is the obvious alternative, and on macOS it works exactly as well — timing
synthesised speech, 10<FEFF>23 and 10 23 come out within 4 ms of each other in
every case I tried. I still went with a space, for two reasons.

It cannot be discarded. U+FEFF is Default_Ignorable_Code_Point, so an engine that
strips it before tokenising is doing what the character's definition invites, not
misbehaving. Were an engine to do that, this fix would silently do nothing there and
the failure would look exactly like the bug it was meant to fix. A space has no such
licence anywhere in the chain.

And the separator is visible to the user in a place where visibility helps. The
utterance string is what the read-aloud panel displays, and that panel shows what is
about to be spoken, not what the book looks like — the sub/sup distinction is already
gone from it. H 2 O truthfully shows the separation being applied; H2O would render
identically whether the subscript were separated, fused, or dropped altogether, which is
precisely the ambiguity that made this bug hard to notice in the first place.

This follows the existing __RUBY convention exactly: tag the text node, and have
combineTextNodes() and the two offset walkers in readaloud.ts agree on the length
that node contributes. The walkers subtract the separator back out when converting an
utterance offset into a DOM offset, so word- and sentence-level highlighting stay
aligned. I verified that by replicating the walker and mapping every utterance
offset back to its DOM character across fusion, non-fusion and nested-element
fixtures — 0 mismatches.

Note this makes the utterance say "ten, twenty-three" rather than "ten to the
twenty-third". Correct pronunciation of an exponent needs semantics, which is what
the aria-label override in commit 2 is for; the separator's job is only to stop the
reader inventing a different number.

Behaviour change / release note

Note references marked up as a bare <sup> with no epub:type or role will now
have their marker spoken. Correctly-marked references are unaffected at the default
setting, since "noteref" is now in _skippables and skippability defaults to on
(mediaOverlaysEnableSkippability: true drives ttsSkippabilityEnable).

# adds a ttsSubSupEnabled preference (default on) as an explicit
opt-out for that case; switching it off reproduces current develop output
byte-for-byte on every fixture above.

The aria-label override emits its own queue item, so it splits the sentence into
three utterances — the same as the existing img/link/pagebreak overrides, but more
audible inline. Change 2 can be dropped independently of 1 and 3 if you would
rather not have that.

[danielweck]

Thank you very much!
We will have a closer look once version 3.5.0 is released :)
(imminently)

[danielweck]

Could we just remove the pendingSubSupSeparator global state and instead test lowerTagName === "sup" || lowerTagName === "sup" here?
(see above const lowerTagName = textNode.parentElement?.tagName?.toLowerCase();)

[danielweck]

I think that the !isRUBY condition is superfluous here, as the TextNode cannot have both ruby/rb and sup/sub parent Elements.

[danielweck]

I appreciate that "readability" is subjective, but for future code reviewers I think it would be better if the existing isRUBY ternary was expanded with isSUBSUP (this is perhaps even more obvious further below, where the proposed Min/Max statements are hard to correlate intuitively with the isSUBSUP case vs. the general case):

I think this captures the intent more obviously, at a quick glance:

const l = isRUBY ?
         0 :
         isOnlyWhiteSpace(txtNode.nodeValue) ?
         1 :
         isSUBSUP ?
         (txtNode.nodeValue.length + 2) :
         txtNode.nodeValue.length

[danielweck]

May I propose this simpler cascading expression which offers the advantage of not altering the existing implementation (i.e. the non-isSUBSUP case):

rangeStartOffset =
       isRUBY ?
       0 :
       isSUBSUP ?
       Math.min(Math.max(l - 1 - (acc - charIndexAdjusted), 0), txtNode.nodeValue.length) :
       (l - (acc - charIndexAdjusted));

[danielweck]

rangeEndOffset =
       isRUBY ?
       (txtNode.nodeValue.length - 1) :
       isSUBSUP ?
       Math.min(Math.max(l - 1 - (acc - charIndexEnd), 0), txtNode.nodeValue.length) :
       (l - (acc - charIndexEnd));

[whosawhatsis]

Thank you for the detailed review — all five comments are applied and the branch is rebased onto develop @ 6af3629c. Two of them I want to comment on specifically, because one caught a real bug and the other I implemented differently from what you suggested.

The cascading-ternary shape (comments 3–5): you caught an actual regression, not just a readability issue. My version wrapped Math.min/Math.max around the offset for every text node, which quietly changed the existing non-sub/sup path — l - (acc - charIndex) was no longer what it is on develop. Your shape confines the clamp to the isSUBSUP branch and leaves everything else byte-identical. Adopted verbatim in both walkers.

The pendingSubSupSeparator state (comment 1): removed, but keyed off closest("sup, sub") rather than a direct parent-tag test. The module state is gone and the tagging now happens next to the __RUBY tagging as you suggested. But the direct lowerTagName test silently regresses inline markup inside a sup/sub, because then the text node's parent is the inner element:

<p>see <em>y</em><sup><em>n</em></sup> there</p>
  direct parent test : "see yn there"     <- fused again
  closest()          : "see y n  there"

<sup><a href="#fn1">1</a></sup> is affected the same way, which matters since that is very common markup for footnote markers. So:

if (textNode.nodeValue.trim().length && textNode.parentElement?.closest("sup, sub")) {
    (textNode as any).__SUBSUP = true;
}

Happy to switch to the direct test if you would rather keep it strictly parallel to __RUBY and treat nested markup as out of scope — it is your call, I just did not want to introduce that gap silently.

Comment 2 (!isRUBY superfluous): dropped, though the reasoning needed adjusting. Your point holds for the direct-parent version, where the two flags genuinely cannot coexist. With closest() they can — <sup><ruby>…</ruby></sup> sets both. Dropping the guard is still correct, but for a different reason: isRUBY is tested first in every cascade, and combineTextNodes() skips __RUBY nodes outright, so such a node contributes 0 either way. I have replaced my (now wrong) "mutually exclusive by construction" comment with that explanation.

Comment 2's premise, verified: a text node cannot have both a ruby/rb and a sup/sub direct parent, agreed.

After the pass:

• tsc --noEmit: one error, in src/renderer/library/components/App.tsx (a react-dropzone noPaste prop) — that file is byte-identical to develop, so it is pre-existing and not from this branch
• eslint clean on both changed files
• all 17 cases of the test EPUB in Read-aloud silently discards all <sub> and <sup> text content #3778 produce the expected utterances
• every utterance offset still maps back to the correct DOM character across fusion, nesting, and the untouched cases — 0 mismatches
• nested cases (<sup><em>, <sup><a>, <sub><span>, two text nodes in one sup) added to my test harness so the above regression cannot come back

No rush on this given 3.5.0 — just flagging that the branch is updated so you are not reviewing the old diff.