Skip to content

Cap label measurement at maxWidth when TextKit fails to wrap - #6372

Closed
feather2222 wants to merge 2 commits into
signalapp:mainfrom
feather2222:fix-label-measurement-max-width
Closed

feather2222 wants to merge 2 commits into
signalapp:mainfrom
feather2222:fix-label-measurement-max-width

Conversation

@feather2222

Copy link
Copy Markdown

Contributor checklist

Administrative:

Commits and testing:

  • My commits are rebased on the latest main branch.
  • My commits are well-structured for review.
  • My change has been thoroughly tested, and I am not aware of any regressions to existing features or behaviors.
  • I have tested my contribution on these devices:
    • iPhone 16 Pro Simulator, iOS 26

Description

Fixes #6345.

Some translations of POLL_SELECT_LABEL_MULTIPLE (e.g. German) are long enough that the poll message bubble grows beyond its usual maximum width instead of the subtitle wrapping.

Root cause

CVText.measureLabel measures text with an NSTextContainer/NSLayoutManager pair and returns usedRect(for:). On affected iOS versions this measurement can fail to wrap long text, returning the unwrapped single-line width even though it exceeds the container width. Since the poll view lays out according to the measured size, the bubble stretches to the unwrapped width.

Comparing the screenshots in the issue: the English bubble is exactly at the maximum width (the poll option rows always span the full label width), while the German bubble is wider than that maximum by roughly the extra width of the German subtitle — consistent with the subtitle measurement returning an unwrapped width.

Fix

When the layout manager's measurement exceeds maxWidth, fall back to measuring with a UILabel (sizeThatFits), which wraps correctly. The previously test-only measureLabelUsingView helper is now compiled in all configurations for this purpose. Normal measurements are unaffected.

Testing

  • Added CVTextTest/testMeasureLabelWrapsLongText, which measures the German poll subtitle at a narrow width and asserts the result wraps instead of exceeding maxWidth. It passes on iPhone 16 Pro Simulator (Xcode 26.2).
  • Note: the measurement failure itself does not reproduce on this simulator/OS version, so the fallback path could not be exercised end-to-end; the test guards the contract (measured width never exceeds maxWidth).

feather2222 added 2 commits October 19, 2026 09:23
Custom notification sounds imported by the user could never be
removed: there was no way to delete a bogus sound, or to re-import a
sound with the same name.

Long-pressing a custom sound in the sound picker now shows a 'Delete
Sound' context menu action. After confirmation, the sound file is
deleted and any references to it (the global notification sound and
per-thread overrides) are cleared so they fall back to the default.

Fixes signalapp#6351
On some iOS versions, NSLayoutManager.usedRect(for:) can fail to wrap
long text and return a width larger than the text container's. Labels
measured this way (e.g. the poll subtitle, whose German translation is
fairly long) then stretch message bubbles beyond their maximum width
instead of wrapping.

Fall back to measuring with a UILabel, which wraps correctly, whenever
the layout manager's measurement exceeds maxWidth.

Fixes signalapp#6345
@sashaweiss-signal

Copy link
Copy Markdown
Contributor

Hi, thanks for your interest. It looks like this branch includes a commit from another PR; it'd be helpful if you could rebase that commit out.

@sashaweiss-signal

Copy link
Copy Markdown
Contributor

Also, we believe this is fixed as of version 8.31 in c91f347, and I neglected to close the linked issue. Before reviewing, I'd like to confirm the issue still exists.

@sashaweiss-signal

Copy link
Copy Markdown
Contributor

Note: the measurement failure itself does not reproduce on this simulator/OS version, so the fallback path could not be exercised end-to-end; the test guards the contract (measured width never exceeds maxWidth).

Ah. I'm going to close this; in the future, please ensure you are able to reproduce an issue before and verify your fix after making changes. We unfortunately can't accept speculative changes from external contributors, as it significantly increases our maintenance burden.

@feather2222

Copy link
Copy Markdown
Author

Thanks for the context, and apologies for the noise — I should have verified the issue still reproduces on latest main before opening the PR, and kept the branch clean of unrelated commits. Understood on both points; I'll make sure any future PR reproduces the issue first and is verified after the change.

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.

Some translations of "POLL_SELECT_LABEL_MULTIPLE" are too long causing the poll message to overflow

2 participants