Improve highlighting in edit not to modify dom (BL-15300)

src/BloomBrowserUI/bookEdit/StyleEditor/StyleEditor.ts

@@ -695,8 +695,10 @@ export default class StyleEditor {
             this.sentenceHiliteRuleSelector,
             false,
         );
-        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;
         }

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 <CSSStyleSheet>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 = (<CSSStyleSheet>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 = (<CSSStyleSheet>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 = (<CSSStyleSheet>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 = (<CSSStyleSheet>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 = (<CSSStyleSheet>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 = (<CSSStyleSheet>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.

src/BloomBrowserUI/bookEdit/css/editMode.less

@@ -1036,15 +1036,31 @@ even when the div is focused (as long as it is empty).*/
     color: @bloom-buff;
 }
 
+// BL-11633: Works around a Chromium bug where elements with position:relative and a
+// background color cause the caret to disappear. We unset position on the element
+// itself and on any inline text-formatting children, since the bug can trigger at
+// any level of nesting.
+.bl11633-unset-position() {
+    position: unset;
+    em,
+    i,
+    strong,
+    span[style],
+    u,
+    sup {
+        position: unset;
+    }
+}
+
 /*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 +1084,20 @@ 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
-    }
+    .bl11633-unset-position();
+}
+
+// p elements can be the ::highlight range in text-box recording mode; they need position:unset
+// for the same BL-11633 reason (caret hidden behind a background-colored ancestor).
+// Sentence spans are the ::highlight range in sentence mode, so they need it too.
+div.bloom-editable p,
+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 +1240,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

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<string | undefined>;
     editTimingsFile: (timingsFilePath?: string) => Promise<void>;
     applyTimingsFile: (timingsFilePath?: string) => Promise<string | undefined>;
@@ -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={<EditIcon />}
@@ -387,9 +391,9 @@ export const AdjustTimingsDialog: React.FunctionComponent<{
             <DialogBottomButtons>
                 <DialogOkButton
                     onClick={() => {
-                        // 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<string | undefined>,
     editTimingsFile: (timingsFilePath?: string) => Promise<void>,
     applyTimingsFile: (timingsFilePath?: string) => Promise<string | undefined>,
@@ -420,6 +425,7 @@ export function showAdjustTimingsDialog(
     try {
         renderRootSync(
             <AdjustTimingsDialog
+                currentTextBox={currentTextBox}
                 split={split}
                 editTimingsFile={editTimingsFile}
                 applyTimingsFile={applyTimingsFile}
@@ -448,21 +454,13 @@ function getModalContainer(): HTMLElement {
     return modalDialogContainer;
 }
 
-function getCurrentTextBox(): HTMLElement {
-    const page = parent.window.document.getElementById(
-        "page",
-    ) as HTMLIFrameElement;
-    const pageBody = page.contentWindow!.document.body;
-    const audioCurrentElements = pageBody.getElementsByClassName(kAudioCurrent);
-    const currentTextBox = audioCurrentElements.item(0) as HTMLElement;
-    return currentTextBox;
-}
-
 // Existing code wants to be passed the actual path (not a BloomServer url) to the timings file.
 // Rather than try to rework all that, I made an api that allows us to get such a path
 // from a path starting at the current book's folder.
-async function getTimingsFileName(): Promise<string> {
-    const bloomEditable = getCurrentTextBox();
+async function getTimingsFileName(
+    currentTextBox: HTMLElement,
+): Promise<string> {
+    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[];