From 3df06c8fd24f3f614b2a24d57c28340dbd0ddfd8 Mon Sep 17 00:00:00 2001
From: Hatton <hattonjohn@gmail.com>
Date: Tue, 10 Dec 2024 11:14:01 -0700
Subject: [PATCH] chore: final review session with JohnT

---
 public/testBooks/multibook-index/history.db   | Bin 24576 -> 0 bytes
 .../multibook-index/publish-settings.json     |   1 -
 src/bloom-player-controls.tsx                 |   4 +-
 src/bloom-player-core.tsx                     | 290 +-----------------
 src/externalContext.ts                        |   2 +-
 src/navigation.test.ts                        |  17 +-
 src/navigation.ts                             |  22 +-
 src/stories/BloomPlayerTester.ts              |   8 +-
 8 files changed, 36 insertions(+), 308 deletions(-)
 delete mode 100644 public/testBooks/multibook-index/history.db
 delete mode 100644 public/testBooks/multibook-index/publish-settings.json

diff --git a/public/testBooks/multibook-index/history.db b/public/testBooks/multibook-index/history.db
deleted file mode 100644
index ce655a89498d4045f1b798347b57ea02d7541dfa..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 24576
zcmeI)%Wu;#90zcFeQd3?1rAeGaj|k?)z)fjyG<OB_Mo7lfp~1<ajEj~TC>F2oMbR=
z6@L(a1}8Xk;m?@F5hra?TLhZ41Do`X>eR7p_xJnMmps~+&-X>l*-@whJ7!ICi%?3o
z7$bxfbSdgG+H$&*A06m-I#&N%wLt10Pv?!Fq%i-P7!QrF3$GT^DmDl}00Izz00bZa
z0SG_<0ucDOz(JvCRx30S(&Hca8JBU?al-JpBfP;;arZ@g`=HJCo<3;5X4YgEi-mI3
zW-VF6fmZcdzGyBl(>Jm0_&hrG^%-F&;-?9h?(|_{IRC-+&VHLs^<9#7z*qFhYTwVz
znw1Lu)Xe4_gfZCXCuSH%XtAZTwdYw($e4GzVkb%jwmN6W{G8cI917_w9_W0T40TC-
zpRJUwB;rcwQD?Std$t-irD04o#^)!xVKnz}U?<>FWOpZfdUmWE9MRRC)9KE*ibN<U
zW*fQv{@7|iPiIZjr2TG|ZrUNrb~BU7W^Gxn<57?2I-cDhX`FEB3EAE4araorC)43q
zt20IOzDY^8I)>lP08g)$Y@I}%wJe78Tz)QHYV*2K;|tL*Y!H9|1Rwwb2tWV=5P$##
zAOHafTyKF*nm=474c@xf;LcjjaozP=%V{_@XLEg{w&8kqv&q*u=dB>|W0771sAX3P
z$~rG<d?)&a4FV8=00bZa0SG_<0uX=z1Rwx`8zyi!cUZd2<$%Hce~%d64NDN=LI45~
zfB*y_009U<00Izz00gd?K)*uEq`64@i^V&K@4}w!^z1ke<yIv8dZ@aOb<@nQ;`*lt
z9<!O`?^$|-o>^1(>MeGp!a(<k!bG`z*lEX(gv)mH0Bi2Y?YED<^-CM|Mt!xudinGJ
z3u0VcGc<$)0SG_<0uX=z1Rwwb2tWV=5P-nd1WGhlDGmOO0nh(mjS{0F009U<00Izz
U00bZa0SG_<0{>Isvd{nj0=l>`P5=M^

diff --git a/public/testBooks/multibook-index/publish-settings.json b/public/testBooks/multibook-index/publish-settings.json
deleted file mode 100644
index 261f25e..0000000
--- a/public/testBooks/multibook-index/publish-settings.json
+++ /dev/null
@@ -1 +0,0 @@
-{"audioVideo":{"pageTurnDelay":3000,"format":"facebook","motion":false,"playerSettings":"","pageRange":[]},"epub":{"howToPublishImageDescriptions":0,"removeFontSizes":false,"mode":"fixed"},"bloomPUB":{"motion":true,"textLangs":{},"audioLangs":{},"signLangs":{},"imageSettings":{"maxWidth":1280,"maxHeight":720}},"bloomLibrary":{"textLangs":{},"audioLangs":{},"signLangs":{},"comic":true}}
\ No newline at end of file
diff --git a/src/bloom-player-controls.tsx b/src/bloom-player-controls.tsx
index 2101755..7e07a6d 100644
--- a/src/bloom-player-controls.tsx
+++ b/src/bloom-player-controls.tsx
@@ -5,7 +5,7 @@ book inside of the Bloom:Publish:Android screen.
 import { BloomPlayerCore } from "./bloom-player-core";
 import * as ReactDOM from "react-dom";
 import {
-    sendBackToHost,
+    informHostOfBackAction,
     showNavBar,
     hideNavBar,
     reportBookProperties,
@@ -933,7 +933,7 @@ export const BloomPlayerControls: React.FunctionComponent<BloomPlayerProps> = (
                     if (
                         !coreRef.current?.HandleBackButtonClickedIfHavePlayerHistory()
                     ) {
-                        sendBackToHost();
+                        informHostOfBackAction();
                     }
                 }}
                 showPlayPause={hasAudio || hasMusic || hasVideo}
diff --git a/src/bloom-player-core.tsx b/src/bloom-player-core.tsx
index 1e1c122..7907238 100644
--- a/src/bloom-player-core.tsx
+++ b/src/bloom-player-core.tsx
@@ -3,8 +3,7 @@ bloom-player-core is responsible for all the behavior of working through a book,
 (other than page turning).
 */
 import * as React from "react";
-import * as ReactDOM from "react-dom";
-import axios, { AxiosPromise } from "axios";
+import axios from "axios";
 import Swiper, { SwiperInstance } from "react-id-swiper";
 // This loads some JS right here that is a polyfill for the (otherwise discontinued) scoped-styles html feature
 import "style-scoped/scoped.min.js";
@@ -20,7 +19,6 @@ import { LocalizationManager } from "./l10n/localizationManager";
 import {
     reportAnalytics,
     reportPlaybackComplete,
-    sendMessageToHost,
     setAmbientAnalyticsProperties,
     updateBookProgressReport,
 } from "./externalContext";
@@ -30,7 +28,6 @@ import {
     sendStringToBloomApi,
 } from "./videoRecordingSupport";
 import LangData from "./langData";
-import Replay from "@material-ui/icons/Replay";
 
 // See related comments in controlBar.tsx
 import IconButton from "@material-ui/core/IconButton";
@@ -83,7 +80,7 @@ import { assembleStyleSheets } from "./stylesheets";
 import {
     canGoBack,
     checkClickForBookOrPageJump,
-    goBackInHistoryIfPossible,
+    tryPopPlayerHistory,
 } from "./navigation";
 
 // BloomPlayer takes a URL param that directs it to Bloom book.
@@ -428,9 +425,7 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
     }
     // called by bloom-player-controls
     public HandleBackButtonClickedIfHavePlayerHistory(): boolean {
-        const backLocation = goBackInHistoryIfPossible(
-            this.bookInfo.bookInstanceId,
-        );
+        const backLocation = tryPopPlayerHistory(this.bookInfo.bookInstanceId);
         if (backLocation) {
             this.navigate(backLocation.bookUrl, backLocation.pageId);
             return true;
@@ -1667,285 +1662,6 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
         }
     }
 
-    // Make a <style> element in the head of the document containing an adjusted version
-    // of the "fonts.css" file shipped with the book, if any.
-    // This file typically contains @font-face declarations, which don't work when embedded
-    // in the <scoped> element we make for each page contents. So we need the rules to
-    // be placed in the <head> of the main document. (We don't want to do this with most
-    // rules because rules from a book, especially from a customStyles file, could mess up
-    // bloom-player itself.)
-    // Since the root file, typically bloomPlayer.htm, is typically not at the same location
-    // as the book files, the simple URLs it contains don't work, since they assume fonts.css
-    // is loaded as part of a file in the same location as the font files. So we modify
-    // the URLs in the file to be relative to the book folder.
-    // There are possible dangers here...if someone got something malicious into fonts.css
-    // it could at least mess up our player...but only for that one book, and in any case,
-    // fonts.css is generated by the harvester so it shouldn't be possible for anyone without
-    // harvester credentials to post a bad version of it.
-    // Enhance: currently we're just looking for the (typically ttf) font uploaded as part
-    // of the book if embedding is permitted. Eventually, we might want to try first for
-    // a corresponding woff font in a standard location. We may even be able to pay to
-    // provide some fonts there for book previewing that are NOT embeddable.
-    private makeFontStylesheet(href: string): void {
-        axios.get(href).then((result) => {
-            let stylesheet = document.getElementById(
-                "fontCssStyleSheet",
-            ) as HTMLStyleElement;
-            if (!stylesheet) {
-                stylesheet = document.createElement("style");
-                document.head.appendChild(stylesheet);
-                stylesheet.setAttribute("id", "fontCssStyleSheet");
-            }
-            const prefix = href.substring(0, href.length - "/fonts.css".length);
-            stylesheet.innerText = result.data.replace(
-                // This is so complex because at one time we weren't adding the
-                // quotes around the original url. So, now we handle no quotes,
-                // single quotes, and double quotes. Note that we also have to
-                // handle possible parentheses in the file name.
-                /src:url\(['"]?(.*\.[^\)'"]*)['"]?\)/g,
-                "src:url('" + prefix + "/$1')",
-            );
-        });
-        // no catch clause...if there's no fonts.css, we should never get a 'then' and
-        // don't need to do anything.
-    }
-
-    // Make a stylesheet that tells the player where to find Andika New Basic.
-    // This can't be part of the <style scoped> where most style rules for the book
-    // content go, because @font-face rules don't work there.
-    private makeAndikaStylesheet(): void {
-        let stylesheet = document.getElementById(
-            "andikaCssStyleSheet",
-        ) as HTMLStyleElement;
-        if (!stylesheet) {
-            stylesheet = document.createElement("style");
-            document.head.appendChild(stylesheet);
-            stylesheet.setAttribute("id", "andikaCssStyleSheet");
-        }
-
-        // Starting in Jul 2022, we provide Andika as a fallback to Andika New Basic.
-        // (And we ship our hosts with Andika instead of ANB.)
-        //
-        // 1. The font might be found already installed (local).
-        // 2. Failing that, if we're inside BloomReader, BloomPUB Viewer, or another host we control,
-        //    we add the fake relative path `./host/fonts/*` so the host can intercept
-        //    this request and serve the appropriate font file.
-        //    (RAB doesn't use this stylesheet at all; rather, it modifies fonts.css.)
-        // 3. If instead we're embedded in a web page like BloomLibrary.org, we need to download from the web.
-        //
-        // (Jun 2022) I'm not sure this is (still) true:
-        // Note that currently that last option will only work when the page origin
-        // is *bloomlibrary.org. This helps limit our exposure to large charges from
-        // people using our font arbitrarily. This does include, however, books
-        // displayed in an iframe using https://bloomlibrary.org/bloom-player/bloomplayer.htm
-        //
-        // Previously, this conditionally included some file:// urls for Android which were
-        // conditionally included because they caused problems for Safari on iOS. That is why
-        // they are here instead of bloom-player-content.less. In their current state,
-        // I believe they could be moved there. But we may need conditional logic again...
-        stylesheet.innerText = `
-            @font-face {
-                font-family: "Andika New Basic";
-                font-weight: normal;
-                font-style: normal;
-                src:
-                    local("Andika New Basic"),
-                    local("Andika"),
-                    url("./host/fonts/Andika New Basic"),
-                    url("https://bloomlibrary.org/fonts/Andika%20New%20Basic/AndikaNewBasic-R.woff")
-                ;
-            }
-
-            @font-face {
-                font-family: "Andika New Basic";
-                font-weight: bold;
-                font-style: normal;
-                src:
-                    local("Andika New Basic Bold"),
-                    local("Andika Bold"),
-                    url("./host/fonts/Andika New Basic Bold"),
-                    url("https://bloomlibrary.org/fonts/Andika%20New%20Basic/AndikaNewBasic-B.woff")
-                ;
-            }
-
-            @font-face {
-                font-family: "Andika New Basic";
-                font-weight: normal;
-                font-style: italic;
-                src:
-                    local("Andika New Basic Italic"),
-                    local("Andika Italic"),
-                    url("./host/fonts/Andika New Basic Italic"),
-                    url("https://bloomlibrary.org/fonts/Andika%20New%20Basic/AndikaNewBasic-I.woff")
-                ;
-            }
-
-            @font-face {
-                font-family: "Andika New Basic";
-                font-weight: bold;
-                font-style: italic;
-                src:
-                    local("Andika New Basic Bold Italic"),
-                    local("Andika Bold Italic"),
-                    url("./host/fonts/Andika New Basic Bold Italic"),
-                    url("https://bloomlibrary.org/fonts/Andika%20New%20Basic/AndikaNewBasic-BI.woff")
-                ;
-            }
-
-            @font-face {
-                font-family: "Andika";
-                font-weight: normal;
-                font-style: normal;
-                src:
-                    local("Andika"),
-                    url("./host/fonts/Andika"),
-                    url("https://bloomlibrary.org/fonts/Andika/Andika-Regular.woff")
-                ;
-            }
-
-            @font-face {
-                font-family: "Andika";
-                font-weight: bold;
-                font-style: normal;
-                src:
-                    local("Andika Bold"),
-                    url("./host/fonts/Andika Bold"),
-                    url("https://bloomlibrary.org/fonts/Andika/Andika-Bold.woff")
-                ;
-            }
-
-            @font-face {
-                font-family: "Andika";
-                font-weight: normal;
-                font-style: italic;
-                src:
-                    local("Andika Italic"),
-                    url("./host/fonts/Andika Italic"),
-                    url("https://bloomlibrary.org/fonts/Andika/Andika-Italic.woff")
-                ;
-            }
-
-            @font-face {
-                font-family: "Andika";
-                font-weight: bold;
-                font-style: italic;
-                src:
-                    local("Andika Bold Italic"),
-                    url("./host/fonts/Andika Bold Italic"),
-                    url("https://bloomlibrary.org/fonts/Andika/Andika-BoldItalic.woff")
-                ;
-            }
-
-            .do-not-display {
-                display:none !important;
-            }
-            `
-
-            // (Jun 2022) these are not actually preventing <br> tags:
-            .replace("\n", "")
-            .replace("\r", ""); // newlines turn to <br> which is wrong in style element
-    }
-
-    // Assemble all the style rules from all the stylesheets the book contains or references.
-    // When the async completes, the result will be set as our state.styles with setState().
-    // Exception: a stylesheet called "fonts.css" will instead be loaded into the <head>
-    // of the main document, since it contains @font-face declarations that don't work
-    // in the <scoped> element.
-    private async assembleStyleSheets(doc: HTMLHtmlElement): Promise<string> {
-        this.makeAndikaStylesheet();
-        const linkElts = doc.ownerDocument!.evaluate(
-            ".//link[@href and @type='text/css']",
-            doc,
-            null,
-            XPathResult.UNORDERED_NODE_SNAPSHOT_TYPE,
-            null,
-        );
-        const promises: Array<AxiosPromise<any>> = [];
-        for (let i = 0; i < linkElts.snapshotLength; i++) {
-            const link = linkElts.snapshotItem(i) as HTMLElement;
-            const href = link.getAttribute("href");
-            const fullHref = this.fullUrl(href);
-            if (fullHref.endsWith("/fonts.css")) {
-                this.makeFontStylesheet(fullHref);
-            } else {
-                promises.push(axios.get(fullHref));
-                if (fullHref.endsWith("/appearance.css")) {
-                    this.bookInfo.hasAppearanceSystem = true;
-                }
-            }
-        }
-        const p = this.legacyQuestionHandler.getPromiseForAnyQuizCss();
-        if (p) {
-            promises.push(p);
-        }
-
-        try {
-            const results = await axios.all(
-                promises.map((promise) =>
-                    promise.catch(
-                        // if one stylesheet doesn't exist or whatever, keep going
-                        () => undefined,
-                    ),
-                ),
-            );
-
-            let combinedStyle = "";
-
-            // start with embedded styles (typically before links in a bloom doc...)
-            const styleElts = doc.ownerDocument!.evaluate(
-                ".//style[@type='text/css']",
-                doc,
-                null,
-                XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,
-                null,
-            );
-            for (let k = 0; k < styleElts.snapshotLength; k++) {
-                const styleElt = styleElts.snapshotItem(k) as HTMLElement;
-                combinedStyle += styleElt.innerText;
-            }
-
-            // then add the stylesheet contents we just retrieved
-            results.forEach((result) => {
-                if (result && result.data) {
-                    // console.log(
-                    //     "Loaded stylesheet: " + JSON.stringify(result.config),
-                    // );
-                    combinedStyle += result.data;
-
-                    // It is somewhat awkward to do this in a method called assembleStyleSheets,
-                    // but this is the best way to access the information currently.
-                    // See further comments in getNationalLanguagesFromCssStyles.
-                    //
-                    // settingsCollectionStyles.css was replaced with defaultLangStyles.css.
-                    // We have to check for both for older books.
-                    if (
-                        result.config!.url!.endsWith(
-                            "/defaultLangStyles.css",
-                        ) ||
-                        result.config!.url!.endsWith(
-                            "/settingsCollectionStyles.css",
-                        )
-                    ) {
-                        this.bookInfo.setLanguage2And3(result.data);
-                    }
-                }
-            });
-
-            // A ":root" selector doesn't work in this scoped css context.
-            // Change all ":root" references to ".bloomPlayer-page", which is the level just above .bloom-page.
-            // We're using this instead of .bloom-page directly so that a selector like `:root .bloom-page` would still work.
-            combinedStyle = combinedStyle.replace(
-                /:root/g,
-                ".bloomPlayer-page",
-            );
-            //console.log("***combinedStyle", combinedStyle);
-            return combinedStyle;
-        } catch (err) {
-            this.HandleLoadingError(err);
-            return "";
-        }
-    }
-
     private fullUrl(url: string | null): string {
         // Enhance: possibly we should only do this if we somehow determine it is a relative URL?
         // But the things we apply it to always are, in bloom books.
diff --git a/src/externalContext.ts b/src/externalContext.ts
index 5dd32a8..f0aa1ca 100644
--- a/src/externalContext.ts
+++ b/src/externalContext.ts
@@ -113,7 +113,7 @@ export function updateBookProgressReport(event: string, properties: any) {
 
 let gotCapabilities = false;
 
-export function sendBackToHost() {
+export function informHostOfBackAction() {
     sendMessageToHost({ messageType: "backButtonClicked" });
 }
 
diff --git a/src/navigation.test.ts b/src/navigation.test.ts
index 64cf2bc..a687b0b 100644
--- a/src/navigation.test.ts
+++ b/src/navigation.test.ts
@@ -1,7 +1,7 @@
 import { describe, test, expect, beforeEach, vi } from "vitest";
 import {
     canGoBack,
-    goBackInHistoryIfPossible,
+    tryPopPlayerHistory,
     checkClickForBookOrPageJump,
 } from "./navigation";
 
@@ -9,7 +9,7 @@ describe("Navigation functions", () => {
     beforeEach(() => {
         // Clear navigation history before each test
         while (canGoBack()) {
-            goBackInHistoryIfPossible("dummy");
+            tryPopPlayerHistory("dummy");
         }
     });
 
@@ -25,16 +25,16 @@ describe("Navigation functions", () => {
         });
     });
 
-    describe("goBackInHistoryIfPossible", () => {
+    describe("tryPopPlayerHistory", () => {
         test("returns undefined when no history", () => {
-            expect(goBackInHistoryIfPossible("book1")).toBeUndefined();
+            expect(tryPopPlayerHistory("book1")).toBeUndefined();
         });
 
         test("returns previous location after navigation", () => {
             const event = createClickEvent("#page2");
             checkClickForBookOrPageJump(event, "book1", () => "page1");
 
-            const result = goBackInHistoryIfPossible("book1");
+            const result = tryPopPlayerHistory("book1");
             expect(result).toEqual({
                 bookUrl: undefined,
                 pageId: "page1",
@@ -114,14 +114,14 @@ describe("Navigation functions", () => {
             checkClickForBookOrPageJump(externalEvent, "book1", () => "page2");
 
             // First back navigation should return to page2 in book1
-            const firstBack = goBackInHistoryIfPossible("book2");
+            const firstBack = tryPopPlayerHistory("book2");
             expect(firstBack).toEqual({
                 bookUrl: "/book/book1/index.htm",
                 pageId: "page2",
             });
 
             // Second back navigation should return to page2 in book1
-            const secondBack = goBackInHistoryIfPossible("book1");
+            const secondBack = tryPopPlayerHistory("book1");
             expect(secondBack).toEqual({
                 // because the target is in the current book, we don't emit a url
                 bookUrl: undefined,
@@ -129,7 +129,7 @@ describe("Navigation functions", () => {
             });
 
             // Third back navigation should return undefined (no more history)
-            const thirdBack = goBackInHistoryIfPossible("book1");
+            const thirdBack = tryPopPlayerHistory("book1");
             expect(thirdBack).toBeUndefined();
         });
 
@@ -152,6 +152,7 @@ function createClickEvent(href: string) {
             href: { nodeValue: href },
         },
         closest: () => mockElement,
+        getAttribute: (attr: string) => mockElement.attributes[attr]?.nodeValue,
     };
 
     return {
diff --git a/src/navigation.ts b/src/navigation.ts
index 78aaa29..bddd013 100644
--- a/src/navigation.ts
+++ b/src/navigation.ts
@@ -26,13 +26,13 @@ export function tryPopPlayerHistory(
 }
 
 export function checkClickForBookOrPageJump(
-    event: any,
+    event: MouseEvent,
     currentBookInstanceId: string,
     getCurrentPageId: () => string,
 ): ILocation | undefined {
     const linkElement = (event.target as HTMLElement).closest(
         "[href], [data-href]",
-    );
+    ) as HTMLElement;
     if (!linkElement) return {};
     event.preventDefault(); // don't let a link click become a drag
     event.stopPropagation();
@@ -75,7 +75,7 @@ export function checkClickForBookOrPageJump(
     } else if (href.startsWith("#")) {
         targetPageId = href.substring(1);
     }
-    const currentPageId = getCurentPageId();
+    const currentPageId = getCurrentPageId();
     if (targetBookId) {
         pushLocation(currentBookInstanceId, currentPageId, "Changing books");
         return {
@@ -106,8 +106,16 @@ function urlFromBookId(bookId: string) {
 
 function parseTargetBookUrl(url: string) {
     // the format is "/book/BOOKID#PAGEID" where the page id is optional
-    const bloomUrl = new URL(url, window.location.origin);
-    const bookId = bloomUrl.pathname.replace("/book/", "");
-    const pageId = bloomUrl.hash.replace("#", "");
-    return { bookId, pageId };
+    try {
+        const bloomUrl = new URL(url, window.location.origin);
+        if (!bloomUrl.pathname.startsWith("/book/")) {
+            throw new Error("Invalid book URL format");
+        }
+        const bookId = bloomUrl.pathname.replace("/book/", "");
+        const pageId = bloomUrl.hash.replace("#", "");
+        return { bookId, pageId };
+    } catch (error) {
+        console.error("Error parsing book URL:", error);
+        return { bookId: undefined, pageId: undefined };
+    }
 }
diff --git a/src/stories/BloomPlayerTester.ts b/src/stories/BloomPlayerTester.ts
index acb0684..d82c724 100644
--- a/src/stories/BloomPlayerTester.ts
+++ b/src/stories/BloomPlayerTester.ts
@@ -1,4 +1,4 @@
-import { userEvent, waitFor, within, expect } from "@storybook/test";
+import { userEvent, waitFor, within, expect, fireEvent } from "@storybook/test";
 export class BloomPlayerTester {
     private iframeDoc: Document;
 
@@ -43,7 +43,11 @@ export class BloomPlayerTester {
             },
             { timeout: 5000 },
         );
-        await userEvent.click(link as HTMLElement);
+        //await userEvent.click(link as HTMLElement);
+        // the "pointerdown" event fails to trigger with the normal click here,
+        // which probably does a number of things, at least mousedown and up.
+        // As a workaround, we're sending the raw "click" event.
+        await fireEvent.click(link as HTMLElement);
     }
 
     async goToNextPage() {