Skip to content

Commit b8a2307

Browse files
committed
fix(helpers): tolerate sub-pixel clipping in inView
getBoundingClientRect returns fractional values while scrollTop, offsetTop and clientHeight are integers, so a container scrolled right up to its limit could still appear to clip the element by a fraction of a pixel. inView then reported the element invisible and scrollIntoViewIfNeeded fell through to elm.scrollIntoView(), needlessly scrolling the whole page (breaking e.g. Perfect Scrollbar integrations). Elements clipped by less than a pixel are now treated as visible. Reported by Ralf Pichler (Uniquare, Jodit OEM).
1 parent 5f3bc2a commit b8a2307

3 files changed

Lines changed: 84 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99
> - :house: [Internal]
1010
> - :nail_care: [Polish]
1111

12+
## 4.13.18
13+
14+
#### :bug: Bug Fix
15+
16+
- **Helpers/`inView`**: `getBoundingClientRect` returns fractional values while scroll offsets are integers, so a container scrolled right up to its limit could still appear to clip the element by a fraction of a pixel — `scrollIntoViewIfNeeded` then needlessly fell through to `elm.scrollIntoView()` and scrolled the whole page (breaking e.g. Perfect Scrollbar integrations). Elements clipped by less than a pixel are now treated as visible. Reported by Ralf Pichler (Uniquare, Jodit OEM).
17+
1218
## 4.13.17
1319

1420
#### :bug: Bug Fix

src/core/helpers/utils/scroll-into-view.test.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,67 @@ describe('Helper inView', () => {
4646
box.remove();
4747
}
4848
});
49+
50+
// Fractional rect values vs integer scroll offsets: a container scrolled
51+
// to its limit can still "clip" the element by a fraction of a pixel and
52+
// must not trigger a whole-page scrollIntoView
53+
describe('Sub-pixel clipping', () => {
54+
const makeScrollable = () => {
55+
const box = document.createElement('div');
56+
box.style.cssText =
57+
'position:fixed;left:0;top:0;width:100px;height:100px;overflow:auto;';
58+
59+
const filler = document.createElement('div');
60+
filler.style.cssText = 'height:90.4px;margin:0;';
61+
62+
const child = document.createElement('p');
63+
child.style.cssText = 'height:19px;margin:0;';
64+
65+
box.appendChild(filler);
66+
box.appendChild(child);
67+
68+
document.body.appendChild(box);
69+
return { box, child };
70+
};
71+
72+
it('Should treat an element clipped by a fraction of a pixel as visible', () => {
73+
const { box, child } = makeScrollable();
74+
try {
75+
// Integer scroll position leaves a 0.4px overhang below the box
76+
box.scrollTop = 9;
77+
expect(inView(child, box, document)).is.true;
78+
} finally {
79+
box.remove();
80+
}
81+
});
82+
83+
it('Should still treat an element clipped by a whole line as invisible', () => {
84+
const { box, child } = makeScrollable();
85+
try {
86+
// Not scrolled: the child sits ~9.4px below the box bottom
87+
expect(inView(child, box, document)).is.false;
88+
} finally {
89+
box.remove();
90+
}
91+
});
92+
93+
it('Should not fall back to scrollIntoView when scrolling the container is enough', () => {
94+
const { scrollIntoViewIfNeeded } = Jodit.modules.Helpers;
95+
const { box, child } = makeScrollable();
96+
97+
let called = false;
98+
child.scrollIntoView = () => {
99+
called = true;
100+
};
101+
102+
try {
103+
scrollIntoViewIfNeeded(child, box, document);
104+
expect(box.scrollTop).is.above(0);
105+
expect(inView(child, box, document)).is.true;
106+
expect(called).is.false;
107+
} finally {
108+
box.remove();
109+
}
110+
});
111+
});
49112
});

src/core/helpers/utils/scroll-into-view.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@ import { Dom } from 'jodit/core/dom/dom';
1515
* @module helpers/utils
1616
*/
1717

18+
/**
19+
* `getBoundingClientRect` returns fractional values while `scrollTop`,
20+
* `offsetTop` and `clientHeight` are rounded to integers, so a container
21+
* scrolled right up to its limit can still appear to clip the element by a
22+
* fraction of a pixel. Treat anything clipped by less than a pixel as
23+
* visible — otherwise `scrollIntoViewIfNeeded` falls through to
24+
* `elm.scrollIntoView()` and needlessly scrolls the whole page.
25+
*/
26+
const SUBPIXEL_TOLERANCE = 1;
27+
1828
/**
1929
* Check if element is in view
2030
*/
@@ -38,7 +48,7 @@ export function inView(
3848
// view even though its top still fits. The `top > rect.top` guard keeps
3949
// elements taller than the container (which can never fully fit) from
4050
// being treated as always out of view. See #1300
41-
if (top + height > rect.bottom && top > rect.top) {
51+
if (top + height > rect.bottom + SUBPIXEL_TOLERANCE && top > rect.top) {
4252
return false;
4353
}
4454

@@ -56,7 +66,10 @@ export function inView(
5666
const clientHeight =
5767
(doc.documentElement && doc.documentElement.clientHeight) || 0;
5868

59-
return (top + height <= clientHeight || top <= 0) && top + height >= 0;
69+
return (
70+
(top + height <= clientHeight + SUBPIXEL_TOLERANCE || top <= 0) &&
71+
top + height >= 0
72+
);
6073
}
6174

6275
/**

0 commit comments

Comments
 (0)