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

.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

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;
         }

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