Repository navigation
fix(cli): recognize painted ancestor text backgrounds - #5342
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Requesting changes for one gap in the older-Chrome list. The rest looks right, and this PR complements #5338 rather than duplicating it.
How it relates to #5338. #5338 fixes the renderer: the Cloud Run and test images move to the CLI's Chrome 152, so layered gradient text stops vanishing in container renders. This PR fixes the checker. Ordinary children stop being flagged on Chrome < 150, and url() text masks are accepted on the ancestor path the same way the direct-text path already accepts them (layout-audit.browser.js:822). They touch different files, and both should land. #5338 is the one that closes #5117.
Blocking: hasIndependentTextPaint misses clip-path and view-transition-name (layout-audit.browser.js:1238). I ran a small Puppeteer probe against this branch's layout-audit.browser.js with a gradient background-clip: text heading and one block child. I counted lit pixels in a screenshot and collected the text_* codes:
| child style | Chrome 146 lit px | audit on this PR (146) | audit on main (146) |
|---|---|---|---|
| none | 7076 | clean | text_not_painted |
transform: translateY(0) |
0 | text_not_painted |
text_not_painted |
clip-path: inset(0) |
0 | clean | text_not_painted |
-webkit-clip-path: inset(0) |
0 | clean | text_not_painted |
will-change: clip-path |
0 | clean | text_not_painted |
view-transition-name: vt1 |
0 | clean | text_not_painted |
overflow: hidden |
7076 | clean | text_not_painted |
On Chrome 152 every row paints 7076 px and the audit is clean. So on older Chrome these children render invisible text, and this PR turns them from reported into silently passing. That is the false negative the PR sets out to keep reportable ("Keep missing layered text reportable on older Chrome"). Suggested fix: treat clipPath/webkitClipPath other than none, viewTransitionName other than none, and clip-path in will-change as independent paint. Add a real-browser case for at least clip-path.
Non-blocking:
- The ancestor check matches
/^\s*url\(/per layer, but the direct path usesincludes("url("), soimage-set(url(...))passes direct text and fails nested text. That is minor and could be aligned now or left.
What I ran: layout-audit.browser.test.ts 191/191; layout-audit.chromium.test.ts 51/51 on Chrome 152.0.7977.42 and on 146.0.7680.153; the probe above on 146 and 152, against this branch and against the merge-base copy of the script.
— Rames
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at f2b0cb7, which clears my earlier request for changes.
Blocker: hasIndependentTextPaint missed clip-path and view-transition-name. Resolved. layout-audit.browser.js:1251-1253 now counts clipPath, webkitClipPath and viewTransitionName other than none as independent paint, and :1278 adds clip-path to the will-change list. Older Chrome draws those children invisibly, so the audit reports them again instead of passing them. The new real-browser cases (layout-audit.chromium.test.ts:146-170) cover all four from my probe, plus a clip-path wrapper around a <span>. They also now assert that the layered descendant's frame equals an empty heading's frame, which ties each case to "really paints nothing" rather than only "differs from direct text".
Non-blocking image-set(url(...)) mismatch. Also resolved. The ancestor check now uses /url\(/i per layer (:1224). Layers are still split with splitTopLevelCommas, so image-set(a 1x, b 2x) stays one layer. Unit and Chromium cases were added for it.
What I ran at this head:
layout-audit.browser.test.ts: 197/197 pass.layout-audit.chromium.test.ts: 57/57 pass on Chrome 146.0.7680.153 and on 152.0.7977.42.- Mutation check: with the three new style entries and the
will-change: clip-pathmatch removed, 9 tests fail on Chrome 146 (4 unit, 5 Chromium). The new tests pin the fix.
— Rames
Older Chromium can paint ordinary nested text through an ancestor's
background-clip: textwhile dropping descendants that paint independently. Accept ordinary children and keep missing layered text reportable, including clip paths, prefixed clip paths,will-change: clip-path, named view transitions and independently painted wrappers.Accept ancestor URL image layers, including
image-set(url(...)), consistently with the direct-text path. Match each image to its text clip and retain transparent-gradient, wrong-layer, background-bounds, SVG and unknown-browser guards.Validation:
text_not_painted; ordinary descendants stay visible. Chrome 150 and 152 paint these cases and pass the audit.image-set()mismatch.Refs #5117 and the review follow-up on #5207. This checker change complements #5338, which updates the container renderer's Chrome version and carries the renderer fix for #5117.
As with the direct-text path, this diagnostic accepts URL images without decoding their alpha or verifying their load success.