Skip to content

Fix offset mapping for multi-character placeholders in SkikoParagraph - #3450

Draft
adamglin0 (adamglin0) wants to merge 1 commit into
JetBrains:jb-mainfrom
adamglin0:fix/cmp-9009-skiko-placeholder-offsets
Draft

adamglin0 (adamglin0) wants to merge 1 commit into
JetBrains:jb-mainfrom
adamglin0:fix/cmp-9009-skiko-placeholder-offsets

Conversation

@adamglin0

@adamglin0 adamglin0 (adamglin0) commented Sep 24, 2026 •

Copy link
Copy Markdown

SkikoParagraphBuilder never adds the characters covered by a placeholder to the Skia paragraph; Skia represents each placeholder as a single U+FFFC. When an inline content item uses an alternate text longer than one character (e.g. appendInlineContent(id, "[icon]")), every Skia offset after it is shifted by length - 1 relative to the Compose text. SkikoParagraph passed Compose offsets to Skia unconverted, so hit testing, cursor rects, selection paths, line start/end and word boundaries after an inline item were wrong on all Skiko platforms, and a link placed after an inline item could not be clicked.

This change introduces PlaceholderOffsetMapping, built once per paragraph from the placeholder ranges (null when every placeholder is one code unit, so the common case only pays a null check), and routes every offset that crosses the Compose/Skia boundary in SkikoParagraph through it:

  • Compose → Skia at the API boundary: getPathForRange, getCursorRect, getLineForOffset, getHorizontalPosition, getBidiRunDirection, getBoundingBox, getWordBoundary.
  • Skia → Compose on outputs: getOffsetForPosition, getLineStart, getLineEnd, getWordBoundary.
  • Internal Skia-space loops (getBoxForwardByOffset / getBoxBackwardByOffset, trailing-whitespace handling) are bounded by the Skia text length and read characters through the mapping.

Semantics: a Compose offset inside a placeholder range maps to the placeholder itself (caret before it, like Android's ReplacementSpan); a hit inside the placeholder resolves to its start or end depending on which half was hit, matching Android. The builder's single-U+FFFC contract and the public API are unchanged.

Fixes https://youtrack.jetbrains.com/issue/CMP-9009

Testing

  • DesktopParagraphPlaceholderOffsetTest (ui-text desktopTest, 12 tests): getOffsetForPosition, getCursorRect, getBoundingBox, getHorizontalPosition, getPathForRange, getWordBoundary, line APIs, accumulated deltas with two placeholders, the single-character identity path, mixed single- and multi-character placeholders, a multi-line paragraph and an RTL paragraph. All positions are derived from the fixed-advance test font and placeholderRects, not from the APIs under test.
  • BasicTextInlineContentOffsetTest (foundation skikoTest, 2 tests): the reported scenario, clicking a link placed after an inline item with a multi-character alternate text.
  • With the mapping stubbed out, 11 of the original 13 new tests fail; only the identity-path test passes.
  • :compose:ui:ui-text:desktopTest and :compose:foundation:foundation:desktopTest --tests "androidx.compose.foundation.text.*" pass.

Release Notes

Fixes - Multiple Platforms

  • Fixed wrong hit testing, cursor and selection positions after inline content whose alternate text is longer than one character (e.g. links after an inline element could not be clicked). (CMP-9009)

@google-cla

google-cla Bot commented Sep 24, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@adamglin0
adamglin0 (adamglin0) force-pushed the fix/cmp-9009-skiko-placeholder-offsets branch from 73a269f to d6dc488 Compare September 24, 2026 05:57
@adamglin0 adamglin0 (adamglin0) changed the title Fix/cmp 9009 skiko placeholder offsets Fix offset mapping for multi-character placeholders in SkikoParagraph Sep 24, 2026
SkikoParagraphBuilder never adds the characters covered by a placeholder
to the Skia paragraph; Skia represents each placeholder as a single
U+FFFC. When an inline content item uses an alternate text longer than
one character (e.g. appendInlineContent(id, "[icon]")), every Skia
offset after it is shifted by length - 1 relative to the Compose text.
SkikoParagraph passed Compose offsets to Skia unconverted, so hit
testing, cursor rects, selection paths, line start/end and word
boundaries after an inline item were wrong on all Skiko platforms, and
a link placed after an inline item could not be clicked.

Introduce PlaceholderOffsetMapping, built once per paragraph from the
placeholder ranges (null when every placeholder is one code unit, so
the common case only pays a null check), and route every offset that
crosses the Compose/Skia boundary in SkikoParagraph through it. Offsets
inside a placeholder range map to the placeholder (caret before it,
like Android's ReplacementSpan); a hit inside the placeholder resolves
to its start or end depending on the half that was hit.

Tests: DesktopParagraphPlaceholderOffsetTest (ui-text desktopTest)
covers getOffsetForPosition, getCursorRect, getBoundingBox,
getHorizontalPosition, getPathForRange, getWordBoundary, line APIs,
accumulated deltas with two placeholders, the single-character identity
path, mixed single- and multi-character placeholders, a multi-line
paragraph and an RTL paragraph.
BasicTextInlineContentOffsetTest (foundation skikoTest) reproduces the
reported scenario: clicking a link after an inline item.

Fixes https://youtrack.jetbrains.com/issue/CMP-9009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant