diff --git a/src/bloom-player-core.tsx b/src/bloom-player-core.tsx index c5c104f..5e7411d 100644 --- a/src/bloom-player-core.tsx +++ b/src/bloom-player-core.tsx @@ -73,7 +73,6 @@ import { logSound } from "./videoRecordingSupport"; import { playSoundOf } from "./dragActivityRuntime"; import { addScrollbarsToPage, - ComputeNiceScrollOffsets, kSelectorForPotentialNiceScrollElements, } from "./scrolling"; import { assembleStyleSheets } from "./stylesheets"; @@ -413,61 +412,6 @@ export class BloomPlayerCore extends React.Component { localStorage.setItem(kLocalStorageBookUrlKey, this.sourceUrl); } - // This horrible hack attempts to fix the worst effects of BL-8900. Basically, something inside - // a widget iframe sometimes causes the wrapper that holds the whole collection of pages to scroll - // over to be out of position. It appears to be related to an unknown bug in FF60...a very nasty one - // since anything happening inside an iframe should not be able to affect its size, position, or - // anything outside it (except with the deliberate cooperation of the host page). We have not found - // any good way to stop it happening, but this basically compensates for it and puts the current page - // back where it belongs. Our theory is that the problem occurs when the activity finishes loading, - // typically sometime after the page before it becomes the 'current' page and the activity itself is - // therefore the 'next' page and no longer blocked by laziness. - // Observed cases take about 2s for the problem to appear on a fast computer - // with a good internet. To be fairly sure of catching it if it happens, but not keep using power - // doing this forever, we monitor for it for 30s. - // Note that the initial call that starts this checking process only happens if we are invoked - // from bloomdesktop. Otherwise, this function will never be called at all. - private repairFF60Offset() { - try { - let wrapper = document.getElementsByClassName( - "swiper-wrapper", - )[0] as HTMLElement; - let current = document.getElementsByClassName( - "swiper-slide-active", - )[0] as HTMLElement; - if (wrapper && current && this.msToContinueFF60RepairChecks > 0) { - let error = - wrapper.parentElement!.getBoundingClientRect().left - - current.getBoundingClientRect().left; - if (Math.abs(error) > 1) { - const scale = - wrapper.getBoundingClientRect().width / - wrapper.offsetWidth; - const paddingPx = wrapper.style.paddingLeft; - const padding = - paddingPx.length === 0 - ? 0 - : parseFloat( - paddingPx.substring(0, paddingPx.length - 2), - ); - wrapper.style.paddingLeft = padding + error / scale + "px"; - } - } - } catch (_) { - // If something goes wrong with this...oh well, we tried, and on most - // browsers we didn't need it anyway. - } - this.msToContinueFF60RepairChecks -= 100; - setTimeout(() => this.repairFF60Offset(), 100); - } - - // This stores a number of milliseconds during which we should continue to check repeatedly - // for the FF60 problem described above. It is set to zero when checking should be disabled - // (e.g., during drag or animation) and to a substantial number after we change pages. - // Then it gets decremented in repairFF60Offset, and eventually decrements to zero so we - // don't consume extra power forever doing this check. - private msToContinueFF60RepairChecks = 0; - private handleDocumentLevelKeyDown = (e) => { if (e.key === "Home") { this.goToFirstPage(); @@ -1017,6 +961,7 @@ export class BloomPlayerCore extends React.Component { if (isNewBook) { window.setTimeout(() => { this.setState({ isFinishUpForNewBookComplete: true }); + const originalStartingUpSwiper = this.startingUpSwiper; // excess caution perhaps this.startingUpSwiper = false; // transition phase is over var startPage = this.state.startPageIndex ?? 0; @@ -1031,6 +976,14 @@ export class BloomPlayerCore extends React.Component { // Now we want all that to happen. this.setIndex(startPage); this.showingPage(startPage); + if (originalStartingUpSwiper) { + // We need to instantiate any needed scrollbars on the first page displayed. + // scrollbars on later pages are instantiated by a transition event which + // doesn't fire on the very first page. + this.addScrollbarsToPageWhenReady( + this.swiperInstance.activeIndex, + ); + } //This allows a user to tab to the prev/next buttons, and also makes the focus() call work const nextButton = document.getElementsByClassName( "swiper-button-next", @@ -1747,11 +1700,8 @@ export class BloomPlayerCore extends React.Component { // ); this.showingPage(this.swiperInstance.activeIndex); } - this.msToContinueFF60RepairChecks = 30000; }, slideChangeTransitionStart: () => { - this.msToContinueFF60RepairChecks = 0; // disable - if (!this.startingUpSwiper) { // console.log( // "setting index to " + @@ -1762,23 +1712,11 @@ export class BloomPlayerCore extends React.Component { this.setIndex(this.swiperInstance.activeIndex); } }, - // Not sure we need all of these. The idea is that if slider is - // messing with offsets itself, we don't want to try to adjust things. - // During e.g. animation, it's normal that the current page isn't aligned - // with the frame. - // It appears that the last setTranslate call happens before at least one - // of the events we're using to turn checking on. - setTranslate: () => (this.msToContinueFF60RepairChecks = 0), // disable - sliderMove: () => (this.msToContinueFF60RepairChecks = 0), // disable - // This (30s) is a pretty generous allowance. Our theory is that the problem occurs - // when the activity finishes loading, typically sometime after the page before - // it becomes the 'current' page and the activity itself is therefore the 'next' - // page. Observed cases take about 2s for the problem to appear on a fast computer - // with a good internet. 30s allows for it to be a LOT slower on a phone with a poor - // connection.. We want SOME limit so - // we don't keep using power for hours if the device is left on this page. - slideChangeTransitionEnd: () => - (this.msToContinueFF60RepairChecks = 30000), + slideChangeTransitionEnd: () => { + this.addScrollbarsToPageWhenReady( + this.swiperInstance.activeIndex, + ); + }, }, keyboard: { enabled: true, @@ -2232,7 +2170,6 @@ export class BloomPlayerCore extends React.Component { this.swiperInstance.keyboard.enable(); } - addScrollbarsToPage(bloomPage); const soundItems = Array.from( bloomPage.querySelectorAll("[data-sound]"), ); @@ -2242,6 +2179,26 @@ export class BloomPlayerCore extends React.Component { }, 0); // do this on the next cycle, so we don't block scrolling and display of the next page } + private addScrollbarsToPageWhenReady(index: number): void { + if (this.state.isLoading || this.startingUpSwiper) { + //console.log("aborting showingPage because still loading"); + return; + } + const bloomPage = this.getPageAtSwiperIndex(index); + if (!bloomPage) { + // It MIGHT be a blank initial or final page placeholder, but more likely, we did a long + // scroll using the slider, so we're switching to a page that is, for the moment, + // empty due to laziness. A later render will fill it in. We want to try again then. Not sure how + // else to make sure that happens. + window.setTimeout( + () => this.addScrollbarsToPageWhenReady(index), + 50, + ); + return; // nothing more we can do until the page we want really exists. + } + addScrollbarsToPage(bloomPage); + } + // This method is attached to the pointermove and pointerup events. If the event was // caused by a mouse and is in the NiceScroll thumb area, we need to stop it from // propagating any further. This is because Swiper interprets the horizontal component diff --git a/src/scrolling.ts b/src/scrolling.ts index 5d60aa4..bc55d4d 100644 --- a/src/scrolling.ts +++ b/src/scrolling.ts @@ -68,10 +68,8 @@ export function addScrollbarsToPage(bloomPage: Element): void { scale = parseFloat(match[1]); } } - let { topAdjust, leftAdjust } = ComputeNiceScrollOffsets( - scale, - elt, - ); + let { topAdjust, leftAdjust, thumbWidth } = + ComputeNiceScrollOffsets(scale, elt); // We don't really want continuous observation, but this is an elegant // way to find out whether each child is entirely contained within its // parent. Unlike computations involving coordinates, we don't have to @@ -188,10 +186,10 @@ export function addScrollbarsToPage(bloomPage: Element): void { top: -topAdjust, left: -leftAdjust, }, - cursorwidth: "12px", + cursorwidth: thumbWidth, cursorcolor: "#000000", cursoropacitymax: 0.1, - cursorborderradius: "12px", // Make the corner more rounded than the 5px default. + cursorborderradius: thumbWidth, // Make the corner more rounded than the 5px default. }); setupSpecialMouseTrackingForNiceScroll( bloomPage, @@ -212,20 +210,89 @@ export function addScrollbarsToPage(bloomPage: Element): void { } } +// This method is copied from the nicescroll source code, albeit with some renamings and +// getting rid of jquery as much as possible. This is how nicescroll determines the parent +// element that should have the inserted scrollbar elements. If nothing is found by this +// method, nicescroll uses the body element. Since the nicescroll release hasn't been updated +// since 2017, I feel safe in copying this method here in January 2025. +function getNiceScrollParent(elt: HTMLElement): HTMLElement | null { + var parentElt = + elt && elt.parentNode ? (elt.parentNode as HTMLElement) : null; + while ( + parentElt && + parentElt.nodeType === Node.ELEMENT_NODE && + !/^BODY|HTML/.test(parentElt.nodeName) + ) { + const computed = window.getComputedStyle(parentElt); + const position = computed.getPropertyValue("position"); + if (/fixed|absolute/.test(position)) return parentElt; + const ov = + computed.getPropertyValue("overflow-y") || + computed.getPropertyValue("overflow-x") || + computed.getPropertyValue("overflow") || + ""; + if ( + /scroll|auto/.test(ov) && + parentElt.clientHeight != parentElt.scrollHeight + ) + return parentElt; + if (($(parentElt).getNiceScroll() as any).length > 0) return parentElt; + parentElt = parentElt.parentNode + ? (parentElt.parentNode as HTMLElement) + : null; + } + return null; +} + // nicescroll doesn't properly scale the padding at the top and left of the // scrollable area of the languageGroup divs when the page is scaled. This -// method computes offset values to correct for this. See BL-13796. +// method computes offset values to correct for this. See BL-13796 and BL-14112. export function ComputeNiceScrollOffsets(scale: number, elt: HTMLElement) { let topAdjust = 0; let leftAdjust = 0; + let thumbWidth = "12px"; // nicescroll calls the thumb a "cursor", but it's really a thumb if (scale !== 1) { - const compStyles = window.getComputedStyle(elt.parentElement!); - const topPadding = compStyles.getPropertyValue("padding-top") ?? "0"; - const leftPadding = compStyles.getPropertyValue("padding-left") ?? "0"; - topAdjust = parseFloat(topPadding) * (scale - 1); - leftAdjust = parseFloat(leftPadding) * (scale - 1); + const translationGroupDiv = elt.parentElement; + if ( + !translationGroupDiv || + !translationGroupDiv.classList.contains("bloom-translationGroup") + ) { + // We don't know how to deal with this case, so we'll just return the default values. + return { topAdjust, leftAdjust, thumbWidth }; + } + const whereToPutTheScrollbars = getNiceScrollParent(elt); + if (whereToPutTheScrollbars) { + // The nicescroll elements are added somewhere in the DOM that is presumably inside + // the element that sets the scaling. + const compStyles = window.getComputedStyle(translationGroupDiv); + const topPadding = + compStyles.getPropertyValue("padding-top") || "0"; + const leftPadding = + compStyles.getPropertyValue("padding-left") || "0"; + topAdjust = parseFloat(topPadding) * (scale - 1); + leftAdjust = parseFloat(leftPadding) * (scale - 1); + } else { + // The nicescroll elements are added directly under the body element, which is presumably + // outside the element that sets the scaling. We need to adjust for the scaling of the + // scrollbar position and size ourselves. See BL-14112. + const splitPageComponentInner = translationGroupDiv.parentElement; + if (!splitPageComponentInner) { + // This should never happen, but if it does, we'll just return the default values. + return { topAdjust, leftAdjust, thumbWidth }; + } + // This seems to apply only to text-only pages which don't have any additional padding + // to worry about: only the basic page dimensions given by the splitPageComponentInner + // element. + thumbWidth = `${12 * scale}px`; + const top = splitPageComponentInner?.offsetTop; + const right = + splitPageComponentInner.offsetLeft + + splitPageComponentInner.offsetWidth; + topAdjust = -(top * (scale - 1)); + leftAdjust = -(right * (scale - 1)); + } } - return { topAdjust, leftAdjust }; + return { topAdjust, leftAdjust, thumbWidth }; } export function setupSpecialMouseTrackingForNiceScroll(bloomPage: Element) { @@ -255,12 +322,11 @@ export function fixNiceScrollOffsets(page: HTMLElement, scale: number) { // The type definition is not correct for getNiceScroll; we expect it to return an array. const groupNiceScroll = $(group).getNiceScroll() as any; if (groupNiceScroll && groupNiceScroll.length > 0) { - let { topAdjust, leftAdjust } = ComputeNiceScrollOffsets( - scale, - group as HTMLElement, - ); + let { topAdjust, leftAdjust, thumbWidth } = + ComputeNiceScrollOffsets(scale, group as HTMLElement); groupNiceScroll[0].opt.railoffset.top = -topAdjust; groupNiceScroll[0].opt.railoffset.left = -leftAdjust; + groupNiceScroll[0].opt.cursorwidth = thumbWidth; groupNiceScroll[0].resize(); } },