Skip to content

Commit

Permalink
Merge pull request #348 from StephenMcConnel/BL-14112-FixJustTextPage…
Browse files Browse the repository at this point in the history 
…ScrollBars

fix: Correct scrollbar position/size for "Just Text" pages (BL-14112)
  • Loading branch information
@StephenMcConnel
StephenMcConnel authored Jan 7, 2025
2 parents 5dddda4 + 24d7b7b commit e4ea0fe
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 94 deletions.
111 changes: 34 additions & 77 deletions src/bloom-player-core.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ import { logSound } from "./videoRecordingSupport";
import { playSoundOf } from "./dragActivityRuntime";
import {
addScrollbarsToPage,
ComputeNiceScrollOffsets,
kSelectorForPotentialNiceScrollElements,
} from "./scrolling";
import { assembleStyleSheets } from "./stylesheets";
Expand Down Expand Up @@ -413,61 +412,6 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
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();
Expand Down Expand Up @@ -1017,6 +961,7 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
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;
Expand All @@ -1031,6 +976,14 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
// 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",
Expand Down Expand Up @@ -1747,11 +1700,8 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
// );
this.showingPage(this.swiperInstance.activeIndex);
}
this.msToContinueFF60RepairChecks = 30000;
},
slideChangeTransitionStart: () => {
this.msToContinueFF60RepairChecks = 0; // disable

if (!this.startingUpSwiper) {
// console.log(
// "setting index to " +
Expand All @@ -1762,23 +1712,11 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
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,
Expand Down Expand Up @@ -2232,7 +2170,6 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
this.swiperInstance.keyboard.enable();
}

addScrollbarsToPage(bloomPage);
const soundItems = Array.from(
bloomPage.querySelectorAll("[data-sound]"),
);
Expand All @@ -2242,6 +2179,26 @@ export class BloomPlayerCore extends React.Component<IProps, IPlayerState> {
}, 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
Expand Down
100 changes: 83 additions & 17 deletions src/scrolling.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
}
},
Expand Down

0 comments on commit e4ea0fe

Please sign in to comment.