diff --git a/.gitignore b/.gitignore index 83fd7f282c74..075093c8045e 100644 --- a/.gitignore +++ b/.gitignore @@ -204,3 +204,7 @@ critiqueAI.json pr-comments*.md *.stackdump + +# Per-developer Claude Code settings (the shared .claude/CLAUDE.md stays tracked). +.claude/settings.json +.claude/settings.local.json diff --git a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts index 257e80cfef2c..90fb61900414 100644 --- a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts +++ b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts @@ -578,8 +578,12 @@ export default class StyleEditor { create: boolean, documentToUse: Document = document, ): CSSStyleRule | null { - const styleSheet = - this.GetOrCreateUserModifiedStyleSheet(documentToUse); + // When we are only reading (create === false), do not create a userModifiedStyles + // sheet as a side effect: a caller looking up a rule that isn't there should not + // mutate the document. Only create the sheet when we actually intend to add a rule. + const styleSheet = create + ? this.GetOrCreateUserModifiedStyleSheet(documentToUse) + : this.FindExistingUserModifiedStyleSheet(documentToUse); if (styleSheet == null) { return null; } @@ -685,7 +689,10 @@ export default class StyleEditor { } } - public getAudioHiliteProps(styleName: string): { + public getAudioHiliteProps( + styleName: string, + documentToUse: Document = document, + ): { hiliteTextColor: string | undefined; hiliteBgColor: string; } { @@ -694,9 +701,12 @@ export default class StyleEditor { // The two should have the same content, so for reading, we only need one. this.sentenceHiliteRuleSelector, false, + documentToUse, ); - const hiliteTextColor = sentenceRule?.style?.color; - let hiliteBgColor = sentenceRule?.style?.backgroundColor; + const hiliteTextColor = + sentenceRule?.style?.getPropertyValue("color") || undefined; + let hiliteBgColor = + sentenceRule?.style?.getPropertyValue("background-color"); if (!hiliteBgColor) { hiliteBgColor = kBloomYellow; } diff --git a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts index cfb8b561bd83..266adcc860ca 100644 --- a/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts +++ b/src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditorSpec.ts @@ -51,14 +51,12 @@ function ChangeSizeAbsolute( editor.changeSizeInternal(newSize + "pt", shouldSetDefaultRule); } -function GetUserModifiedStyleSheet(): any { +function GetUserModifiedStyleSheet(): CSSStyleSheet | undefined { for (let i = 0; i < document.styleSheets.length; i++) { - if (document.styleSheets[i].title == "userModifiedStyles") + if (document.styleSheets[i].title === "userModifiedStyles") return document.styleSheets[i]; } - // this is not a valid constructor - //return new CSSStyleSheet(); - return {}; + return undefined; } function GetFooStyleRuleFontSize(): number { @@ -83,8 +81,8 @@ function ParseRuleForFontSize(ruleText: string): number { } function GetRuleForFooStyle(): CSSRule | null { - const x: CSSRuleList = (GetUserModifiedStyleSheet()) - .cssRules; + const x = GetUserModifiedStyleSheet()?.cssRules; + if (!x) return null; for (let i = 0; i < x.length; i++) { if (x[i].cssText.indexOf("foo-style") > -1) { @@ -95,8 +93,7 @@ function GetRuleForFooStyle(): CSSRule | null { } function GetRuleForNormalStyle(): CSSRule | null { - const x: CSSRuleList = (GetUserModifiedStyleSheet()) - .cssRules; + const x = GetUserModifiedStyleSheet()?.cssRules; if (!x) return null; for (let i = 0; i < x.length; i++) { @@ -108,8 +105,7 @@ function GetRuleForNormalStyle(): CSSRule | null { } function GetRuleForCoverTitleStyle(): CSSRule | null { - const x: CSSRuleList = (GetUserModifiedStyleSheet()) - .cssRules; + const x = GetUserModifiedStyleSheet()?.cssRules; if (!x) return null; for (let i = 0; i < x.length; i++) { if (x[i].cssText.indexOf("Title-On-Cover-style") > -1) { @@ -128,8 +124,9 @@ function GetCalculatedFontSize(target: string): number { } function GetRuleMatchingSelector(selector: string): CSSRule | null { - const x = (GetUserModifiedStyleSheet()).cssRules; - const count = 0; + const sheet = GetUserModifiedStyleSheet(); + const x = sheet?.cssRules; + if (!x) return null; for (let i = 0; i < x.length; i++) { if (x[i].cssText.indexOf(selector) > -1) { return x[i]; @@ -139,7 +136,9 @@ function GetRuleMatchingSelector(selector: string): CSSRule | null { } function HasRuleMatchingThisSelector(selector: string): boolean { - const x = (GetUserModifiedStyleSheet()).cssRules; + const sheet = GetUserModifiedStyleSheet(); + const x = sheet?.cssRules; + if (!x) return false; let count = 0; for (let i = 0; i < x.length; i++) { if (x[i].cssText.indexOf(selector) > -1) { @@ -150,8 +149,8 @@ function HasRuleMatchingThisSelector(selector: string): boolean { } function countFooStyleRules(): number { - const x: CSSRuleList = (GetUserModifiedStyleSheet()) - .cssRules; + const x = GetUserModifiedStyleSheet()?.cssRules; + if (!x) return 0; let count = 0; for (let i = 0; i < x.length; i++) { @@ -320,6 +319,69 @@ describe("StyleEditor", () => { if (rule != null) expect(ParseRuleForFontSize(rule.cssText)).toBe(20); }); + it("putAudioHiliteRulesInDom stores audio highlight legacy properties", () => { + const editor = new StyleEditor( + "file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit", + ); + + const sentenceSelector = "foo-style span.ui-audioCurrent"; + const paddedSentenceSelector = + "foo-style span.ui-audioCurrent > span.ui-enableHighlight"; + const paragraphSelector = "foo-style.ui-audioCurrent p"; + + // sanity check that the rules do not yet exist + expect(HasRuleMatchingThisSelector(sentenceSelector)).toBeFalsy(); + expect(HasRuleMatchingThisSelector(paddedSentenceSelector)).toBeFalsy(); + expect(HasRuleMatchingThisSelector(paragraphSelector)).toBeFalsy(); + + editor.putAudioHiliteRulesInDom( + "foo-style", + "rgb(1, 2, 3)", + "rgb(4, 5, 6)", + ); + + const sentenceRule = GetRuleMatchingSelector(sentenceSelector); + const paddedSentenceRule = GetRuleMatchingSelector( + paddedSentenceSelector, + ); + const paragraphRule = GetRuleMatchingSelector(paragraphSelector); + + expect(sentenceRule?.cssText).toContain( + "background-color: rgb(4, 5, 6)", + ); + expect(sentenceRule?.cssText).toContain("color: rgb(1, 2, 3)"); + expect(paddedSentenceRule?.cssText).toContain( + "background-color: rgb(4, 5, 6)", + ); + expect(paddedSentenceRule?.cssText).toContain("color: rgb(1, 2, 3)"); + expect(paragraphRule?.cssText).toContain( + "background-color: rgb(4, 5, 6)", + ); + expect(paragraphRule?.cssText).toContain("color: rgb(1, 2, 3)"); + }); + + it("getAudioHiliteProps reads colors from audio highlight legacy properties", () => { + const editor = new StyleEditor( + "file://" + "C:/dev/Bloom/src/BloomBrowserUI/bookEdit", + ); + + const origProps = editor.getAudioHiliteProps("foo-style"); + + expect(origProps?.hiliteTextColor).not.toBe("rgb(1, 2, 3)"); + expect(origProps?.hiliteBgColor).not.toBe("rgb(4, 5, 6)"); + + editor.putAudioHiliteRulesInDom( + "foo-style", + "rgb(1, 2, 3)", + "rgb(4, 5, 6)", + ); + + const props = editor.getAudioHiliteProps("foo-style"); + + expect(props.hiliteTextColor).toBe("rgb(1, 2, 3)"); + expect(props.hiliteBgColor).toBe("rgb(4, 5, 6)"); + }); + // Skipped because currently we're running in jsdom. Making use of the existing rule depends on // getComputedStyle, which jsdom does not support. ChatGpt thinks it also depends on actual // dom element sizes, which jsdom also does not support. Attempts to polyfill proved difficult. diff --git a/src/BloomBrowserUI/bookEdit/css/editMode.less b/src/BloomBrowserUI/bookEdit/css/editMode.less index 0732fca8181a..b7a956134b73 100644 --- a/src/BloomBrowserUI/bookEdit/css/editMode.less +++ b/src/BloomBrowserUI/bookEdit/css/editMode.less @@ -1036,15 +1036,40 @@ even when the div is focused (as long as it is empty).*/ color: @bloom-buff; } +// BL-11633: Works around a Chromium bug where an element with position:relative and a +// background color makes the caret disappear. The bug can trigger on inline text-formatting +// elements at any level of nesting, so this unsets position on those children wherever they +// appear. Use this (rather than .bl11633-unset-position()) on BLOCK containers such as +// div.bloom-editable and p: those must keep their own position:relative, because the little +// language-code tags and paragraph/line-break markers are positioned relative to them and +// would otherwise jump to the wrong corner. +.bl11633-unset-position-on-children() { + em, + i, + strong, + span[style], + u, + sup { + position: unset; + } +} +// For an INLINE element that is itself background-colored (e.g. a decodable/leveled-reader +// violation span, or the sentence span that is the ::highlight range), also unset position +// on the element itself. Do not use this on block containers - see the note above. +.bl11633-unset-position() { + position: unset; + .bl11633-unset-position-on-children(); +} + /*This block handles marking elements that violate decodable book and leveled reader constraints*/ span.sentence-too-long { background-color: @LeveledReaderViolationColor; - position: unset; // BL-11633, works around Chromium bug + .bl11633-unset-position(); } span.word-too-long { background-color: @bloom-lightblue; - position: unset; // BL-11633, works around Chromium bug + .bl11633-unset-position(); } .page-too-many-words-or-sentences .marginBox { @@ -1068,21 +1093,24 @@ span.sight-word { span.word-not-found { background-color: @DecodableReaderViolationColor; - position: unset; // BL-11633, works around Chromium bug + .bl11633-unset-position(); } -div.bloom-editable { - // Chrome 105 and later (ie, WebView2) has the :has selector. - // These rules may not need the :has(span) selector, but I - // don't want to risk breaking something. This is enough to - // work around the Chromium bug reported in BL-11633. - span[style]:has(span), - em:has(span), - strong:has(span), - u:has(span), - sup:has(span) { - position: unset; // BL-11633, works around Chromium bug - } +// div.bloom-editable and p are block-level and carry the language-code tags and paragraph/ +// line-break markers, which are positioned relative to them, so they must keep their own +// position:relative; only their inline children need position:unset. (In the edit tab the +// current highlight is a ::highlight pseudo-element drawn with text-decoration, not a +// background color, so the block itself does not trigger the BL-11633 caret bug.) +div.bloom-editable, +div.bloom-editable p { + .bl11633-unset-position-on-children(); +} + +// Sentence spans / highlight segments are inline and are themselves the ::highlight range in +// sentence mode, so they (and their inline children) need position:unset on the element itself. +div.bloom-editable p > .audio-sentence, +div.bloom-editable p > .bloom-highlightSegment { + .bl11633-unset-position(); } /* We are disabling the "Possible Word" feature at this time. @@ -1225,10 +1253,10 @@ body { // and a background color cause the caret not to show up. Somehow, this // triggers when a decodable reader error span is nested inside one of these // spans, even though when DR is active the rules that give these segments -// background color are not active. Nothing depends on thei being position:relative, +// background color are not active. Nothing depends on their being position:relative, // so it's easiest to just prevent it. See BL-11633 for reproduction steps in Bloom 5.5. .bloom-highlightSegment { - position: unset; + .bl11633-unset-position(); } // Don't show text over picture borders, resizing handles, or format buttons unless hovering over the image diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/AdjustTimingsDialog.tsx b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/AdjustTimingsDialog.tsx index b5a5d5794f39..31ee121ce124 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/AdjustTimingsDialog.tsx +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/AdjustTimingsDialog.tsx @@ -20,7 +20,6 @@ import { } from "../../../react_components/BloomDialog/commonDialogComponents"; import { useL10n } from "../../../react_components/l10nHooks"; import { getAsync, postBoolean, postJsonAsync } from "../../../utils/bloomApi"; -import { kAudioCurrent } from "./audioRecording"; import { AdjustTimingsControl, TimedTextSegment } from "./AdjustTimingsControl"; import { getUrlPrefixFromWindowHref, @@ -40,6 +39,7 @@ const timingsMenuId = "timingsMenuAnchor"; export const AdjustTimingsDialog: React.FunctionComponent<{ dialogEnvironment?: IBloomDialogEnvironmentParams; + currentTextBox: HTMLElement; split: (timingFilePath: string) => Promise; editTimingsFile: (timingsFilePath?: string) => Promise; applyTimingsFile: (timingsFilePath?: string) => Promise; @@ -154,7 +154,7 @@ export const AdjustTimingsDialog: React.FunctionComponent<{ // if we have one. This really wants to not happen again, since it would discard any changes // the user has made. setAudioRecordingEndTimes( - getCurrentTextBox()?.getAttribute("data-audiorecordingendtimes"), + props.currentTextBox?.getAttribute("data-audiorecordingendtimes"), ); // This is supposed to execute exactly once, when the dialog is first opened. // React insists it must have this dependency, even though I set up a useCallback @@ -167,7 +167,7 @@ export const AdjustTimingsDialog: React.FunctionComponent<{ // will be passed to the control to tell it to fine tune the segments based on the audio. // gets set back to false when the control sends us the adjusted times. setShouldAdjustSegments(true); - const bloomEditable = getCurrentTextBox(); + const bloomEditable = props.currentTextBox; const segmentElements = Array.from( bloomEditable.getElementsByClassName(kHighlightSegmentClass), @@ -180,9 +180,9 @@ export const AdjustTimingsDialog: React.FunctionComponent<{ React.useEffect(() => { if (!propsForBloomDialog.open) return; - const bloomEditable = getCurrentTextBox(); + const bloomEditable = props.currentTextBox; async function getTimingsFileData() { - setTimingsFilePath(await getTimingsFileName()); + setTimingsFilePath(await getTimingsFileName(props.currentTextBox)); } getTimingsFileData(); const ff = ( @@ -361,7 +361,11 @@ export const AdjustTimingsDialog: React.FunctionComponent<{ l10nId="EditTab.Toolbox.TalkingBookTool.EditTimingsFile" onClick={() => { closeMoreMenu(); - exportTimingsFile(timingsFilePath!, endTimes); + exportTimingsFile( + timingsFilePath!, + endTimes, + props.currentTextBox, + ); props.editTimingsFile(timingsFilePath); }} icon={} @@ -387,9 +391,9 @@ export const AdjustTimingsDialog: React.FunctionComponent<{ { - // Update the data-audiorecordingendtimes attribute in the getCurrentTextBox() div to match + // Update the data-audiorecordingendtimes attribute in the props.currentTextBox div to match // the current state of the adjustments. - const bloomEditable = getCurrentTextBox(); + const bloomEditable = props.currentTextBox; bloomEditable.setAttribute( "data-audiorecordingendtimes", endTimes.join(" "), @@ -412,6 +416,7 @@ let show: () => void = () => { }; export function showAdjustTimingsDialog( + currentTextBox: HTMLElement, split: (timingFilePath: string) => Promise, editTimingsFile: (timingsFilePath?: string) => Promise, applyTimingsFile: (timingsFilePath?: string) => Promise, @@ -420,6 +425,7 @@ export function showAdjustTimingsDialog( try { renderRootSync( { - const bloomEditable = getCurrentTextBox(); +async function getTimingsFileName( + currentTextBox: HTMLElement, +): Promise { + const bloomEditable = currentTextBox; const fileName = `audio/${bloomEditable.getAttribute("id")}_timings.txt`; // id should be a guid, so should not need encoding. const result = await postJsonAsync("fileIO/completeRelativePath?", { @@ -514,8 +512,9 @@ const computeSegments = ( const exportTimingsFile = async ( timingsFileName: string, endTimes: number[], + currentTextBox: HTMLElement, ) => { - const bloomEditable = getCurrentTextBox(); + const bloomEditable = currentTextBox; const segmentElements = Array.from( bloomEditable.getElementsByClassName(kHighlightSegmentClass), ) as HTMLElement[]; diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.less b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.less index b87d9d5ec44e..1ebbd9f7d567 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.less +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.less @@ -10,99 +10,69 @@ @textOnLightBackground: black; @disablingOverlayZindex: 1001; // higher than #audio-devlist -// See BL-7442: When low line height causes lines to overlap, the highlighting overlaps and -// can over up the bottom of the characters in the line above. For Chrome/Safari we have -// a fix that can be found in bloom-player. But those don't work in Firefox yet (as of FF 68). -// So we deal with the most common case of overlap (the book title on the cover) by just -// treating the text as a rectangle. And it's fine that this will show this way in Chrome too: -.Title-On-Cover-style - span.ui-audioCurrent:not(.ui-suppressHighlight):not(.ui-disableHighlight) { - display: inline-block; - white-space: pre-wrap; -} - -span.ui-audioCurrent:not(.ui-suppressHighlight):not(.ui-disableHighlight), -div.ui-audioCurrent:not(.ui-suppressHighlight):not(.ui-disableHighlight) p { - // This behavior is now achieved by a default rule in content/basePage-shared.less, - // from whence it can work in various players and be overridden by our new audio-hightlighting - // control. The negation for suppress/disable is now handled by a separate rule below. - // background: @highlightColor; - // /* make highlighted text easier to read if it is normally a light color (like white)*/ - // color: @textOnLightBackground; -} - -.bloom-ui-current-audio-marker:before { - background-image: url(currentTextIndicator.svg); - background-repeat: no-repeat; - background-size: 10px 13px; - background-position: 2px 5px; - left: -15px; - top: 0; // should have no effect, but prevents a FF bug causing BL-6796 +.bloom-ui-current-audio-marker { + // Use the same highlight background color as the current sentence. + background-color: var( + --bloom-audio-current-highlight-background, + @highlightColor + ); + // Mask to the microphone shape so the background-color shows through it. + -webkit-mask-image: url(currentTextIndicator.svg); + -webkit-mask-size: 10px 13px; + -webkit-mask-position: 2px 5px; + -webkit-mask-repeat: no-repeat; + mask-image: url(currentTextIndicator.svg); + mask-size: 10px 13px; + mask-position: 2px 5px; + mask-repeat: no-repeat; width: 15px; height: 19px; - position: absolute; - content: " "; + // Must be above the split-pane-component and other content in the page frame. + z-index: 1; } -// These classes get applied to ui-audioCurrent elements when we don't currently want -// them highlighted. Because a lot of rules now set various foreground and background -// colors for .ui-audioCurrent, not only during editing when the suppress and disable -// classes are relevant, it's cleaner to do the suppression with separate rules -// rather than :not claues like the above. -// The goal is that rules that set background and foreground color for the current -// playback element shall be defeated when the disable/suppress classes are present. -// There could pathologically be a case where some other rule was defeated unintentionally, -// but we don't have any other rule-based text or background coloring that applies to -// audio elements, so I think the neatness of not having to complicate many rules -// with :not clauses that aren't relevant in most contexts is worth it. -.ui-audioCurrent.ui-suppressHighlight, -.ui-audioCurrent.ui-suppressHighlight p, -.ui-audioCurrent.ui-disableHighlight, -.ui-audioCurrent.ui-disableHighlight p { - background-color: unset !important; - color: unset !important; +// Use pseudoelements to display TBT highlighting in the Edit tab. See audioTextHighlightManager.ts +::highlight(bloom-audio-current) { + // A thick underline offset upwards (the skip-ink setting allows it to be drawn right against the text) + // does not extend upwards as far as background-color highlighting, which greatly reduces the + // extent to which it covers previous lines of text when tightly spaced. + // Color is read from a CSS variable so the Format dialog's stored audio highlight colors apply. + text-decoration-line: underline; + text-decoration-style: solid; + text-decoration-color: var( + --bloom-audio-current-highlight-background, + @highlightColor + ); + text-decoration-thickness: 1.1em; + text-underline-offset: -0.8em; + text-decoration-skip-ink: none; + color: var(--bloom-audio-current-highlight-color, black); } - -.ui-audioCurrent .ui-enableHighlight { - background-color: @highlightColor; - // If we can one day get rid of this, we can simplify code for positioning the microphone icon near the span. - position: unset; // BL-11633, works around Chromium bug +// Note: This highlighting is expected to persist across sessions, but to be hidden +// (displayed with the yellow color) while each segment is playing. +::highlight(bloom-audio-split-1) { + text-decoration-line: underline; + text-decoration-style: solid; + text-decoration-color: #bfedf3; + text-decoration-thickness: 1.1em; + text-underline-offset: -0.8em; + text-decoration-skip-ink: none; } - -span.ui-audioCurrent, -div.ui-audioCurrent p { - // The rule that sets the background color is in basepage.less. However, we only need to - // interfere with position: relative when editing, so I _think_ this rule belongs here. - position: unset; // BL-11633, works around Chromium bug +::highlight(bloom-audio-split-2) { + text-decoration-line: underline; + text-decoration-style: solid; + text-decoration-color: #7fdae6; + text-decoration-thickness: 1.1em; + text-underline-offset: -0.8em; + text-decoration-skip-ink: none; } - -.ui-audioCurrent.bloom-postAudioSplit[data-audiorecordingmode="TextBox"]:not( - .ui-suppressHighlight - ):not(.ui-disableHighlight) { - // Special highlighting after the Split button completes to show it completed. - // Note: This highlighting is expected to persist across sessions, but to be hidden (displayed with the yellow color) while each segment is playing. - // This is accomplished because this rule temporarily drops out of effect when .ui-audioCurrent is moved to the span as that segment plays. - // (The rule requires a span BELOW the .ui-audioCurrent, so it drops out of effect the span IS the .ui-audioCurrent). - span:nth-child(3n + 1 of .bloom-highlightSegment) { - background-color: #bfedf3; - } - - span:nth-child(3n + 2 of .bloom-highlightSegment) { - background-color: #7fdae6; - } - - span:nth-child(3n + 3 of .bloom-highlightSegment) { - background-color: #29c2d6; - } - - span { - position: unset; // BL-11633, works around Chromium bug - } - - p { - // Override the normal yellow highlight so it doesn't clash with the ones we just added - background: none; - } +::highlight(bloom-audio-split-3) { + text-decoration-line: underline; + text-decoration-style: solid; + text-decoration-color: #29c2d6; + text-decoration-thickness: 1.1em; + text-underline-offset: -0.8em; + text-decoration-skip-ink: none; } .ui-audioBody { diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts index 3f13b9240437..87bf26108284 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecording.ts @@ -69,6 +69,7 @@ import { } from "../../../react_components/featureStatus"; import { animateStyleName } from "../../../utils/shared"; import jQuery from "jquery"; +import { AudioTextHighlightManager } from "./audioTextHighlightManager"; enum Status { Disabled, // Can't use button now (e.g., Play when there is no recording) @@ -107,14 +108,8 @@ const kEnableHighlightClass = "ui-enableHighlight"; // For example, some elements have highlighting prevented at this level // because its content has been broken into child elements, only some of which show the highlight const kDisableHighlightClass = "ui-disableHighlight"; -// Indicates that highlighting is briefly/temporarily suppressed, -// but may become highlighted later. -// For example, audio highlighting is suppressed until the related audio starts playing (to avoid flashes) -const kSuppressHighlightClass = "ui-suppressHighlight"; const kAudioSentence = "audio-sentence"; // Even though these can now encompass more than strict sentences, we continue to use this class name for backwards compatability reasons const kAudioSentenceClassSelector = "." + kAudioSentence; -export const kAudioCurrent = "ui-audioCurrent"; -const kAudioCurrentClassSelector = "." + kAudioCurrent; const kBloomEditableTextBoxClass = "bloom-editable"; const kBloomEditableTextBoxSelector = "div.bloom-editable"; const kBloomTranslationGroupClass = "bloom-translationGroup"; @@ -183,6 +178,9 @@ export default class AudioRecording implements IAudioRecorder { private audioSplitButton: HTMLButtonElement; + // Tracks which element currently has the audio highlight (replaces DOM-based kAudioCurrent class lookup). + private highlightedElement: HTMLElement | null = null; + private showingImageDescriptions: boolean; public recordingMode: RecordingMode; private previousRecordMode: RecordingMode; @@ -207,6 +205,7 @@ export default class AudioRecording implements IAudioRecorder { private playbackOrderCache: IPlaybackOrderInfo[] = []; private disablingOverlay: HTMLDivElement; + private audioTextHighlightManager = new AudioTextHighlightManager(); constructor(maySetHighlight: boolean = true) { this.audioSplitButton = ( @@ -258,7 +257,9 @@ export default class AudioRecording implements IAudioRecorder { .click(async (e) => { const mediaPlayer = this.getMediaPlayer(); mediaPlayer.pause(); + if (!this.highlightedElement) return; getWorkspaceBundleExports().showAdjustTimingsDialogFromWorkspaceRoot( + this.highlightedElement, this.split, this.editTimingsFileAsync, this.applyTimingsFileAsync, @@ -514,9 +515,6 @@ export default class AudioRecording implements IAudioRecorder { public removeRecordingSetup() { this.removeAudioCurrentFromPageDocBody(); const page = this.getPageDocBodyJQuery(); - page.find(kAudioCurrentClassSelector) - .removeClass(kAudioCurrent) - .removeClass(kSuppressHighlightClass); if (this.inShowPlaybackOrderMode) { // We are removing the UI because we're changing tools or pages, but we want to leave // the checkbox checked for the next time this tool is active, so it will turn on the @@ -824,10 +822,8 @@ export default class AudioRecording implements IAudioRecorder { // Enhance: Maybe this would be safer to advance/rewind to the next SPAN instead of next audio-sentence. const incrementAmount = isTraverseInReverseOn ? -1 : 1; - const current = (( - this.getPageDocBody() - )).getElementsByClassName(kAudioCurrent); - if (!current || current.length === 0) { + const currentItem = this.highlightedElement; + if (!currentItem) { return null; } @@ -836,8 +832,7 @@ export default class AudioRecording implements IAudioRecorder { if (audioElts.length === 0) { return null; } - const nextIndex = - audioElts.indexOf(current.item(0)) + incrementAmount; + const nextIndex = audioElts.indexOf(currentItem) + incrementAmount; if (nextIndex < 0 || nextIndex >= audioElts.length) { return null; } @@ -898,31 +893,21 @@ export default class AudioRecording implements IAudioRecorder { if (pageDocBody) { this.removeAudioCurrent(pageDocBody); } + + this.audioTextHighlightManager.clearAllManagedHighlights( + pageDocBody ?? undefined, + ); } private removeAudioCurrent(parentElement: Element) { - // Note that HTMLCollectionOf's length can change if you change the number of elements matching the selector. - const audioCurrentCollection: HTMLCollectionOf = - parentElement.getElementsByClassName(kAudioCurrent); - - // Convert to an array whose length won't be changed - const audioCurrentArray: Element[] = Array.from(audioCurrentCollection); - - for (let i = 0; i < audioCurrentArray.length; i++) { - audioCurrentArray[i].classList.remove( - kAudioCurrent, - kSuppressHighlightClass, - ); + if ( + this.highlightedElement && + parentElement.contains(this.highlightedElement) + ) { + this.highlightedElement = null; } - const iconHolders = Array.from( - parentElement.getElementsByClassName( - "bloom-ui-current-audio-marker", - ), - ); - for (let i = 0; i < iconHolders.length; i++) { - iconHolders[i].remove(); - } + this.updateIconMarker(null); } // I'm not sure why activeToolId falsy should count as "true" but that's how some old code @@ -1002,7 +987,9 @@ export default class AudioRecording implements IAudioRecorder { } if (oldElement === newElement && !forceRedisplay) { - // No need to do much, and better not to so we can avoid any temporary flashes as the highlight is removed and re-applied + // The current element is unchanged, so avoid tearing down and rebuilding anything in the DOM. + // We still need to refresh the custom-highlight pseudoelement state so yellow/split highlights stay in sync without a flash. + this.refreshAudioTextHighlights(newElement); return; } @@ -1013,7 +1000,7 @@ export default class AudioRecording implements IAudioRecorder { // It's good for this to happen before awaiting the subsequent async behavior, // especially if the caller doesn't await this function. // This allows us to generally represent the correct current element immediately. - newElement.classList.add(kAudioCurrent); + this.highlightedElement = newElement as HTMLElement; if (!this.listening) { // We don't need to mess with the canvas element focus while listening, and especially, // if we're doing a motion preview we don't want to see the edit controls there. @@ -1021,60 +1008,167 @@ export default class AudioRecording implements IAudioRecorder { newElement as HTMLElement, ); } - // during animation we don't want to add this, even in stuff that's not visible. - // It can get left behind and get wrapped in an extra paragraph (BL-15293) - let bloomPageHidden = false; - const page = newElement.closest(".bloom-page"); - if (page && window.getComputedStyle(page).visibility === "hidden") - bloomPageHidden = true; - if (visible && !inAnimation && !bloomPageHidden) { - // Show a record icon - // This is a workaround for a Chromium bug; see BL-11633. We'd like our style rules - // to just put the icon on the element that has kAudioCurrent. But that element - // has a background color, so (due to the bug) it cannot have position:relative, - // or we lose the cursor. So insert an empty element (which by default will have - // position: relative) to hold the icon. - const iconHolder = - newElement.ownerDocument.createElement("span"); - iconHolder.classList.add("bloom-ui-current-audio-marker"); - iconHolder.classList.add("bloom-ui"); // makes sure it never becomes part of saved document. - // If we're recording by text-box, we want the icon to be at the beginning of the text box, - // but we also want it inside the text-box div. Otherwise, the appearance system introduced - // by Bloom 6.0 will cause a gap to appear between the invisible icon and the text, shifting - // the text down while it is being recorded. See BL-13128. - // (The icon doesn't actually display for whole text box recording or for the first sentence - // of sentence-by-sentence recording, but that's a separate issue that makes the text shift - // even more mysterious.) - if (newElement.tagName === "DIV") { - newElement.insertBefore(iconHolder, newElement.firstChild); - } else { - newElement.parentElement?.insertBefore( - iconHolder, - newElement, - ); - } - } } + let suppressHighlight = false; if (suppressHighlightIfNoAudio && visible) { - // prevents highlight showing at once - // FYI: Because of how JS works, no rendering should happen between setting audioCurrent above and setting ui-suppressHighlight here. - newElement.classList.add(kSuppressHighlightClass); try { const response: AxiosResponse = await axios.get( "/bloom/api/audio/checkForSegment?id=" + newElement.id, ); - - if (response.data === "exists") { - newElement.classList.remove(kSuppressHighlightClass); - } + suppressHighlight = response.data !== "exists"; } catch (error) { - //server couldn't find it, so just leave it unhighlighted + //server couldn't find it, so leave it unhighlighted toastr.error( - "Error checking on audio file " + error.statusText, + "Error checking on audio file " + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (error as any)?.response?.statusText, ); + suppressHighlight = true; } } + + this.refreshAudioTextHighlights(newElement, suppressHighlight); + } + + private refreshAudioTextHighlights( + currentHighlight?: Element | null, + suppressCurrentHighlight?: boolean, + ) { + const activeHighlight = currentHighlight ?? this.getCurrentHighlight(); + const currentTextBox = activeHighlight + ? ((this.getTextBoxOfElement( + activeHighlight, + ) as HTMLElement | null) ?? null) + : null; + // The manager keeps both the yellow current highlight and the blue post-split + // highlights in sync so callers do not need separate refresh paths. + this.audioTextHighlightManager.refreshHighlights( + activeHighlight, + currentTextBox, + suppressCurrentHighlight, + ); + this.updateIconMarker( + suppressCurrentHighlight + ? null + : (activeHighlight as HTMLElement | null), + ); + } + + // The ID used for the single persistent microphone icon marker element. + private readonly kIconMarkerId = "bloom-ui-current-audio-icon"; + + // Update the position of the absolutely-placed microphone icon marker, or hide it. + // The marker lives inside #page-scaling-container (a sibling of .bloom-page) so it + // is inside the page zoom transform but outside the page content, where it cannot be + // accidentally saved or disturb CKEditor. getBoundingClientRects() gives positions + // in viewport (post-zoom) coordinates; we divide by the container's CSS scale to + // convert to the container's local coordinate space for position: absolute. + private updateIconMarker(element: HTMLElement | null): void { + const pageDocBody = this.getPageDocBody(); + if (!pageDocBody) return; + + const hideExisting = () => { + const existing = pageDocBody.ownerDocument.getElementById( + this.kIconMarkerId, + ) as HTMLElement | null; + if (existing) existing.style.display = "none"; + }; + + if (!element || this.inShowPlaybackOrderMode) { + hideExisting(); + return; + } + + // Don't show during motion-tool animation or on hidden pages. + if (element.closest("." + animateStyleName)) { + hideExisting(); + return; + } + const bloomPage = element.closest(".bloom-page") as HTMLElement | null; + const docView = element.ownerDocument.defaultView; + if ( + bloomPage && + docView?.getComputedStyle(bloomPage).visibility === "hidden" + ) { + hideExisting(); + return; + } + + // Don't show for hidden language blocks or other invisible elements. + if (!this.isVisible(element)) { + hideExisting(); + return; + } + + const container = pageDocBody.querySelector( + "#page-scaling-container", + ) as HTMLElement | null; + if (!container) return; + + // getClientRects() returns one rect per line box; the first is the first line. + const rects = element.getClientRects(); + if (rects.length === 0) { + hideExisting(); + return; + } + + const firstLineRect = rects[0]; + const containerRect = container.getBoundingClientRect(); + + // #page-scaling-container uses transform: scale(zoom) so viewport distances + // must be divided by the zoom factor to get local coordinates. + const transformStr = + docView?.getComputedStyle(container).transform ?? "none"; + const zoom = + transformStr !== "none" ? new DOMMatrix(transformStr).a : 1; + + // getComputedStyle().fontSize is in CSS pixels (pre-zoom), same coordinate space + // as the top/left values we set, so we can add the em-based offset directly. + const fontSizePx = parseFloat( + docView?.getComputedStyle(element).fontSize ?? "16", + ); + + // All conditions met — get or create the icon only now that we'll show it. + const icon = this.getOrCreateIconMarker(pageDocBody); + if (!icon) return; + + icon.style.display = ""; + // Shift down ~0.2em so the icon tracks the text rather than the top of the line box. + icon.style.top = `${(firstLineRect.top - containerRect.top) / zoom + fontSizePx * 0.2}px`; + // Place the icon 15px to the left of the sentence start, matching the + // background-position offset in audioRecording.less. + icon.style.left = `${(firstLineRect.left - containerRect.left) / zoom - 15}px`; + } + + // Find the icon marker element, or create and insert it inside #page-scaling-container. + private getOrCreateIconMarker( + pageDocBody: HTMLElement, + ): HTMLElement | null { + const container = pageDocBody.querySelector( + "#page-scaling-container", + ) as HTMLElement | null; + if (!container) return null; + + const existing = pageDocBody.ownerDocument.getElementById( + this.kIconMarkerId, + ) as HTMLElement | null; + if (existing) return existing; + + const icon = pageDocBody.ownerDocument.createElement("span"); + icon.id = this.kIconMarkerId; + icon.className = "bloom-ui-current-audio-marker bloom-ui"; + icon.style.position = "absolute"; + icon.style.pointerEvents = "none"; + // Ensure #page-scaling-container is a positioning context for our absolute child. + const containerPosition = + container.ownerDocument.defaultView?.getComputedStyle(container) + .position ?? "static"; + if (containerPosition === "static") { + container.style.position = "relative"; + } + container.appendChild(icon); + return icon; } // Scrolls an element into view. @@ -1172,6 +1266,11 @@ export default class AudioRecording implements IAudioRecorder { } this.resetAudioIfPaused(); + // Clear any split-complete state before rebuilding the current highlight. + // Otherwise the custom-highlight logic will still treat the textbox as "post split" + // and suppress the yellow current highlight we want to show as Speak starts. + this.clearAudioSplit(); + // If we were paused highlighting one sentence but are recording in text box mode, // things could get confusing. At least make sure the selection reflects what we // actually want to record. @@ -1187,8 +1286,6 @@ export default class AudioRecording implements IAudioRecorder { const id = this.getCurrentAudioId(); - this.clearAudioSplit(); - return axios .post("/bloom/api/audio/startRecord?id=" + id) .then(() => { @@ -1222,13 +1319,7 @@ export default class AudioRecording implements IAudioRecorder { private getCurrentAudioId(): string | undefined { let id: string | undefined = undefined; - const pageDocBody = this.getPageDocBody(); - const audioCurrentElements = - pageDocBody!.getElementsByClassName(kAudioCurrent); - let currentElement: Element | null = null; - if (audioCurrentElements.length > 0) { - currentElement = audioCurrentElements.item(0); - } + const currentElement = this.highlightedElement; if (currentElement) { if (currentElement.hasAttribute("id")) { id = currentElement.getAttribute("id")!; @@ -2059,7 +2150,7 @@ export default class AudioRecording implements IAudioRecorder { // If the highlight was on something not currently visible, move the selection const current = this.getCurrentHighlight(); if (page && current && !this.isVisible(current)) { - this.removeAudioCurrent(page); + this.removeAudioCurrentFromPageDocBody(); await this.setCurrentAudioElementToDefaultAsync(); } // Whether or not we had to move the selection, some button states may need to change. @@ -2069,7 +2160,7 @@ export default class AudioRecording implements IAudioRecorder { this.updateDisplay(); } private showPlaybackOrderUi(docBody: HTMLElement) { - this.removeAudioCurrent(docBody); + this.removeAudioCurrentFromPageDocBody(); this.playbackOrderCache = []; const translationGroups = this.getVisibleTranslationGroups(docBody); if (translationGroups.length < 1) { @@ -2389,20 +2480,13 @@ export default class AudioRecording implements IAudioRecorder { // Returns the element (could be either div, span, etc.) which is currently highlighted. public getCurrentHighlight(): HTMLElement | null { - let page = this.getPageDocBodyJQuery(); - // ENHANCE: I don't think this really needs to be here? - if (page.length <= 0) { + if (!this.getPageDocBodyJQuery().length) { // The first one is probably the right one when this case is triggered, but even if not, it's better than nothing. this.setCurrentAudioElementToDefaultAsync(); - page = this.getPageDocBodyJQuery(); } - const current = page.find(kAudioCurrentClassSelector); - if (current && current.length > 0) { - return current.get(0); - } - return null; + return this.highlightedElement; } // Returns the text of the currently highlighted element @@ -2467,14 +2551,13 @@ export default class AudioRecording implements IAudioRecorder { return null; } - let audioCurrentElements = ( - Array.from( - pageBody.getElementsByClassName(kAudioCurrent), - ) as HTMLElement[] - ).filter((x) => this.isVisible(x)); + let audioCurrentElements = + this.highlightedElement && this.isVisible(this.highlightedElement) + ? [this.highlightedElement] + : []; if (audioCurrentElements.length === 0 && maySetHighlight) { - // Oops, ui-audioCurrent not set on anything. Just going to have to stick it onto the first element. + // Oops, highlightedElement not set or not visible. Just going to have to stick it onto the first element. // ENHANCE: Theoretically, we should await this. (Or at least, the end of the function should await this promise // That means all the callers should be async'ify'd, which is like... everything. :( @@ -2486,9 +2569,9 @@ export default class AudioRecording implements IAudioRecorder { // 1) This original version (that includes the asynchronous fallback) // 2) Also a synchronous (but no fallback) version of this function called getCurrentTextBoxSync() this.setCurrentAudioElementToDefaultAsync(); - audioCurrentElements = Array.from( - pageBody.getElementsByClassName(kAudioCurrent), - ) as HTMLElement[]; + audioCurrentElements = this.highlightedElement + ? [this.highlightedElement] + : []; if (audioCurrentElements.length <= 0) { return null; @@ -2503,19 +2586,7 @@ export default class AudioRecording implements IAudioRecorder { } public getAudioCurrentElement(): HTMLElement | null { - const pageBody = this.getPageDocBody(); - if (!pageBody) { - return null; - } - - const audioCurrentElements = - pageBody.getElementsByClassName(kAudioCurrent); - - if (audioCurrentElements.length === 0) { - return null; - } - - return audioCurrentElements.item(0) as HTMLElement; + return this.highlightedElement; } // Gets the current text box. If none exists, immediately returns null. @@ -2523,26 +2594,21 @@ export default class AudioRecording implements IAudioRecorder { // TODO: Refactor the old getCurrentTextBox to something like: getCurrentTextBoxWithFallbackAsync // After that, you can rename this function to getCurrentTextBox - const pageBody = this.getPageDocBody(); if ( - !pageBody || + !this.getPageDocBody() || // Tests may not have a value for 'showPlaybackInput'. this.inShowPlaybackOrderMode ) { return null; } - const audioCurrentElements = - pageBody.getElementsByClassName(kAudioCurrent); - - if (audioCurrentElements.length === 0) { - // Oops, ui-audioCurrent not set on anything. Just give up. + const currentItem = this.highlightedElement; + if (!currentItem) { + // Oops, highlightedElement not set. Just give up. return null; } - const currentTextBox = this.getTextBoxOfElement( - audioCurrentElements.item(0), - ); + const currentTextBox = this.getTextBoxOfElement(currentItem); console.assert(!!currentTextBox, "CurrentTextBox should not be null"); return currentTextBox; } @@ -2603,6 +2669,12 @@ export default class AudioRecording implements IAudioRecorder { this.initializeAudioRecordingMode(); const docBody = this.getPageDocBody(); + // Defensive cleanup: strip any ui-audioCurrent class left by older Bloom versions that used DOM marking. + // Modern code tracks the highlight via this.highlightedElement instead. + Array.from( + docBody?.getElementsByClassName("ui-audioCurrent") ?? [], + ).forEach((el) => el.classList.remove("ui-audioCurrent")); + // This check needs to be before the check for recordable divs below (which may return immediately), because sometimes // we may have empty textboxes that should nevertheless show the playback order UI. if (this.inShowPlaybackOrderMode) { @@ -2680,7 +2752,7 @@ export default class AudioRecording implements IAudioRecorder { const oldHighlight = this.getCurrentHighlight(); if (!boxToSelect) { this.resetAudioIfPaused(); - this.removeAudioCurrent(this.getPageDocBody()!); + this.removeAudioCurrentFromPageDocBody(); await this.changeStateAndSetExpectedAsync(""); this.updateDisplay(false); return false; @@ -2876,6 +2948,23 @@ export default class AudioRecording implements IAudioRecorder { await this.tryGetUpdateMarkupForTextBoxActionAsync(currentTextBox); return async () => { + // Save the index (position) of the highlighted sentence among its siblings + // before markup changes the DOM. IDs are regenerated when text changes, so + // we use ordinal position instead — it survives ordinary edits. + let previousHighlightIndex = -1; + if ( + this.highlightedElement && + this.highlightedElement.isConnected && + currentTextBox + ) { + const sentences = Array.from( + currentTextBox.getElementsByClassName(kAudioSentence), + ); + previousHighlightIndex = sentences.indexOf( + this.highlightedElement as Element, + ); + } + updateTheElement(); // Adjust the current highlight appropriately // Regardless of whether it's present, we always need to set the current audio element @@ -2888,7 +2977,42 @@ export default class AudioRecording implements IAudioRecorder { // the async actions complete. await this.resetCurrentAudioElementAsync(currentTextBox); + // cleanUpNbsps() in toolbox.ts runs synchronously while we are suspended at the + // first await above. It unconditionally sets editableDiv.innerHTML, detaching + // the span that resetCurrentAudioElementAsync just registered as highlightedElement. + // IDs are preserved through that replacement, so we can recover the live DOM node. + if ( + this.highlightedElement && + !this.highlightedElement.isConnected && + this.highlightedElement.id + ) { + const pageBody = this.getPageDocBody(); + const freshHighlight = pageBody + ? (pageBody.querySelector( + `#${this.highlightedElement.id}`, + ) as HTMLElement | null) + : null; + if (freshHighlight) { + this.highlightedElement = freshHighlight; + } + } + + // resetCurrentAudioElementAsync always resets to the first sentence, but the + // user was editing a specific sentence. Restore by ordinal index; IDs are + // regenerated when text changes so they cannot be used to find the same sentence. + if (previousHighlightIndex >= 0 && currentTextBox) { + const sentences = + currentTextBox.getElementsByClassName(kAudioSentence); + const targetSentence = sentences.item( + previousHighlightIndex, + ) as HTMLElement | null; + if (targetSentence) { + this.highlightedElement = targetSentence; + } + } + await this.changeStateAndSetExpectedAsync("record"); + this.refreshAudioTextHighlights(this.highlightedElement); }; } @@ -3060,30 +3184,23 @@ export default class AudioRecording implements IAudioRecorder { } console.assert(this.recordingMode == RecordingMode.TextBox); - const pageDocBody = this.getPageDocBody(); - if (!pageDocBody) { + if (!this.getPageDocBody()) { return; } - const audioCurrentList = - pageDocBody.getElementsByClassName(kAudioCurrent); - if (isEarlyAbortEnabled && audioCurrentList.length >= 1) { + if (isEarlyAbortEnabled && this.highlightedElement !== null) { // audioCurrent highlight is already working, so don't bother trying to fix anything up. // I think this probably can also help if you rapidly check and uncheck the checkbox, then click Next. // We wouldn't want multiple things highlighted, or end up pointing to the wrong thing, etc. return; } - let audioCurrent: Element | null = null; - if (audioCurrentList.length >= 1) { - audioCurrent = audioCurrentList.item(0); - } const changeTo = this.getTextBoxOfElement(element); if (changeTo) { return this.setSoundAndHighlightAsync({ newElement: changeTo, // Don't automatically scroll because tool is possibly being initialized (we only want it to scroll on explicit user interaction like Next/Prev) shouldScrollToElement: false, - oldElement: audioCurrent, + oldElement: this.highlightedElement, }); } } @@ -3097,11 +3214,7 @@ export default class AudioRecording implements IAudioRecorder { return; } - const audioCurrentList = this.getPageDocBodyJQuery().find( - kAudioCurrentClassSelector, - ); - - if (isEarlyAbortEnabled && audioCurrentList.length >= 1) { + if (isEarlyAbortEnabled && this.highlightedElement !== null) { // audioCurrent highlight is already working, so don't bother trying to fix anything up. // I think this probably can also help if you rapidly check and uncheck the checkbox, then click Next. // We wouldn't want multiple things highlighted, or end up pointing to the wrong thing, etc. @@ -3733,8 +3846,7 @@ export default class AudioRecording implements IAudioRecorder { // Finding no audioCurrent is only unexpected if there are non-zero number of audio elements if ( - this.getPageDocBodyJQuery().find(kAudioCurrentClassSelector) - .length === 0 && + this.highlightedElement === null && this.containsAnyAudioElements() ) { // We have reached an unexpected state :( @@ -4412,6 +4524,7 @@ export default class AudioRecording implements IAudioRecorder { const currentTextBox = this.getCurrentTextBox(); if (currentTextBox) { currentTextBox.classList.add("bloom-postAudioSplit"); + this.refreshAudioTextHighlights(currentTextBox); } } @@ -4421,6 +4534,10 @@ export default class AudioRecording implements IAudioRecorder { currentTextBox.classList.remove("bloom-postAudioSplit"); currentTextBox.removeAttribute("data-audioRecordingEndTimes"); } + + this.audioTextHighlightManager.clearSplitHighlights( + currentTextBox ?? undefined, + ); } private getElementsToUpdateForCursor(): (Element | null)[] { @@ -4860,6 +4977,7 @@ export default class AudioRecording implements IAudioRecorder { } }); this.nodesToRestoreAfterPlayEnded.clear(); + this.refreshAudioTextHighlights(); } } diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts index 6e7dbe83a6ac..b8c2ae34bcae 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioRecordingSpec.ts @@ -1,4 +1,4 @@ -import { +import { describe, it, expect, @@ -19,6 +19,98 @@ import { RecordingMode } from "./recordingMode"; import axios from "axios"; import $ from "jquery"; import { mockReplies } from "../../../utils/bloomApi"; +import { + currentHighlightName, + splitHighlightNames, +} from "./audioTextHighlightManager"; + +class FakeHighlight { + public ranges: Range[]; + + public constructor(...ranges: Range[]) { + this.ranges = ranges; + } +} + +type FakeHighlightRegistry = Map; +type TestCssWithHighlights = { + highlights?: FakeHighlightRegistry; +}; + +const installPseudoHighlightPolyfill = (targetWindow: Window) => { + const targetWindowWithCss = targetWindow as Window & { + CSS?: TestCssWithHighlights; + }; + if (!targetWindowWithCss.CSS) { + targetWindowWithCss.CSS = {}; + } + + const cssWithHighlights = targetWindowWithCss.CSS; + cssWithHighlights.highlights = new Map(); + ( + targetWindow as Window & { + Highlight?: typeof FakeHighlight; + } + ).Highlight = FakeHighlight; +}; + +const getPageWindow = (): Window | undefined => { + const iframe = parent.window.document.getElementById( + "page", + ) as HTMLIFrameElement | null; + return iframe?.contentWindow ?? undefined; +}; + +const getPseudoHighlightsRegistry = (): FakeHighlightRegistry => { + const targetWindow = getPageWindow() ?? globalThis.window; + const cssWithHighlights = ( + targetWindow as Window & { + CSS?: TestCssWithHighlights; + } + ).CSS; + if (!cssWithHighlights?.highlights) { + throw new Error( + "Expected CSS.highlights test polyfill to be installed", + ); + } + + return cssWithHighlights.highlights; +}; + +const getSplitHighlightTexts = (): string[][] => { + const registry = getPseudoHighlightsRegistry(); + return splitHighlightNames.map((name) => { + const highlight = registry.get(name); + return highlight + ? highlight.ranges.map((range) => range.toString()) + : []; + }); +}; + +const getHighlightTexts = (highlightName: string): string[] => { + const highlight = getPseudoHighlightsRegistry().get(highlightName); + return highlight ? highlight.ranges.map((range) => range.toString()) : []; +}; + +/** + * In BL-15300, the "current" audio element is tracked by AudioRecording.highlightedElement. + * Tests mark the intended pre-selected element with data-test-preselect="true" in their + * HTML fixture (never via the obsolete ui-audioCurrent class). This helper reads that + * attribute and transfers the element to the recording's highlightedElement field before + * calling methods under test. + */ +const setHighlightedElementFromDom = (recording: AudioRecording) => { + const pageFrame = parent.window.document.getElementById( + "page", + ) as HTMLIFrameElement | null; + const doc = pageFrame?.contentDocument ?? document; + const elem = doc.querySelector( + "[data-test-preselect]", + ) as HTMLElement | null; + ( + recording as unknown as { highlightedElement: HTMLElement | null } + ).highlightedElement = elem; +}; // Notes: // For any async tests: @@ -38,13 +130,21 @@ import { mockReplies } from "../../../utils/bloomApi"; describe("audio recording tests", () => { beforeAll(async () => { + installPseudoHighlightPolyfill(globalThis.window); + await setupForAudioRecordingTests(); + + const pageWindow = getPageWindow(); + if (pageWindow) { + installPseudoHighlightPolyfill(pageWindow); + } }); afterEach(() => { // Clean up any pending timers to prevent "parent is not defined" errors // when tests finish before timers fire theOneAudioRecorder?.clearTimeouts(); + getPseudoHighlightsRegistry().clear(); }); // In an earlier version of our API, checkForAnyRecording was designed to fail (404) if there was no recording. @@ -68,16 +168,16 @@ describe("audio recording tests", () => { // Returns the HTML for a single text box for a variety of recording modes function getTextBoxHtmlSimple1(scenario: AudioMode) { if (scenario === AudioMode.PureSentence) { - return `

Sentence 1.1. Sentence 1.2

`; + return `

Sentence 1.1. Sentence 1.2

`; } else if (scenario === AudioMode.PreTextBox) { - return `

Sentence 1.1. Sentence 1.2

`; + return `

Sentence 1.1. Sentence 1.2

`; } else if (scenario === AudioMode.PureTextBox) { - return `

Sentence 1.1. Sentence 1.2

`; + return `

Sentence 1.1. Sentence 1.2

`; } else if (scenario === AudioMode.HardSplitTextBox) { - // FYI: Yes, it is confirmed that in hardSplit, ui-audioCurrent goes on the div, not the span. - return `

Sentence 1.1. Sentence 1.2

`; + // FYI: Yes, it is confirmed that in hardSplit, the pre-selected element is the div, not the span. + return `

Sentence 1.1. Sentence 1.2

`; } else if (scenario === AudioMode.SoftSplitTextBox) { - return `

Sentence 1.1. Sentence 1.2

`; + return `

Sentence 1.1. Sentence 1.2

`; } else { throw new Error("Unknown scenario: " + AudioMode[scenario]); } @@ -86,9 +186,10 @@ describe("audio recording tests", () => { describe("- Next()", () => { it("Record=Sentence, last sentence returns disabled for Next button", () => { SetupIFrameFromHtml( - "

Sentence 1.

", + "

Sentence 1.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.Sentence; const observed = recording.getNextAudioElement(); @@ -98,9 +199,10 @@ describe("audio recording tests", () => { it("Record=TextBox/Play=Sentence, last sentence returns disabled for Next button", () => { SetupIFrameFromHtml( - "

Sentence 1.

", + "

Sentence 1.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getNextAudioElement(); @@ -110,9 +212,10 @@ describe("audio recording tests", () => { it("Record=TextBox/Play=TextBox, last sentence returns disabled for Next button", () => { SetupIFrameFromHtml( - "
p>Sentence 1.

", + "
p>Sentence 1.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getNextAudioElement(); @@ -122,7 +225,7 @@ describe("audio recording tests", () => { it("SS -> SS, returns next box's first sentence", () => { const box1Html = - "

Sentence 1.

"; + "

Sentence 1.

"; const box2Html = "

Sentence 2.Sentence 3.

"; SetupIFrameFromHtml( @@ -130,6 +233,7 @@ describe("audio recording tests", () => { ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.Sentence; const observed = recording.getNextAudioElement(); @@ -139,7 +243,7 @@ describe("audio recording tests", () => { it("TS -> TT, returns next box", () => { const box1Html = - "

Sentence 1.

"; + "

Sentence 1.

"; const box2Html = "

Sentence 2.Sentence 3.

"; SetupIFrameFromHtml( @@ -147,6 +251,7 @@ describe("audio recording tests", () => { ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getNextAudioElement(); @@ -156,9 +261,10 @@ describe("audio recording tests", () => { it("TT -> TT, returns next box", () => { SetupIFrameFromHtml( - "
p>Sentence 1.

p>Sentence 2.

", + "
p>Sentence 1.

p>Sentence 2.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getNextAudioElement(); @@ -167,10 +273,14 @@ describe("audio recording tests", () => { }); it("Next() skips over empty box, TT -> TT -> TT", () => { - const boxTemplate = (index: number, extraClasses: string) => { - return `
p>Sentence ${index}.

`; + const boxTemplate = ( + index: number, + extraClasses: string, + preselect = false, + ) => { + return `
p>Sentence ${index}.

`; }; - const box1Html = boxTemplate(1, " ui-audioCurrent"); + const box1Html = boxTemplate(1, "", true); const box2Html = '

'; const box3Html = boxTemplate(3, ""); @@ -180,6 +290,7 @@ describe("audio recording tests", () => { ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getNextAudioElement(); @@ -192,9 +303,10 @@ describe("audio recording tests", () => { describe("- Prev()", () => { it("Record=Sentence, first sentence returns disabled for Back button", () => { SetupIFrameFromHtml( - "

Sentence 1.

", + "

Sentence 1.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.Sentence; const observed = recording.getPreviousAudioElement(); @@ -204,9 +316,10 @@ describe("audio recording tests", () => { it("Record=TextBox/Play=Sentence, first sentence returns disabled for Back button", () => { SetupIFrameFromHtml( - "

Sentence 1.

", + "

Sentence 1.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getPreviousAudioElement(); @@ -216,9 +329,10 @@ describe("audio recording tests", () => { it("Record=TextBox/Play=TextBox, first sentence returns disabled for Back button", () => { SetupIFrameFromHtml( - "
p>Sentence 1.

", + "
p>Sentence 1.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getPreviousAudioElement(); @@ -230,12 +344,13 @@ describe("audio recording tests", () => { const box1Html = "

Sentence 1.Sentence 2.

"; const box2Html = - "

Sentence 3.

"; + "

Sentence 3.

"; SetupIFrameFromHtml( `
${box1Html}${box2Html}
`, ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.Sentence; const observed = recording.getPreviousAudioElement(); @@ -247,12 +362,13 @@ describe("audio recording tests", () => { const box1Html = "

Sentence 1.Sentence 2.

"; const box2Html = - "

Sentence 3.

"; + "

Sentence 3.

"; SetupIFrameFromHtml( `
${box1Html}${box2Html}
`, ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getPreviousAudioElement(); @@ -262,9 +378,10 @@ describe("audio recording tests", () => { it("TT <- TT, returns previous box", () => { SetupIFrameFromHtml( - "
p>Sentence 1.

p>Sentence 2.

", + "
p>Sentence 1.

p>Sentence 2.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getPreviousAudioElement(); @@ -273,19 +390,24 @@ describe("audio recording tests", () => { }); it("Prev() skips over empty box, TT <- TT <- TT", () => { - const boxTemplate = (index: number, extraClasses: string) => { - return `
p>Sentence ${index}.

`; + const boxTemplate = ( + index: number, + extraClasses: string, + preselect = false, + ) => { + return `
p>Sentence ${index}.

`; }; const box1Html = boxTemplate(1, ""); const box2Html = '

'; - const box3Html = boxTemplate(3, " ui-audioCurrent"); + const box3Html = boxTemplate(3, "", true); SetupIFrameFromHtml( `
${box1Html}${box2Html}${box3Html}
`, ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const observed = recording.getPreviousAudioElement(); @@ -298,7 +420,7 @@ describe("audio recording tests", () => { describe("- PlayingMultipleAudio()", () => { it("returns true while in listen to whole page with multiple text boxes", async () => { SetupIFrameFromHtml( - "
p>Sentence 1.

p>Sentence 2.

", + "
p>Sentence 1.

p>Sentence 2.

", ); const recording = new AudioRecording(); await recording.listenAsync(); @@ -316,7 +438,7 @@ describe("audio recording tests", () => { it("returns false while preloading", () => { SetupIFrameFromHtml( - "
p>Sentence 1.

p>Sentence 2.

", + "
p>Sentence 1.

p>Sentence 2.

", ); const recording = new AudioRecording(); @@ -861,7 +983,7 @@ describe("audio recording tests", () => { const textBoxDivHtml = '
'; const paragraphsMarkedBySentenceHtml = - '

Sentence 1. Sentence 2. Sentence 3.

Paragraph 2.

'; + '

Sentence 1. Sentence 2. Sentence 3.

Paragraph 2.

'; const formatButtonHtml = '
'; const originalHtml = @@ -942,11 +1064,7 @@ describe("audio recording tests", () => { expect( StripAllGuidIds(StripEmptyClasses(parent.html())), "Swap back to original", - ).toBe( - StripAllGuidIds( - StripEmptyClasses(StripAudioCurrent(originalHtml)), - ), - ); + ).toBe(StripAllGuidIds(StripEmptyClasses(originalHtml))); }); it("converts by-text-box into by-sentence (bloom-editable includes format button)", () => { @@ -1049,17 +1167,13 @@ describe("audio recording tests", () => { expect( StripAllGuidIds(StripEmptyClasses(parentDiv.html())), "Swap back to original", - ).toBe( - StripAllGuidIds( - StripEmptyClasses(StripAudioCurrent(originalHtml)), - ), - ); + ).toBe(StripAllGuidIds(StripEmptyClasses(originalHtml))); }); it("loads by-text-box without changing anything", () => { // This tests real input from Bloom that has been marked up in by-text-box mode (e.g., clicking the checkbox from not-by-sentence into by-sentence) const textBoxDivHtml = - '
'; + '
'; const formatButtonHtml = '
'; const originalHtml = @@ -1252,7 +1366,7 @@ describe("audio recording tests", () => { describe("startRecordCurrentAsync", () => { const setupRecordButtonHtml = () => { SetupIFrameFromHtml( - `

Sentence 1.1.

`, + `

Sentence 1.1.

`, ); }; @@ -1318,8 +1432,8 @@ describe("audio recording tests", () => { setupRecordButtonHtml(); let resolveStartRecord: () => void; - const startRecordPromise = new Promise((resolve) => { - resolveStartRecord = resolve; + const startRecordPromise = new Promise((resolve) => { + resolveStartRecord = () => resolve(); }); vi.spyOn(axios, "post").mockImplementation((url: string) => { if (url.startsWith("/bloom/api/audio/startRecord")) { @@ -1375,6 +1489,7 @@ describe("audio recording tests", () => { const runClearRecordingAsync = async (scenario) => { const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); if (scenario === AudioMode.PureSentence) { recording.recordingMode = RecordingMode.Sentence; } else { @@ -1518,17 +1633,18 @@ describe("audio recording tests", () => { // Verification const firstDiv = getFrameElementById("page", "div1")!; - expect(firstDiv).toHaveClass("ui-audioCurrent"); + expect(recording.getAudioCurrentElement()).toBe(firstDiv); }); }); describe("- initializeAudioRecordingMode()", () => { it("initializeAudioRecordingMode gets mode from current div if available (synchronous) (Text Box)", () => { SetupIFrameFromHtml( - "
Sentence 1. Sentence 2.
Paragraph 2.
", + "
Sentence 1. Sentence 2.
Paragraph 2.
", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.Sentence; // Just to make sure that the code under test can read the current div at all. @@ -1545,10 +1661,11 @@ describe("audio recording tests", () => { it("initializeAudioRecordingMode gets mode from current div if available (synchronous) (Sentence)", () => { SetupIFrameFromHtml( - "
Paragraph 1.
Paragraph 2.
", + "
Paragraph 1.
Paragraph 2.
", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.Sentence; // Just to make sure that the code under test can read the current div at all. @@ -1565,10 +1682,11 @@ describe("audio recording tests", () => { it("initializeAudioRecordingMode gets mode from other divs on page as fallback (synchronous) (TextBox)", () => { SetupIFrameFromHtml( - "
Paragraph 1
Paragraph 2.
", + "
Paragraph 1
Paragraph 2.
", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.Sentence; // Just to make sure that the code under test can read the current div at all. @@ -1587,10 +1705,11 @@ describe("audio recording tests", () => { // The 2nd div doesn't really look well-formed because we're trying to get the test to exercise some fallback cases // The first div doesn't look well-formed either but I want the test to exercise that it is getting it from the data-audiorecordingmode attribute not from any of the div's innerHTML markup. SetupIFrameFromHtml( - "
Paragraph 1
Paragraph 2.
", + "
Paragraph 1
Paragraph 2.
", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; // Just to make sure that the code under test can read the current div at all. @@ -1607,10 +1726,11 @@ describe("audio recording tests", () => { it("initializeAudioRecordingMode identifies 4.3 audio-sentences (synchronous)", () => { SetupIFrameFromHtml( - "
Sentence 1. Sentence 2.
Paragraph 2.
", + "
Sentence 1. Sentence 2.
Paragraph 2.
", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; // Just to make sure that the code under test can read the current div at all. @@ -1739,9 +1859,12 @@ describe("audio recording tests", () => { it("getCurrentText works", () => { setupDefaultApiResponses(); - SetupIFrameFromHtml("
Hello world
"); + SetupIFrameFromHtml( + "
Hello world
", + ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); expect(recording.getCurrentHighlight()).toBeTruthy(); const returnedText = recording.getCurrentText(); @@ -1751,10 +1874,11 @@ describe("audio recording tests", () => { it("getAutoSegmentLanguageCode works", () => { setupDefaultApiResponses(); SetupIFrameFromHtml( - "
Hello world
", + "
Hello world
", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); const returnedText = recording.getAutoSegmentLanguageCode(); expect(returnedText).toBe("es"); @@ -1763,10 +1887,11 @@ describe("audio recording tests", () => { it("extractFragmentsForAudioSegmentation works", () => { setupDefaultApiResponses(); SetupIFrameFromHtml( - "
Sentence 1. Sentence 2.
", + "
Sentence 1. Sentence 2.
", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); const returnedFragmentIds: AudioTextFragment[] = recording.extractFragmentsAndSetSpanIdsForAudioSegmentation(); @@ -1782,10 +1907,11 @@ describe("audio recording tests", () => { it("extractFragmentsForAudioSegmentation handles duplicate sentences separately", () => { setupDefaultApiResponses(); SetupIFrameFromHtml( - "
What color is the sky? Blue. What color is the ocean? Blue. Hello. Hello.

", + "
What color is the sky? Blue. What color is the ocean? Blue. Hello. Hello.

", ); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; const returnedFragmentIds: AudioTextFragment[] = recording.extractFragmentsAndSetSpanIdsForAudioSegmentation(); @@ -1883,10 +2009,11 @@ describe("audio recording tests", () => { it("importRecording() encodes special characters", async () => { // Setup const div1 = - '

One. Two. Three.

'; + '

One. Two. Three.

'; SetupIFrameFromHtml(div1); const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); recording.recordingMode = RecordingMode.TextBox; // Should be the old state, toggleRecordingMode() will flip the state const baseName = "A`B~C!D@E#F$G%H^I&J(K)L-M_N=O+P[Q{R]S}T;U'V,W.X"; @@ -1997,7 +2124,7 @@ describe("audio recording tests", () => { scenarios.forEach((scenario) => { it(`[${scenario}] doesn't change anything for two or fewer whitespace in split text box w/single highlight segment`, () => { const originalHtml = - '

One Two  Three

'; + '

One Two  Three

'; SetupIFrameFromHtml(originalHtml); const box1 = getFrameElementById("page", "box1")!; @@ -2012,7 +2139,7 @@ describe("audio recording tests", () => { it(`[${scenario}] disables highlight on 3 or more whitespace in split text box w/single highlight segment`, () => { SetupIFrameFromHtml( - '

One Two  Three   Four    End

', + '

One Two  Three   Four    End

', ); const box1 = getFrameElementById("page", "box1")!; @@ -2035,7 +2162,7 @@ describe("audio recording tests", () => { it(`[${scenario}] disables highlight on 3 or more whitespace in split text box w/multiple highlight segments.`, () => { const html = - '

One Two  End1.Three   End2.Four    Five     End3.

'; + '

One Two  End1.Three   End2.Four    Five     End3.

'; SetupIFrameFromHtml(html); const box1 = getFrameElementById("page", "box1")!; @@ -2056,7 +2183,7 @@ describe("audio recording tests", () => { it(`[${scenario}] doesn't do anything on unsplit text box.`, () => { const originalHtml = - '

One Two  End1. Three   End2. Four    Five     End3.

'; + '

One Two  End1. Three   End2. Four    Five     End3.

'; SetupIFrameFromHtml(`
${originalHtml}
`); const box1 = getFrameElementById("page", "box1")!; @@ -2071,7 +2198,7 @@ describe("audio recording tests", () => { it(`[${scenario}] disables highlight on 3 or more whitespace in record-by-sentence box`, () => { const originalHtml = - '

One Two  End1.Three   End2.Four    Five     End3.

'; + '

One Two  End1.Three   End2.Four    Five     End3.

'; SetupIFrameFromHtml( `
${originalHtml}
`, ); @@ -2085,7 +2212,7 @@ describe("audio recording tests", () => { // Verification expect(box1.innerHTML).toBe( "

" + - 'One Two  End1.' + + 'One Two  End1.' + 'Three   End2.' + 'Four    Five     End3.' + "

", @@ -2094,7 +2221,7 @@ describe("audio recording tests", () => { it(`[${scenario}] disables highlight on emphasized text`, () => { const originalHtml = - '

Three   End2.

'; + '

Three   End2.

'; SetupIFrameFromHtml(`
${originalHtml}
`); const box1 = getFrameElementById("page", "box1")!; @@ -2106,14 +2233,14 @@ describe("audio recording tests", () => { // Verification expect(box1.innerHTML).toBe( "

" + - 'Three   End2.' + + 'Three   End2.' + "

", ); }); it(`[${scenario}] disables highlight for ​ (\u200B)`, () => { const originalHtml = - '

Three\u200B \u200BEnd2.

'; + '

Three\u200B \u200BEnd2.

'; SetupIFrameFromHtml( `
${originalHtml}
`, ); @@ -2127,7 +2254,7 @@ describe("audio recording tests", () => { // Verification expect(box1.innerHTML).toBe( "

" + - 'Three\u200B \u200BEnd2.' + + 'Three\u200B \u200BEnd2.' + "

", ); }); @@ -2151,7 +2278,7 @@ describe("audio recording tests", () => { it(`[${scenario}] reverts fixHighlighting() in split text box.`, () => { const originalHtml = - '

One Two  End1.Three   End2.Four    Five     End3.

'; + '

One Two  End1.Three   End2.Four    Five     End3.

'; SetupIFrameFromHtml(`
${originalHtml}
`); const box1 = getFrameElementById("page", "box1")!; @@ -2166,6 +2293,218 @@ describe("audio recording tests", () => { }); }); }); + + describe("- pseudo-element split highlights", () => { + it("registers the current sentence highlight with pseudo-element highlights", async () => { + SetupIFrameFromHtml( + '

One.

', + ); + + const recording = new AudioRecording(); + (recording as unknown as { isShowing: boolean }).isShowing = true; + const setHighlightToAsync = ( + recording as unknown as { + setHighlightToAsync(args: { + newElement: Element; + shouldScrollToElement: boolean; + forceRedisplay?: boolean; + }): Promise; + } + ).setHighlightToAsync.bind(recording); + + await setHighlightToAsync({ + newElement: getFrameElementById("page", "span1")!, + shouldScrollToElement: false, + forceRedisplay: true, + }); + + expect(getHighlightTexts(currentHighlightName)).toEqual(["One."]); + }); + + it("clears pseudo-element highlights when entering show playback order mode", async () => { + SetupIFrameFromHtml( + '

One.

', + ); + + const recording = new AudioRecording(); + (recording as unknown as { isShowing: boolean }).isShowing = true; + const setHighlightToAsync = ( + recording as unknown as { + setHighlightToAsync(args: { + newElement: Element; + shouldScrollToElement: boolean; + forceRedisplay?: boolean; + }): Promise; + } + ).setHighlightToAsync.bind(recording); + + await setHighlightToAsync({ + newElement: getFrameElementById("page", "span1")!, + shouldScrollToElement: false, + forceRedisplay: true, + }); + + expect(getHighlightTexts(currentHighlightName)).toEqual(["One."]); + + await recording.setShowPlaybackOrderMode(true); + + expect(getHighlightTexts(currentHighlightName)).toEqual([]); + expect(getSplitHighlightTexts()).toEqual([[], [], []]); + }); + + it("uses only ui-enableHighlight descendants for current highlight when its own background is disabled", async () => { + SetupIFrameFromHtml( + '

One   Two

', + ); + + const recording = new AudioRecording(); + (recording as unknown as { isShowing: boolean }).isShowing = true; + const setHighlightToAsync = ( + recording as unknown as { + setHighlightToAsync(args: { + newElement: Element; + shouldScrollToElement: boolean; + forceRedisplay?: boolean; + }): Promise; + } + ).setHighlightToAsync.bind(recording); + + await setHighlightToAsync({ + newElement: getFrameElementById("page", "span1")!, + shouldScrollToElement: false, + forceRedisplay: true, + }); + + expect(getHighlightTexts(currentHighlightName)).toEqual([ + "One", + "Two", + ]); + }); + + it("sets default highlight colors when no user styles are configured", async () => { + // Tests that setHighlightToAsync writes the CSS custom properties to the document + // element. When no userModifiedStyles sheet is present the defaults (#febf00, black) + // are used. Reading custom colors from the userModifiedStyles stylesheet requires + // real CSS rule parsing that jsdom's iframe documents don't support. + SetupIFrameFromHtml( + '

One.

', + ); + + const recording = new AudioRecording(); + (recording as unknown as { isShowing: boolean }).isShowing = true; + const setHighlightToAsync = ( + recording as unknown as { + setHighlightToAsync(args: { + newElement: Element; + shouldScrollToElement: boolean; + forceRedisplay?: boolean; + }): Promise; + } + ).setHighlightToAsync.bind(recording); + + await setHighlightToAsync({ + newElement: getFrameElementById("page", "span1")!, + shouldScrollToElement: false, + forceRedisplay: true, + }); + + const pageDocument = ( + parent.window.document.getElementById( + "page", + ) as HTMLIFrameElement + ).contentDocument!; + const documentStyle = pageDocument.documentElement.style; + expect( + documentStyle.getPropertyValue( + "--bloom-audio-current-highlight-background", + ), + ).toBe("#febf00"); + expect( + documentStyle.getPropertyValue( + "--bloom-audio-current-highlight-color", + ), + ).toBe("black"); + }); + + it("registers the split highlight color groups for the current text box", () => { + SetupIFrameFromHtml( + '

One.Two.Three.Four.

', + ); + + const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); + recording.markAudioSplit(); + + expect(getSplitHighlightTexts()).toEqual([ + ["One.", "Four."], + ["Two."], + ["Three."], + ]); + }); + + it("clears split highlights while playback moves to an individual segment", async () => { + SetupIFrameFromHtml( + '

One.Two.

', + ); + + const recording = new AudioRecording(); + (recording as unknown as { isShowing: boolean }).isShowing = true; + recording.markAudioSplit(); + + const setHighlightToAsync = ( + recording as unknown as { + setHighlightToAsync(args: { + newElement: Element; + shouldScrollToElement: boolean; + }): Promise; + } + ).setHighlightToAsync.bind(recording); + + await setHighlightToAsync({ + newElement: getFrameElementById("page", "span1")!, + shouldScrollToElement: false, + }); + + expect(getSplitHighlightTexts()).toEqual([[], [], []]); + }); + + it("keeps the current highlight when Speak clears a previous split state", async () => { + SetupIFrameFromHtml( + '

One.Two.

', + ); + + vi.spyOn(axios, "post").mockResolvedValue({}); + + const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); + (recording as unknown as { isShowing: boolean }).isShowing = true; + recording.recordingMode = RecordingMode.TextBox; + document.getElementById("audio-record")?.classList.add("expected"); + + await recording.startRecordCurrentAsync(); + + expect(getHighlightTexts(currentHighlightName).join("")).toContain( + "One.", + ); + expect(getSplitHighlightTexts()).toEqual([[], [], []]); + }); + + it("uses only ui-enableHighlight descendants when a segment disables its own background", () => { + SetupIFrameFromHtml( + '

One.Two   Three

', + ); + + const recording = new AudioRecording(); + setHighlightedElementFromDom(recording); + recording.markAudioSplit(); + + expect(getSplitHighlightTexts()).toEqual([ + ["One."], + ["Two", "Three"], + [], + ]); + }); + }); }); function StripEmptyClasses(html) { @@ -2189,12 +2528,6 @@ export function StripAllGuidIds(html) { .replace(/ id=""/g, ""); } -function StripAudioCurrent(html) { - return html - .replace(/ ui-audioCurrent/g, "") - .replace(/ class="ui-audioCurrent"/g, ""); -} - export function StripRecordingMd5(html: string): string { return html.replace(/ recordingmd5="[0-9A-Za-z]*"/g, ""); } @@ -2242,7 +2575,7 @@ async function SetupIFrameAsync(id = "page"): Promise { } // bodyContentHtml should not contain HTML or Body tags. It should be the innerHtml of the body -// It might look something like this:
Hello world
+// It might look something like this:
Hello world
export function SetupIFrameFromHtml(bodyContentHtml: string, id = "page") { const iframe = parent.window.document.getElementById(id); diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioTextHighlightManager.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioTextHighlightManager.ts new file mode 100644 index 000000000000..d27b6245642e --- /dev/null +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/audioTextHighlightManager.ts @@ -0,0 +1,359 @@ +import StyleEditor from "../../StyleEditor/StyleEditor"; + +const kSegmentClass = "bloom-highlightSegment"; +const kEnableHighlightClass = "ui-enableHighlight"; +const kDisableHighlightClass = "ui-disableHighlight"; +const kPostAudioSplitClass = "bloom-postAudioSplit"; +const kTextBoxRecordingMode = "textbox"; + +const kCurrentHighlightBackgroundCssVar = + "--bloom-audio-current-highlight-background"; +const kCurrentHighlightColorCssVar = "--bloom-audio-current-highlight-color"; + +// This manager translates Bloom's audio-highlight classes into the CSS highlight registry and +// ::highlight pseudo-elements. +// In rare cases, the browser can automatically move computed css into an inline style within a contenteditable, which +// we suspect is causing BL-15300 where TBT highlighting gets stuck in the book. This method of highlighting without +// modifying the dom should prevent that, and is also the direction we want to move in for highlighting. +// The DOM still decides which pieces of text are eligible and marks them with the appropriate classes, but in the Edit +// Tab the visible paint comes from ::highlight pseudo-elements instead of the element +// background colors, which we continue to use in Bloom Player etc. - we will need to keep the original class and css +// rules for a while so that old versions of Bloom player display the highlights, but a next step would be to make +// newer versions of Bloom Player switch to using pseudo-elements to display highlights like we do here + +export const currentHighlightName = "bloom-audio-current"; + +export const splitHighlightNames = [ + "bloom-audio-split-1", + "bloom-audio-split-2", + "bloom-audio-split-3", +] as const; + +const allManagedHighlightNames = [currentHighlightName, ...splitHighlightNames]; + +type HighlightRegistry = Map; +type HighlightConstructor = new (...ranges: Range[]) => unknown; + +function getDocumentWindow(contextNode: Node): Window | undefined { + return contextNode.ownerDocument?.defaultView ?? undefined; +} + +function getDocumentElement(contextNode: Node): HTMLElement | undefined { + return contextNode.ownerDocument?.documentElement ?? undefined; +} + +function getHighlightRegistry( + contextNode: Node, +): HighlightRegistry | undefined { + const docWindow = getDocumentWindow(contextNode) as + | (Window & typeof globalThis) + | undefined; + const cssWithHighlights = docWindow?.CSS as + | (typeof globalThis.CSS & { + highlights?: HighlightRegistry; + }) + | undefined; + return cssWithHighlights?.highlights; +} + +function getHighlightConstructor( + contextNode: Node, +): HighlightConstructor | undefined { + const docWindow = getDocumentWindow(contextNode) as + | (Window & { + Highlight?: HighlightConstructor; + }) + | undefined; + return docWindow?.Highlight; +} + +// A StyleEditor instance used only to read the user's audio-highlight colors from a book's +// userModifiedStyles sheet. We share StyleEditor's rule lookup rather than duplicating the +// selector-matching logic, so the read and the write (StyleEditor.putAudioHiliteRulesInDom) +// can never drift apart. supportFilesRoot is irrelevant for this read-only use. +let styleEditorForColorLookup: StyleEditor | undefined; +function getStyleEditorForColorLookup(): StyleEditor { + if (!styleEditorForColorLookup) { + styleEditorForColorLookup = new StyleEditor("/bloom/bookEdit"); + } + return styleEditorForColorLookup; +} + +export class AudioTextHighlightManager { + // Remove all current and split highlights from the registry for the document containing contextNode. + public clearAllManagedHighlights(contextNode?: Node): void { + if (!contextNode) { + return; + } + + const registry = getHighlightRegistry(contextNode); + if (!registry) { + return; + } + + allManagedHighlightNames.forEach((name) => registry.delete(name)); + } + + // Remove only the split (blue segment) highlights from the registry, leaving the current highlight intact. + public clearSplitHighlights(contextNode?: Node): void { + if (!contextNode) { + return; + } + + const registry = getHighlightRegistry(contextNode); + if (!registry) { + return; + } + + splitHighlightNames.forEach((name) => registry.delete(name)); + } + + // currentHighlight is the element currently selected for recording etc. + // It might be a span (sentence mode) or text box (text box mode). + // currentTextBox is either the same as currentHighlight (text box mode) + // or its TextBox ancestor (sentence mode). + // Adjust pseudo-element highlights to what they should be for this state of things. + public refreshHighlights( + currentHighlight: Element | null, + currentTextBox: HTMLElement | null, + suppressCurrentHighlight?: boolean, + ): void { + const contextNode = currentHighlight ?? currentTextBox; + if (!contextNode) { + return; + } + + const registry = getHighlightRegistry(contextNode); + const Highlight = getHighlightConstructor(contextNode); + if (!registry || !Highlight) { + return; + } + + if (suppressCurrentHighlight) { + allManagedHighlightNames.forEach((name) => registry.delete(name)); + return; + } + + // Split highlights (blue segments after a textbox recording) and the current highlight + // (yellow sentence) are mutually exclusive: split state replaces the yellow highlight. + if (this.shouldShowSplitHighlights(currentHighlight, currentTextBox)) { + registry.delete(currentHighlightName); + this.refreshSplitHighlights(currentTextBox, registry, Highlight); + } else { + splitHighlightNames.forEach((name) => registry.delete(name)); + this.refreshCurrentHighlight( + currentHighlight, + currentTextBox, + registry, + Highlight, + ); + } + } + + private refreshCurrentHighlight( + currentHighlight: Element | null, + currentTextBox: HTMLElement | null, + registry: HighlightRegistry, + Highlight: HighlightConstructor, + ): void { + const highlightInfo = this.getCurrentHighlightInfo( + currentHighlight, + currentTextBox, + ); + if (!highlightInfo || highlightInfo.ranges.length === 0) { + registry.delete(currentHighlightName); + return; + } + + // enhance: don't check for highlight color settings changes so often + this.updateCurrentHighlightColors(highlightInfo.styleSource); + registry.set( + currentHighlightName, + new Highlight(...highlightInfo.ranges), + ); + } + + private refreshSplitHighlights( + currentTextBox: HTMLElement, + registry: HighlightRegistry, + Highlight: HighlightConstructor, + ): void { + // Cycle through 3 colors using a page-relative index so adjacent paragraphs + // never share the same color at their boundary. + const rangesByName = new Map(); + splitHighlightNames.forEach((name) => rangesByName.set(name, [])); + + Array.from( + currentTextBox.querySelectorAll(`span.${kSegmentClass}`), + ).forEach((segment, index) => { + const highlightName = + splitHighlightNames[index % splitHighlightNames.length]; + const ranges = rangesByName.get(highlightName); + ranges?.push(...this.getRangesForSegment(segment)); + }); + + splitHighlightNames.forEach((name) => { + const ranges = rangesByName.get(name) ?? []; + if (ranges.length > 0) { + registry.set(name, new Highlight(...ranges)); + } else { + registry.delete(name); + } + }); + } + + private getCurrentHighlightInfo( + currentHighlight: Element | null, + currentTextBox: HTMLElement | null, + ): + | { + ranges: Range[]; + styleSource: Element; + } + | undefined { + if (!currentHighlight) { + return undefined; + } + + // copilot says: fixHighlighting() can carve the visible pieces into nested ui-enableHighlight + // spans so punctuation or outer whitespace stays unpainted. Prefer those exact + // spans whenever they exist so the pseudo-highlight matches the background-color highlight behavior + const enabledDescendants = Array.from( + currentHighlight.querySelectorAll(`span.${kEnableHighlightClass}`), + ); + const enabledRanges = enabledDescendants + .map((enabledSpan) => this.makeRange(enabledSpan)) + .filter((range): range is Range => !!range); + if (enabledRanges.length > 0) { + return { + ranges: enabledRanges, + styleSource: enabledDescendants[0], + }; + } + + if (currentHighlight.classList.contains(kDisableHighlightClass)) { + return undefined; + } + + if (currentHighlight === currentTextBox) { + const paragraphs = Array.from(currentTextBox.querySelectorAll("p")); + const paragraphRanges = paragraphs + .map((paragraph) => this.makeRange(paragraph)) + .filter((range): range is Range => !!range); + if (paragraphRanges.length > 0) { + return { + ranges: paragraphRanges, + styleSource: paragraphs[0], + }; + } + } + + const wholeElementRange = this.makeRange(currentHighlight); + if (!wholeElementRange) { + return undefined; + } + + return { + ranges: [wholeElementRange], + styleSource: currentHighlight, + }; + } + + // Set the CSS variables that control the ::highlight colors to match the user's chosen + // highlight color for the current text style, falling back to the default yellow. + private updateCurrentHighlightColors(styleSource: Element): void { + const documentElement = getDocumentElement(styleSource); + if (!documentElement) { + console.error( + "AudioTextHighlightManager.updateCurrentHighlightColors() could not find documentElement for the style source.", + ); + return; + } + + const bloomEditable = styleSource.closest(".bloom-editable"); + const styleName = bloomEditable + ? Array.from(bloomEditable.classList).find((c) => + c.endsWith("-style"), + ) + : undefined; + + // Read the user's chosen highlight colors for this style from the same + // userModifiedStyles rules that StyleEditor.putAudioHiliteRulesInDom writes, by + // delegating to StyleEditor's own lookup (looking in the page's document, not the + // toolbox's). When there is no such style, fall back to the default yellow/black. + const userColors = styleName + ? getStyleEditorForColorLookup().getAudioHiliteProps( + styleName, + styleSource.ownerDocument, + ) + : undefined; + + documentElement.style.setProperty( + kCurrentHighlightBackgroundCssVar, + userColors?.hiliteBgColor ?? "#febf00", + ); + documentElement.style.setProperty( + kCurrentHighlightColorCssVar, + userColors?.hiliteTextColor ?? "black", + ); + } + + private shouldShowSplitHighlights( + currentHighlight: Element | null, + currentTextBox: HTMLElement | null, + ): currentTextBox is HTMLElement { + if (!currentHighlight || !currentTextBox) { + return false; + } + + if (currentHighlight !== currentTextBox) { + return false; + } + + if (currentTextBox.classList.contains(kDisableHighlightClass)) { + return false; + } + + // Split highlights are only for textbox recordings after AudioRecording has + // split the textbox into segment spans and marked it as post-split. + return ( + currentTextBox.classList.contains(kPostAudioSplitClass) && + currentTextBox + .getAttribute("data-audiorecordingmode") + ?.toLowerCase() === kTextBoxRecordingMode + ); + } + + private getRangesForSegment(segment: Element): Range[] { + const enabledRanges = Array.from( + segment.querySelectorAll(`span.${kEnableHighlightClass}`), + ) + .map((enabledSpan) => this.makeRange(enabledSpan)) + .filter((range): range is Range => !!range); + + if (enabledRanges.length > 0) { + return enabledRanges; + } + + const wholeSegmentRange = this.makeRange(segment); + return wholeSegmentRange ? [wholeSegmentRange] : []; + } + + private makeRange(node: Node): Range | undefined { + if (node.textContent === null || node.textContent.length === 0) { + return undefined; + } + + const ownerDocument = node.ownerDocument; + if (!ownerDocument) { + console.error( + "AudioTextHighlightManager.makeRange() could not find ownerDocument for a highlighted node.", + ); + return undefined; + } + + const range = ownerDocument.createRange(); + range.selectNodeContents(node); + return range; + } +} diff --git a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/talkingBookSpec.ts b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/talkingBookSpec.ts index bb8e8c0e7311..4625f83aba36 100644 --- a/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/talkingBookSpec.ts +++ b/src/BloomBrowserUI/bookEdit/toolbox/talkingBook/talkingBookSpec.ts @@ -390,16 +390,24 @@ describe("talking book tests", () => { } } + // The new highlight code tracks highlighting via highlightedElement instead of DOM + // class manipulation. setCurrentAudioElementToDefaultAsync() strips ui-audioCurrent + // from all DOM elements as defensive cleanup, so the class will never appear in + // outerHTML after showTool() — strip it from both sides before comparing. + function stripAudioCurrent(html: string): string { + return html.replace(/\s*\bui-audioCurrent\b/g, ""); + } + function verifyHtmlPreserved(_scenario: AudioMode) { const currentHtml1 = getFrameElementById("page", "div1") ?.outerHTML as string; - expect(StripRecordingMd5(currentHtml1)).toBe( - StripRecordingMd5(originalDiv1Html), + expect(StripRecordingMd5(stripAudioCurrent(currentHtml1))).toBe( + StripRecordingMd5(stripAudioCurrent(originalDiv1Html)), ); const currentHtml2 = getFrameElementById("page", "div2") ?.outerHTML as string; - expect(StripRecordingMd5(currentHtml2)).toBe( - StripRecordingMd5(originalDiv2Html), + expect(StripRecordingMd5(stripAudioCurrent(currentHtml2))).toBe( + StripRecordingMd5(stripAudioCurrent(originalDiv2Html)), ); } @@ -429,18 +437,18 @@ describe("talking book tests", () => { return; } - const page1 = getFrameElementById("page", "page1"); - const numCurrents = - page1?.querySelectorAll(".ui-audioCurrent").length; + // The new code tracks the current element via highlightedElement rather than + // the ui-audioCurrent DOM class. Use getCurrentHighlight() instead. + const currentHighlight = theOneAudioRecorder.getCurrentHighlight(); expect( - numCurrents, - "Only 1 item is allowed to be the current: " + page1?.innerHTML, - ).toBe(1); + currentHighlight, + "getCurrentHighlight() should return a non-null element after showTool", + ).not.toBeNull(); switch (scenario) { case AudioMode.PureSentence: { const firstSpan = div.querySelector("span.audio-sentence"); - expect(firstSpan).toHaveClass("ui-audioCurrent"); + expect(currentHighlight).toBe(firstSpan); break; } @@ -448,7 +456,7 @@ describe("talking book tests", () => { case AudioMode.PureTextBox: case AudioMode.HardSplitTextBox: case AudioMode.SoftSplitTextBox: { - expect(div).toHaveClass("ui-audioCurrent"); + expect(currentHighlight).toBe(div); break; } diff --git a/src/BloomBrowserUI/bookEdit/workspaceRoot.ts b/src/BloomBrowserUI/bookEdit/workspaceRoot.ts index ae0dcb2c9eb2..febdbb3065cc 100644 --- a/src/BloomBrowserUI/bookEdit/workspaceRoot.ts +++ b/src/BloomBrowserUI/bookEdit/workspaceRoot.ts @@ -34,6 +34,7 @@ export interface IWorkspaceExports { showCopyrightAndLicenseDialog(imageUrl?: string): void; showEditViewTopicChooserDialog(): void; showAdjustTimingsDialogFromWorkspaceRoot( + currentTextBox: HTMLElement, // The split and applyTimingsFile calls both return a list of new timings, // such as we might find in data-audioRecordingEndTimes split: (timingFilePath: string) => Promise, diff --git a/src/BloomBrowserUI/vite.config.mts b/src/BloomBrowserUI/vite.config.mts index 1c2e7344c3c6..87e1329f821d 100644 --- a/src/BloomBrowserUI/vite.config.mts +++ b/src/BloomBrowserUI/vite.config.mts @@ -813,11 +813,18 @@ export default defineConfig(async ({ command }) => { globals: false, // Don't inject global test functions (use imports instead) testTimeout: 30000, // 30 second timeout for async operations teardownTimeout: 10000, // 10s max for after-test cleanup; prevents hung workers from blocking the pool - // Cap parallel workers. On Windows, running too many jsdom workers simultaneously - // may exhaust system resources (TCP connections to localhost, libuv thread pool), causing - // workers to stall indefinitely. This may slightly slow things down on some computers - // but reduces the chance of the tests hanging altogether. - maxWorkers: "70%", + // Use worker threads instead of child-process forks. Vitest 4.0 changed the + // default pool to 'forks', but on Windows the process-creation overhead causes + // workers to time out before they start ("Timeout starting forks runner"). + // The threads pool is lighter-weight and avoids that issue. + pool: "threads", + // Limit concurrent workers. The heavy transform cost of some test files + // (notably PrepareAppStepper.spec.tsx with its deep MUI import chain) saturates + // the CPU while workers are starting, causing other workers to miss the + // hardcoded 5-second vitest startup timeout. With pool: "threads" the + // per-worker startup overhead is negligible, so we can safely use more workers. + // 4 workers roughly halves the wall-clock collect time compared to 2. + maxWorkers: 4, minWorkers: 2, sourcemap: true, // Enable source maps for debugging test code deps: { diff --git a/src/BloomBrowserUI/vitest.setup.ts b/src/BloomBrowserUI/vitest.setup.ts index 93f62fbfe9de..b089cd9542ba 100644 --- a/src/BloomBrowserUI/vitest.setup.ts +++ b/src/BloomBrowserUI/vitest.setup.ts @@ -1,6 +1,11 @@ import "@testing-library/jest-dom/vitest"; import { vi } from "vitest"; +// Tell React 18 that this environment supports act(), suppressing the +// "not configured to support act(...)" warning in every React component test. +(globalThis as unknown as Record).IS_REACT_ACT_ENVIRONMENT = + true; + // Some legacy specs use the Jasmine/Jest-style fail() helper, which vitest does // not provide. Define it globally (throwing an Error) so those assertions both // work at runtime and type-check. Test-only: this file is loaded only by vitest. @@ -44,6 +49,21 @@ console.error = (...args: unknown[]) => { return; } + // React 18 emits two families of act() warnings in jsdom test environments: + // • "not configured to support act()" – the environment flag is absent + // • "not wrapped in act(...)" – an async state update escaped an act() boundary + // Both arise from React 18's concurrent scheduler and from third-party libraries + // (e.g. MUI Accordion/Tooltip transitions) whose timer-based state updates fire + // outside of synchronous act() blocks. All tests still pass; the warnings are + // noise. @testing-library/react suppresses the same patterns for the same reason. + const isReactActWarning = + serialized.includes("not wrapped in act(...)") || + serialized.includes("not configured to support act(...)"); + + if (isReactActWarning) { + return; + } + originalConsoleError(...args); }; diff --git a/src/content/bookLayout/basePage-sharedRules.less b/src/content/bookLayout/basePage-sharedRules.less index 5ee951ca6dc7..395f1862d61e 100644 --- a/src/content/bookLayout/basePage-sharedRules.less +++ b/src/content/bookLayout/basePage-sharedRules.less @@ -15,19 +15,19 @@ body { // Bloom applies in the talking-book tool. Accordingly, we tell the agent to // apply that class. However, the user may NOT have configured a custom highlighting. // In case they do not, this provides the default highlighting. In this location, -// it works for edit mode, bloomPubs, both kinds of ePUBs, and very likely anywhere -// else we'd want ui-audioCurrent to produce a highlight. +// it works for bloomPubs and both kinds of ePUBs, and very likely anywhere +// else we'd want ui-audioCurrent to produce a highlight. (As of March 2026, in the +// edit tab we are using highlighting pseudo-elements instead, BL-15300. Those read +// the computed highlight colors and copy them to document-level variables.) // Setting the foreground color is usually redundant, but digital comic book covers // use white text, and it's now possible for the user to choose a text color, so // we want to ensure a contrast with the background color (unless the user overrides, // in which case, adequate contrast is up to them). // (Color is @bloom-yellow.) -// (When editing the text, this background causes problems (due to a Chromium bug) -// if the elements have their default position: relative. This is overidden in -// audioRecording.less.) span.ui-audioCurrent, div.ui-audioCurrent p, .ui-audioCurrent .ui-enableHighlight { + // Default audio highlight colors background-color: #febf00; color: black; }