perf(composer): make the single-row autoResize fast path reachable (long-session typing lag) (#7604)
* perf(composer): make the single-row autoResize fast path reachable autoResize() skips the height:'auto' -> scrollHeight -> restore round trip only when the composer is already at its one-row height, but the guard compared el.offsetHeight against the CSS min-height (44px) while the composer's natural one-row border box is line-height + vertical padding + borders (~48px at the stock font). 48 <= ceil(44)+1 is false, so the skip was unreachable in a real browser: every append keystroke wrote style.height twice, and the scrollHeight read that follows forces a synchronous layout of the whole document, transcript included. The cost therefore grows with the rendered transcript, which is the "typing gets laggy in long sessions" report. Accept the natural one-row height as the skip ceiling, computed from the same already-read getComputedStyle. The strict finite-pixel gate is kept for every value involved, so a percentage / calc / normal line-height, padding or min-height still fails closed to the full resize (the #6349 re-gate property is preserved, and test_issue5514_composer_grow_scroll_pin.py still pins it). Measured on a 758-message / 5 MB session (31k DOM nodes rendered) with the browser Event Timing API (input -> next paint) at 6x CPU throttle: a keystroke went from 168ms to ~136ms median echo latency (72ms with the transcript detached), 96ms to ~72ms in the windowed state, and the style writes per 8 keystrokes from 16 to 2. The #5514 invariant holds: an oversized composer still takes the full resize because its offsetHeight is far above the one-row ceiling. Tests: - new tests/test_long_session_composer_typing_latency.py runs the real autoResize() body from messages.js in a node sandbox whose getComputedStyle returns the composer's computed styles; it is red before this change (2 failed, 27 passed) and green after (29 passed). - tests/test_issue5514_composer_grow_scroll_pin.py source pins updated to the new guard shape. * test(docs): address #7604 review - behavior-only tests + composer sizing docs Review feedback on the PR flagged two repo-rule items: tests that pin source strings, and an undocumented runtime behavior change. Both are addressed here. - tests/test_long_session_composer_typing_latency.py: delete test_fix_is_pinned_in_source. It asserted exact JS declarations, which the repo testing guidance rejects and which the harness in this module already covers by executing the REAL autoResize() body. In its place add test_natural_row_skip_survives_every_append_of_a_typed_word, which drives the actual typing loop ('' -> 'h' -> ... -> 'hello') and asserts zero height writes on every keystroke. Red on the pre-fix tree, green with the fix. - tests/test_issue5514_composer_grow_scroll_pin.py: convert test_single_line_growth_skips_the_height_round_trip from source-string pins to a behavior fixture whose getComputedStyle returns the composer's real computed styles. It asserts the observable outcome: a one-row append at the natural 48px row takes no height write, while an oversized 176px composer still remeasures back to one row. Red before the fix (2 writes), green after. - docs/UIUX-GUIDE.md: document the composer sizing invariants in the composer section - the single-row skip is a hot path and must stay reachable, its ceiling is the natural one-row height or the CSS min-height (whichever is larger), everything else still remeasures, and non-px computed values fail closed. ./scripts/test.sh on the two touched modules: 29 passed (3 failed on the pre-fix tree). Neighbour sweep over the 51 composer/scroll/typing modules: 386 passed. ruff check clean. * fix(tests,docs): derive composer test dimensions from style.css (#7604 review) Greptile's re-review is right: the fixtures hardcoded the 18px `data-font-size=large` composer (line-height 29.7px -> a 47.7px one-row box) as if it were the stylesheet's 16px default (44.4px). At the default the pre-fix ceiling (ceil(44)+1 = 45px) DID clear the one-row box, so the regression is font-size dependent - the 16px default reaches the skip, the 18px and 20px appearance settings do not - and not a stock-configuration bug. The runtime change is unaffected and unchanged. - new `tests/_composer_metrics.py`: parses `static/style.css` (the base `textarea#msg` rule, the `data-font-size` overrides, the skin overrides) and exposes each configuration's box metrics - natural one-row height, the integer offsetHeight a browser reports, and both the pre-fix and post-fix ceilings - so a fixture cannot encode the wrong font size again. - `tests/test_long_session_composer_typing_latency.py`: parameterized over every configuration the stylesheet can produce, with `test_the_regression_was_font_size_dependent_not_stock` pinning the grounding. - `tests/test_issue5514_composer_grow_scroll_pin.py`: the converted single-row test uses the same derived metrics. - `docs/UIUX-GUIDE.md`: the sizing section now states that the natural row tracks the appearance font size (44 / 48 / 51px) while `min-height` stays 44px, which is why a min-height-only ceiling silently disables the skip. Pre-fix tree: `4 failed, 31 passed` - exactly the large-18px and xlarge-20px parameterizations plus the two tests that assert the reported configuration. Post-fix tree: `35 passed`. Neighbour sweep over the 51 composer/scroll/typing modules: `392 passed`. ruff clean.
This commit is contained in:
Kevin Meinon authored and GitHub committed
1 parent
03b2be0d38
commit
be5ee8f503
5 files changed
+604
-15
No files matched your search
@@ -146,6 +146,33 @@ When adding a control, consider where users will find it on both wide desktop an
|
||||
mobile. If a setting or quota/control surface does not fit in the composer, route
|
||||
it through the appropriate Control Center panel instead of squeezing the footer.
|
||||
|
||||
### Composer sizing
|
||||
|
||||
The composer grows with its content up to a 200px cap. Where the browser supports
|
||||
`field-sizing: content` (the stylesheet sets it on `textarea#msg`, with
|
||||
`field-sizing: fixed` while the placeholder shows) CSS owns that; everywhere else
|
||||
the JavaScript fallback (`autoResize()` in `static/messages.js`) measures, and it
|
||||
runs on every keystroke - so treat it as a hot path and keep these invariants when
|
||||
touching it (regression coverage:
|
||||
`tests/test_long_session_composer_typing_latency.py`, which derives every
|
||||
dimension from `static/style.css`, and
|
||||
`tests/test_issue5514_composer_grow_scroll_pin.py`):
|
||||
|
||||
- A single-row append that already fits its box skips the height round trip. That
|
||||
round trip reads `scrollHeight`, which forces a synchronous layout of the whole
|
||||
document, so its cost grows with the rendered transcript - this is the
|
||||
long-session typing-lag class. Do not remove the skip.
|
||||
- The skip's ceiling is the textarea's natural ONE-ROW height (`line-height` +
|
||||
vertical padding + borders) or the CSS `min-height`, whichever is larger.
|
||||
Compare against that natural row, never against `min-height` alone: the natural
|
||||
row follows the appearance font size (44px at the 16px default, 48px at
|
||||
`data-font-size=large`, 51px at `xlarge`), while `min-height` stays 44px, so a
|
||||
min-height-only ceiling silently disables the skip for the larger sizes.
|
||||
- Everything else still fully remeasures: an oversized composer, a replacement, a
|
||||
shrink, a multi-line append, and session/draft restore.
|
||||
- Non-pixel computed values (a percentage, `calc()`, `auto`) fail closed to the
|
||||
full resize rather than enabling the skip from a bogus pixel parse.
|
||||
|
||||
## Responsive behavior
|
||||
|
||||
Mobile is not an afterthought. The repository documents a responsive layout with
|
||||
|
||||
+22
-3
@@ -7267,9 +7267,28 @@ function autoResize(){
|
||||
// read as a bogus pixel number (parseFloat("50%")===50), which would wrongly
|
||||
// enable the fast path and leave the composer stuck tall. Reject anything that
|
||||
// is not exactly "<number>px" so those cases fail closed to the full resize.
|
||||
const _minHeightRaw=_isAppendOnly&&_fitsCurrentHeight?getComputedStyle(el).minHeight:'';
|
||||
const _minHeight=/^(?:\d+(?:\.\d+)?|\.\d+)px$/.test(_minHeightRaw)?parseFloat(_minHeightRaw):NaN;
|
||||
const _isAtMinimumHeight=Number.isFinite(_minHeight)&&el.offsetHeight<=Math.ceil(_minHeight)+1;
|
||||
const _composerStyle=_isAppendOnly&&_fitsCurrentHeight?getComputedStyle(el):null;
|
||||
const _composerPx=(raw)=>/^(?:\d+(?:\.\d+)?|\.\d+)px$/.test(raw==null?'':String(raw))?parseFloat(raw):NaN;
|
||||
const _minHeightRaw=_composerStyle?_composerStyle.minHeight:'';
|
||||
const _minHeight=_composerPx(_minHeightRaw);
|
||||
// The ONE-ROW height the composer naturally settles at is line-height +
|
||||
// vertical padding + borders (~48px at the stock font), which is TALLER than
|
||||
// the CSS min-height (44px). Comparing el.offsetHeight against min-height
|
||||
// alone therefore never matched in a real browser, so this skip was dead code
|
||||
// and EVERY append keystroke paid the height:'auto' round trip below — and
|
||||
// that scrollHeight read forces a synchronous layout of the whole document,
|
||||
// transcript included. The cost grows with the rendered transcript: on a
|
||||
// 758-message transcript (31k DOM nodes) a keystroke measured ~168ms median
|
||||
// echo latency at 6x CPU throttle vs 72ms with the transcript detached.
|
||||
// Accept the natural one-row height too; an oversized composer still takes the
|
||||
// full resize because its offsetHeight exceeds that ceiling by far.
|
||||
const _lineHeight=_composerStyle?_composerPx(_composerStyle.lineHeight):NaN;
|
||||
const _naturalRowHeight=Number.isFinite(_lineHeight)
|
||||
?_lineHeight+(_composerPx(_composerStyle.paddingTop)||0)+(_composerPx(_composerStyle.paddingBottom)||0)
|
||||
+(_composerPx(_composerStyle.borderTopWidth)||0)+(_composerPx(_composerStyle.borderBottomWidth)||0)
|
||||
:NaN;
|
||||
const _rowCeiling=Number.isFinite(_naturalRowHeight)&&Number.isFinite(_minHeight)?Math.max(_minHeight,_naturalRowHeight):_minHeight;
|
||||
const _isAtMinimumHeight=Number.isFinite(_rowCeiling)&&el.offsetHeight<=Math.ceil(_rowCeiling)+1;
|
||||
if(_isAppendOnly&&_fitsCurrentHeight&&_isAtMinimumHeight){
|
||||
_composerLastResizeValue=_nextValue;
|
||||
updateSendBtn();
|
||||
|
||||
@@ -0,0 +1,210 @@
|
||||
"""Composer box metrics derived from the REAL stylesheet.
|
||||
|
||||
``autoResize()`` in ``static/messages.js`` decides between skipping the height
|
||||
round trip and doing a full remeasure from *computed* styles: the composer's
|
||||
natural one-row border box is ``line-height + vertical padding + borders`` and it
|
||||
is compared against the CSS ``min-height`` (see the guard discussed in
|
||||
``docs/UIUX-GUIDE.md``, "Composer sizing").
|
||||
|
||||
Hardcoding those numbers in a test fixture is how the first #7604 fixture went
|
||||
wrong: it used the 18px ``data-font-size="large"`` composer (29.7px line-height,
|
||||
47.7px natural row) as if it were the stylesheet's 16px default. Parse
|
||||
``static/style.css`` here instead and derive every dimension from it, so the
|
||||
fixtures follow the stylesheet and cannot quietly encode the wrong font size.
|
||||
|
||||
Usage::
|
||||
|
||||
from tests._composer_metrics import CONFIG_DEFAULT, CONFIG_LARGE, composer_metrics
|
||||
|
||||
m = composer_metrics(CONFIG_LARGE)
|
||||
m.natural_row # 47.7: what one row of text wants (border box)
|
||||
m.offset_height # 48: what a browser reports (integer, min-height floored)
|
||||
m.old_ceiling # ceil(min-height) + 1: the pre-#7604 skip ceiling
|
||||
m.new_ceiling # ceil(max(min-height, natural row)) + 1: with the fix
|
||||
m.computed # {"lineHeight": "29.7px", ...} for a getComputedStyle stub
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import math
|
||||
import re
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
STYLE_CSS = (Path(__file__).parents[1] / "static" / "style.css").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _declarations(rules: list[str]) -> dict[str, str]:
|
||||
out: dict[str, str] = {}
|
||||
for rule in rules:
|
||||
for decl in rule.split(";"):
|
||||
if ":" not in decl:
|
||||
continue
|
||||
prop, _, value = decl.partition(":")
|
||||
out[prop.strip().lower()] = value.strip()
|
||||
return out
|
||||
|
||||
|
||||
def _rule(pattern: str, *, flags: int = re.M) -> str | None:
|
||||
m = re.search(pattern, STYLE_CSS, flags)
|
||||
return m.group(1) if m else None
|
||||
|
||||
|
||||
def _px(value: str) -> float:
|
||||
return float(value.replace("px", "").strip())
|
||||
|
||||
|
||||
def _padding_shorthand(value: str) -> tuple[float, float]:
|
||||
"""(padding-top, padding-bottom) from a CSS padding shorthand."""
|
||||
parts = value.split()
|
||||
if len(parts) == 1: # all sides
|
||||
return _px(parts[0]), _px(parts[0])
|
||||
if len(parts) == 2: # vertical horizontal
|
||||
return _px(parts[0]), _px(parts[0])
|
||||
if len(parts) == 3: # top horizontal bottom
|
||||
return _px(parts[0]), _px(parts[2])
|
||||
return _px(parts[0]), _px(parts[2])
|
||||
|
||||
|
||||
# The base composer rule: `textarea#msg{...}` (skin rules are prefixed with
|
||||
# `:root[data-skin=...]`, so anchor the match at the start of a line).
|
||||
_BASE = _declarations([_rule(r"^\s*textarea#msg\{([^}]*)\}") or ""])
|
||||
_BASE_FONT_SIZE = _px(_BASE["font-size"])
|
||||
_BASE_LINE_RATIO = float(_BASE["line-height"])
|
||||
_BASE_PAD_TOP, _BASE_PAD_BOTTOM = _padding_shorthand(_BASE["padding"])
|
||||
_MIN_HEIGHT = _px(_BASE["min-height"])
|
||||
# `border:none` / unset borders contribute nothing to the computed border box.
|
||||
_BORDER = _px(_BASE.get("border-width", "0px")) if _BASE.get("border-width") else 0.0
|
||||
|
||||
# `:root[data-font-size="large"] #msg { font-size: 18px; }` overrides, i.e. the
|
||||
# appearance setting a real user can switch on.
|
||||
_FONT_SIZE_OVERRIDES = {
|
||||
name: _px(size)
|
||||
for name, size in re.findall(
|
||||
r':root\[data-font-size="(\w+)"\]\s*#msg\s*\{\s*font-size:\s*([\d.]+px)', STYLE_CSS
|
||||
)
|
||||
}
|
||||
|
||||
# Skins that restyle the composer's font metrics, e.g.
|
||||
# `:root[data-skin="graphite"] textarea#msg{...font-size:14px;...line-height:1.45;}`.
|
||||
_SKIN_OVERRIDES = {
|
||||
name: _declarations([body])
|
||||
for name, body in re.findall(
|
||||
r':root\[data-skin="([\w-]+)"\]\s*textarea#msg\{([^}]*)\}', STYLE_CSS
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class ComposerMetrics:
|
||||
"""One composer configuration, all values derived from ``static/style.css``."""
|
||||
|
||||
name: str
|
||||
font_size: float
|
||||
line_height: float
|
||||
padding_top: float
|
||||
padding_bottom: float
|
||||
border_top: float
|
||||
border_bottom: float
|
||||
min_height: float
|
||||
|
||||
@property
|
||||
def natural_row(self) -> float:
|
||||
"""The border box one row of text wants (line-height + padding + borders)."""
|
||||
return (
|
||||
self.line_height
|
||||
+ self.padding_top
|
||||
+ self.padding_bottom
|
||||
+ self.border_top
|
||||
+ self.border_bottom
|
||||
)
|
||||
|
||||
@property
|
||||
def offset_height(self) -> int:
|
||||
"""What a browser reports for a one-row composer: an integer, min-height floored."""
|
||||
return round(max(self.natural_row, self.min_height))
|
||||
|
||||
@property
|
||||
def old_ceiling(self) -> int:
|
||||
"""The pre-#7604 skip ceiling: ``ceil(min-height) + 1`` (min-height only)."""
|
||||
return math.ceil(self.min_height) + 1
|
||||
|
||||
@property
|
||||
def new_ceiling(self) -> int:
|
||||
"""The skip ceiling with the natural one-row height considered."""
|
||||
return math.ceil(max(self.min_height, self.natural_row)) + 1
|
||||
|
||||
@property
|
||||
def skip_was_reachable(self) -> bool:
|
||||
return self.offset_height <= self.old_ceiling
|
||||
|
||||
@property
|
||||
def skip_is_reachable(self) -> bool:
|
||||
return self.offset_height <= self.new_ceiling
|
||||
|
||||
@property
|
||||
def content_two_rows(self) -> int:
|
||||
"""scrollHeight for a two-row value in the same box."""
|
||||
return round(2 * self.line_height + self.padding_top + self.padding_bottom)
|
||||
|
||||
@property
|
||||
def oversized_box(self) -> int:
|
||||
"""A composer several rows tall that must still remeasure back down."""
|
||||
return round(self.natural_row + 3 * self.line_height)
|
||||
|
||||
@property
|
||||
def computed(self) -> dict[str, str]:
|
||||
"""The ``getComputedStyle`` payload the JS guard reads."""
|
||||
|
||||
def px(value: float) -> str:
|
||||
return f"{value:g}px"
|
||||
|
||||
return {
|
||||
"minHeight": px(self.min_height),
|
||||
"lineHeight": px(self.line_height),
|
||||
"paddingTop": px(self.padding_top),
|
||||
"paddingBottom": px(self.padding_bottom),
|
||||
"borderTopWidth": px(self.border_top),
|
||||
"borderBottomWidth": px(self.border_bottom),
|
||||
"fontSize": px(self.font_size),
|
||||
}
|
||||
|
||||
|
||||
def composer_metrics(
|
||||
font_size: float | None = None,
|
||||
line_height_ratio: float | None = None,
|
||||
*,
|
||||
name: str = "custom",
|
||||
) -> ComposerMetrics:
|
||||
"""Metrics for the stylesheet's composer at ``font_size`` (default: 16px)."""
|
||||
size = _BASE_FONT_SIZE if font_size is None else font_size
|
||||
ratio = _BASE_LINE_RATIO if line_height_ratio is None else line_height_ratio
|
||||
return ComposerMetrics(
|
||||
name=name,
|
||||
font_size=size,
|
||||
line_height=size * ratio,
|
||||
padding_top=_BASE_PAD_TOP,
|
||||
padding_bottom=_BASE_PAD_BOTTOM,
|
||||
border_top=_BORDER,
|
||||
border_bottom=_BORDER,
|
||||
min_height=_MIN_HEIGHT,
|
||||
)
|
||||
|
||||
|
||||
#: The stylesheet default (16px), i.e. `textarea#msg` with no appearance override.
|
||||
CONFIG_DEFAULT = composer_metrics(name="default-16px")
|
||||
|
||||
#: The `data-font-size` appearance overrides.
|
||||
CONFIG_SMALL = composer_metrics(_FONT_SIZE_OVERRIDES.get("small"), name="small-14px")
|
||||
CONFIG_LARGE = composer_metrics(_FONT_SIZE_OVERRIDES.get("large"), name="large-18px")
|
||||
CONFIG_XLARGE = composer_metrics(_FONT_SIZE_OVERRIDES.get("xlarge"), name="xlarge-20px")
|
||||
|
||||
#: A skin that restyles the composer metrics (14px / 1.45).
|
||||
_SKIN = _SKIN_OVERRIDES.get("graphite", {})
|
||||
CONFIG_SKIN = composer_metrics(
|
||||
_px(_SKIN["font-size"]) if "font-size" in _SKIN else None,
|
||||
float(_SKIN["line-height"]) if "line-height" in _SKIN else None,
|
||||
name="skin-graphite-14px",
|
||||
)
|
||||
|
||||
#: Every composer configuration the stylesheet can produce.
|
||||
COMPOSER_CONFIGS = (CONFIG_DEFAULT, CONFIG_SMALL, CONFIG_LARGE, CONFIG_XLARGE, CONFIG_SKIN)
|
||||
@@ -35,6 +35,8 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from tests._composer_metrics import CONFIG_LARGE
|
||||
|
||||
ROOT = Path(__file__).parents[1]
|
||||
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
@@ -409,19 +411,88 @@ def test_resize_observer_installed_on_composer():
|
||||
assert "can't strand" in BOOT_JS
|
||||
|
||||
|
||||
def test_single_line_growth_skips_the_height_round_trip():
|
||||
def _run_composer_single_row_fixture(*, value: str, previous_value: str,
|
||||
offset_height: int, content_height: int | None = None,
|
||||
config=None):
|
||||
"""Run the REAL autoResize() body against a faithful textarea stub whose
|
||||
``getComputedStyle`` returns a real composer's computed styles, and report the
|
||||
observable resize behaviour (height writes, settled height, repins).
|
||||
|
||||
The configuration (and therefore every dimension) is derived from
|
||||
``static/style.css`` by ``tests/_composer_metrics.py`` instead of being typed
|
||||
in here: the composer's box depends on the appearance font-size setting, and
|
||||
hardcoding one configuration is exactly how the first revision of this test
|
||||
encoded the 18px ``data-font-size=large`` composer as "stock".
|
||||
|
||||
``content_height`` is the height the content WANTS (default: one row); a real
|
||||
textarea reports its box height in scrollHeight while the box is taller than
|
||||
the content and the content height once the box collapses to ``height:'auto'``.
|
||||
"""
|
||||
node = shutil.which("node")
|
||||
if not node: # pragma: no cover
|
||||
pytest.skip("node not available")
|
||||
config = CONFIG_LARGE if config is None else config
|
||||
if content_height is None:
|
||||
content_height = config.offset_height
|
||||
body = _autoresize_body()
|
||||
assert "let _composerLastResizeValue='';" in MESSAGES_JS
|
||||
assert "const _isAppendOnly=_nextValue.length>_composerLastResizeValue.length&&_nextValue.startsWith(_composerLastResizeValue);" in body
|
||||
assert "const _fitsCurrentHeight=el.scrollHeight<=el.offsetHeight;" in body
|
||||
assert "const _minHeightRaw=_isAppendOnly&&_fitsCurrentHeight?getComputedStyle(el).minHeight:'';" in body
|
||||
assert "const _minHeight=/^(?:\\d+(?:\\.\\d+)?|\\.\\d+)px$/.test(_minHeightRaw)?parseFloat(_minHeightRaw):NaN;" in body
|
||||
# The strict finite-pixel guard rejects a percentage/auto/calc min-height so a
|
||||
# bogus parseFloat("50%")===50 can't wrongly enable the fast path (Codex #6349 re-gate).
|
||||
assert "parseFloat(getComputedStyle(el).minHeight)" not in body # old lax parse is gone
|
||||
assert "const _isAtMinimumHeight=Number.isFinite(_minHeight)&&el.offsetHeight<=Math.ceil(_minHeight)+1;" in body
|
||||
assert "if(_isAppendOnly&&_fitsCurrentHeight&&_isAtMinimumHeight){" in body
|
||||
assert "el.style.height='auto'" in body
|
||||
harness = textwrap.dedent(
|
||||
"""
|
||||
let _composerAutoResizeRaf = 0;
|
||||
let _composerLastResizeValue = %(previous_value)r;
|
||||
let writes = 0, height = %(offset_height)s, repins = 0;
|
||||
const NATURAL_ROW = %(natural_row)s;
|
||||
const CONTENT_H = %(content_height)s;
|
||||
const msg = {
|
||||
value: %(value)r,
|
||||
get offsetHeight() { return height; },
|
||||
get scrollHeight() { return height > NATURAL_ROW ? height : CONTENT_H; },
|
||||
style: {
|
||||
set height(value) { writes += 1; height = value === 'auto' ? NATURAL_ROW : parseInt(value, 10); },
|
||||
get height() { return height + 'px'; },
|
||||
},
|
||||
};
|
||||
const messages = { scrollTop: 0 };
|
||||
const $ = (id) => id === 'msg' ? msg : id === 'messages' ? messages : null;
|
||||
const COMPUTED = %(computed)s;
|
||||
function getComputedStyle() { return COMPUTED; }
|
||||
function updateSendBtn() {}
|
||||
function _repinMessagesAfterComposerResize() { repins += 1; }
|
||||
%(autoresize)s
|
||||
autoResize();
|
||||
console.log(JSON.stringify({ writes, height, lastValue: _composerLastResizeValue, repins }));
|
||||
"""
|
||||
) % {
|
||||
"previous_value": previous_value,
|
||||
"offset_height": offset_height,
|
||||
"content_height": content_height,
|
||||
"natural_row": config.offset_height,
|
||||
"value": value,
|
||||
"computed": json.dumps(config.computed),
|
||||
"autoresize": body,
|
||||
}
|
||||
proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30)
|
||||
assert proc.returncode == 0, f"node harness failed: {proc.stderr}"
|
||||
return json.loads(proc.stdout)
|
||||
|
||||
|
||||
def test_single_line_growth_skips_the_height_round_trip():
|
||||
# The one-row fast path must be REACHABLE. It gates on el.offsetHeight against
|
||||
# the min-height ceiling, and the composer's natural one-row height
|
||||
# (line-height + vertical padding + borders) is 47.7px in the reported
|
||||
# configuration (the 18px `data-font-size=large` composer) against a pre-fix
|
||||
# ceiling of ceil(44px)+1 = 45px - so every append at rest missed the skip and
|
||||
# paid the height:'auto' round trip, whose scrollHeight read forces a
|
||||
# synchronous full-document reflow (typing lag that grows with the rendered
|
||||
# transcript). Behaviour, not source: the fixture runs the real autoResize().
|
||||
one_row = CONFIG_LARGE.offset_height
|
||||
out = _run_composer_single_row_fixture(value="a", previous_value="", offset_height=one_row)
|
||||
assert out == {"writes": 0, "height": one_row, "lastValue": "a", "repins": 0}, out
|
||||
# ...and the widened ceiling stays BOUNDED: an oversized composer is far above
|
||||
# one row, so it still remeasures back down (the #5514 shrink-back invariant).
|
||||
oversized = _run_composer_single_row_fixture(
|
||||
value="a", previous_value="", offset_height=CONFIG_LARGE.oversized_box
|
||||
)
|
||||
assert oversized == {"writes": 2, "height": one_row, "lastValue": "a", "repins": 0}, oversized
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _run(scenario):
|
||||
|
||||
@@ -0,0 +1,262 @@
|
||||
"""Regression coverage for composer typing lag in long chat sessions.
|
||||
|
||||
Symptom: in a long session, typing becomes laggy - keystrokes echo with a
|
||||
visible delay. Measured on a 758-message / 5 MB session (31k DOM nodes
|
||||
rendered), with the browser's own Event Timing API (input -> next paint) at 6x
|
||||
CPU throttle:
|
||||
|
||||
transcript rendered keystroke -> echo (median)
|
||||
full (36 rows) @6x 168ms -> 136ms with this fix
|
||||
windowed (11 rows) @6x 96ms -> 72ms with this fix
|
||||
floor (transcript detached) 72ms
|
||||
|
||||
Root cause: ``autoResize()`` in ``static/messages.js`` has a single-row fast path
|
||||
that must skip the ``height:'auto'`` -> read ``scrollHeight`` -> restore round
|
||||
trip. That round trip forces a SYNCHRONOUS layout of the whole document (the
|
||||
transcript included), so its cost grows with the rendered transcript - exactly
|
||||
the reported symptom. The guard that gates the skip compared ``el.offsetHeight``
|
||||
against the CSS ``min-height`` (44px -> a ceiling of ``ceil(44)+1 = 45px``)
|
||||
instead of the composer's natural ONE-ROW height - ``line-height + vertical
|
||||
padding + borders``.
|
||||
|
||||
Which configurations that broke is a property of the stylesheet, so the numbers
|
||||
below are DERIVED from ``static/style.css`` by ``tests/_composer_metrics.py``
|
||||
rather than typed into a fixture (an earlier revision of this module hardcoded
|
||||
the 18px numbers and mislabelled them as the stock font size):
|
||||
|
||||
composer config one-row box pre-fix ceiling skip fired?
|
||||
default 16px 44px 45px yes (0.6px margin)
|
||||
data-font-size=large (18px) 48px 45px NO - dead code
|
||||
data-font-size=xlarge (20px) 51px 45px NO - dead code
|
||||
skin graphite (14px/1.45) 44px 45px yes
|
||||
|
||||
So the fix is font-size independent: it derives the skip ceiling from the same
|
||||
computed styles the guard already reads, and therefore also covers the smaller
|
||||
configurations (which only cleared the old ceiling by 0.6px, or purely because
|
||||
``min-height`` floored the box).
|
||||
|
||||
Fix: accept the natural one-row height (computed from line-height + padding +
|
||||
borders) as the skip ceiling, failing closed to the old min-height-only
|
||||
behaviour whenever those computed values are not strict px values (so a
|
||||
percentage / ``normal`` line-height cannot wrongly enable the skip).
|
||||
|
||||
These tests exercise the REAL ``autoResize()`` body extracted from messages.js in
|
||||
a node sandbox whose ``getComputedStyle`` returns the requested composer's real
|
||||
computed styles. ``test_skip_is_reachable_in_every_composer_configuration`` is
|
||||
red on the pre-fix tree for the 18px/20px configurations and green after it.
|
||||
"""
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from tests._composer_metrics import (
|
||||
COMPOSER_CONFIGS,
|
||||
CONFIG_DEFAULT,
|
||||
CONFIG_LARGE,
|
||||
CONFIG_SKIN,
|
||||
CONFIG_SMALL,
|
||||
CONFIG_XLARGE,
|
||||
)
|
||||
|
||||
ROOT = Path(__file__).parents[1]
|
||||
MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _autoresize_body() -> str:
|
||||
start = MESSAGES_JS.find("function autoResize(")
|
||||
assert start != -1
|
||||
end = MESSAGES_JS.find("function scheduleComposerAutoResize(", start)
|
||||
assert end > start
|
||||
return MESSAGES_JS[start:end]
|
||||
|
||||
|
||||
def _run_autoresize(config, *, value: str, previous_value: str, box_height: int,
|
||||
content_height: int, computed=None):
|
||||
"""Run the real autoResize() against a faithful textarea stub.
|
||||
|
||||
``box_height`` is the composer's current offsetHeight and ``content_height``
|
||||
the height its content WANTS (one row = the config's one-row box, two rows =
|
||||
``config.content_two_rows``); the stub models a real textarea, whose
|
||||
scrollHeight is the box height while the box is taller than the content and
|
||||
the content height once the box collapses to ``height:'auto'``.
|
||||
"""
|
||||
node = shutil.which("node")
|
||||
assert node, "node is required for the autoResize harness"
|
||||
body = _autoresize_body()
|
||||
harness = textwrap.dedent(
|
||||
"""
|
||||
let _composerAutoResizeRaf = 0;
|
||||
let _composerLastResizeValue = %(previous_value)r;
|
||||
let writes = 0, height = %(box_height)s;
|
||||
const NATURAL_ROW = %(natural_row)s;
|
||||
const CONTENT_H = %(content_height)s;
|
||||
const msg = {
|
||||
value: %(value)r,
|
||||
get offsetHeight() { return height; },
|
||||
get scrollHeight() { return height > NATURAL_ROW ? height : CONTENT_H; },
|
||||
style: {
|
||||
set height(v) { writes += 1; height = v === 'auto' ? NATURAL_ROW : parseInt(v, 10); },
|
||||
get height() { return height + 'px'; },
|
||||
},
|
||||
};
|
||||
const messages = { scrollTop: 0 };
|
||||
const $ = (id) => id === 'msg' ? msg : id === 'messages' ? messages : null;
|
||||
const COMPUTED = %(computed)s;
|
||||
function getComputedStyle() { return COMPUTED; }
|
||||
let sendUpdates = 0;
|
||||
function updateSendBtn() { sendUpdates += 1; }
|
||||
function _repinMessagesAfterComposerResize() {}
|
||||
%(autoresize)s
|
||||
autoResize();
|
||||
console.log(JSON.stringify({ writes, height, lastValue: _composerLastResizeValue, sendUpdates }));
|
||||
"""
|
||||
) % {
|
||||
"previous_value": previous_value,
|
||||
"value": value,
|
||||
"box_height": box_height,
|
||||
"content_height": content_height,
|
||||
"natural_row": config.offset_height,
|
||||
"computed": json.dumps(config.computed if computed is None else computed),
|
||||
"autoresize": body,
|
||||
}
|
||||
proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30)
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
return json.loads(proc.stdout)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("config", COMPOSER_CONFIGS, ids=lambda c: c.name)
|
||||
def test_skip_is_reachable_in_every_composer_configuration(config):
|
||||
"""THE typing-lag regression: at its natural one-row height an append
|
||||
keystroke must NOT run the height round trip, in every configuration the
|
||||
stylesheet can produce.
|
||||
|
||||
Pre-fix the one-row box was compared against the 44px CSS min-height (a
|
||||
45px ceiling). That still cleared the 16px default (44px) by 0.6px, but the
|
||||
18px/20px ``data-font-size`` composers sit at 48px/51px, where the skip was
|
||||
dead code and every keystroke forced a synchronous full-document reflow.
|
||||
"""
|
||||
out = _run_autoresize(
|
||||
config, value="a", previous_value="", box_height=config.offset_height,
|
||||
content_height=config.offset_height,
|
||||
)
|
||||
assert out["writes"] == 0, f"one-row append must skip the resize round trip; got {out}"
|
||||
assert out["height"] == config.offset_height, out
|
||||
assert out["lastValue"] == "a", out
|
||||
assert out["sendUpdates"] == 1, "the primary-button refresh must still run"
|
||||
|
||||
|
||||
def test_the_regression_was_font_size_dependent_not_stock():
|
||||
"""Grounding for this fix, straight from the stylesheet.
|
||||
|
||||
This is the check the first revision of this module failed: it asserted a
|
||||
48px "stock" composer, which is the 18px ``data-font-size=large`` box. The
|
||||
real default (16px) already reached the old ceiling, so the fix is about
|
||||
making the skip font-size independent - not about a default the reporter
|
||||
never ran.
|
||||
"""
|
||||
assert CONFIG_DEFAULT.offset_height <= CONFIG_DEFAULT.old_ceiling, (
|
||||
"the stylesheet default is expected to have reached the pre-fix ceiling"
|
||||
)
|
||||
assert CONFIG_LARGE.offset_height > CONFIG_LARGE.old_ceiling
|
||||
assert CONFIG_XLARGE.offset_height > CONFIG_XLARGE.old_ceiling
|
||||
# ...and after the fix every configuration's one-row box clears the ceiling,
|
||||
# including the two that were dead and the two that were already fine.
|
||||
for config in COMPOSER_CONFIGS:
|
||||
assert config.skip_is_reachable, f"{config.name} must reach the skip after the fix"
|
||||
assert CONFIG_SMALL.offset_height == CONFIG_SMALL.min_height, (
|
||||
"the small configuration's box is floored by min-height, not by its text"
|
||||
)
|
||||
assert CONFIG_SKIN.offset_height == CONFIG_SKIN.min_height
|
||||
|
||||
|
||||
def test_typed_word_appends_skip_in_the_reported_configuration():
|
||||
"""The reported lag configuration (18px composer) must skip on EVERY append
|
||||
keystroke, not just the first: the real typing loop is '' -> 'h' -> 'he' ...
|
||||
"""
|
||||
previous = ""
|
||||
for letter in "hello":
|
||||
value = previous + letter
|
||||
out = _run_autoresize(
|
||||
CONFIG_LARGE, value=value, previous_value=previous,
|
||||
box_height=CONFIG_LARGE.offset_height,
|
||||
content_height=CONFIG_LARGE.offset_height,
|
||||
)
|
||||
assert out["writes"] == 0, f"append {value!r} must skip; got {out}"
|
||||
assert out["lastValue"] == value, out
|
||||
previous = value
|
||||
|
||||
|
||||
def test_oversized_composer_still_remeasures_to_its_natural_height():
|
||||
"""The skip must not preserve an oversized composer (the #5514 invariant).
|
||||
|
||||
A composer several rows tall holding a one-line value remeasures down to one
|
||||
row: its offsetHeight is far above the natural one-row ceiling.
|
||||
"""
|
||||
out = _run_autoresize(
|
||||
CONFIG_LARGE, value="short prefix", previous_value="short prefi",
|
||||
box_height=CONFIG_LARGE.oversized_box, content_height=CONFIG_LARGE.offset_height,
|
||||
)
|
||||
assert out["writes"] == 2, f"an oversized composer must remeasure; got {out}"
|
||||
assert out["height"] == CONFIG_LARGE.offset_height, out
|
||||
|
||||
|
||||
def test_single_line_delete_still_runs_the_height_round_trip():
|
||||
"""Shrinking values are not append-only and must remeasure."""
|
||||
out = _run_autoresize(
|
||||
CONFIG_LARGE, value="a", previous_value="hello",
|
||||
box_height=CONFIG_LARGE.oversized_box, content_height=CONFIG_LARGE.offset_height,
|
||||
)
|
||||
assert out["writes"] == 2, f"a delete must remeasure; got {out}"
|
||||
assert out["height"] == CONFIG_LARGE.offset_height, out
|
||||
|
||||
|
||||
def test_multi_line_append_still_runs_the_height_round_trip():
|
||||
"""A newline append is not a one-row append: it must measure and grow."""
|
||||
out = _run_autoresize(
|
||||
CONFIG_LARGE, value="a\nb", previous_value="a",
|
||||
box_height=CONFIG_LARGE.offset_height,
|
||||
content_height=CONFIG_LARGE.content_two_rows,
|
||||
)
|
||||
assert out["writes"] == 2, f"newline growth must remeasure; got {out}"
|
||||
assert out["height"] == CONFIG_LARGE.content_two_rows, out
|
||||
|
||||
|
||||
def test_non_px_computed_values_fail_closed_to_min_height_only():
|
||||
"""Without strict px padding/line-height values the skip falls back to the
|
||||
pre-fix min-height-only semantics, i.e. the 18px one-row box remeasures."""
|
||||
out = _run_autoresize(
|
||||
CONFIG_LARGE, value="a", previous_value="",
|
||||
box_height=CONFIG_LARGE.offset_height,
|
||||
content_height=CONFIG_LARGE.offset_height,
|
||||
computed={"minHeight": "44px"},
|
||||
)
|
||||
assert out["writes"] == 2, f"must fail closed to the full resize; got {out}"
|
||||
|
||||
|
||||
def test_percentage_line_height_fails_closed():
|
||||
"""A percentage line-height is not a strict px value and must not enable the
|
||||
skip (mirrors the #6349 min-height gate)."""
|
||||
out = _run_autoresize(
|
||||
CONFIG_LARGE, value="a", previous_value="",
|
||||
box_height=CONFIG_LARGE.offset_height,
|
||||
content_height=CONFIG_LARGE.offset_height,
|
||||
computed={**CONFIG_LARGE.computed, "lineHeight": "165%"},
|
||||
)
|
||||
assert out["writes"] == 2, f"percentage line-height must fail closed; got {out}"
|
||||
|
||||
|
||||
def test_percentage_min_height_fails_closed():
|
||||
"""A percentage min-height must not be read as a pixel number by
|
||||
``parseFloat`` (``parseFloat('50%') === 50``) - the #6349 property, still in
|
||||
force after this change: no strict px min-height means no reachable ceiling."""
|
||||
out = _run_autoresize(
|
||||
CONFIG_DEFAULT, value="a", previous_value="",
|
||||
box_height=CONFIG_DEFAULT.offset_height,
|
||||
content_height=CONFIG_DEFAULT.offset_height,
|
||||
computed={**CONFIG_DEFAULT.computed, "minHeight": "50%"},
|
||||
)
|
||||
assert out["writes"] == 2, f"percentage min-height must fail closed; got {out}"
|
||||
Reference in new issue
Block a user