Skip to content

fix(textinput): correct placeholder layout constraints (px vs DIP) and no-op NaN fontSize guard#16303

Draft
FaithfulAudio wants to merge 2 commits into
microsoft:0.83-stablefrom
FacilitronWorks:fix/textinput-placeholder-layout
Draft

fix(textinput): correct placeholder layout constraints (px vs DIP) and no-op NaN fontSize guard#16303
FaithfulAudio wants to merge 2 commits into
microsoft:0.83-stablefrom
FacilitronWorks:fix/textinput-placeholder-layout

Conversation

@FaithfulAudio

@FaithfulAudio FaithfulAudio commented Jul 11, 2026

Copy link
Copy Markdown

Problem

The native placeholder renders at a different height/size than typed text; a placeholder with no explicit fontSize gets a NaN size that is never repaired.

Root cause

In CreatePlaceholderLayout(): (1) LayoutConstraints are in DIPs, but the code feeds m_imgWidth/m_imgHeight, which are physical pixels (frame * pointScaleFactor) — the placeholder is laid out in a box pointScaleFactor× too large. (2) The NaN-fontSize guard is a no-op: TextAttributes::defaultTextAttributes().fontSize; is a discarded expression (missing assignment), so NaN is never replaced.

Fix

Divide the constraint by pointScaleFactor to get DIPs; actually assign the default into textAttributes.fontSize.

Validation (honest)

Probed in a production RNW 0.83.2 new-arch app (Facilitron FIT, Windows 11 ARM64, Debug, 250% scale) with stock deep-import TextInputs and no app shims:

placeholder vs typed at 250%

Findings stated plainly: the fontSize no-op is an unambiguous code bug (discarded expression), though on 0.83.2 a no-fontSize placeholder still renders at a plausible default — so that part is correctness/robustness, not a visible-crash fix. At this probe geometry (44-DIP box, fontSize 18) placeholder and typed text measured pixel-identical glyph rows (both parked low — dominated by the GetContentSize centering bug, companion PR #), so the oversized constraint did not produce a visible offset in this configuration; the mismatch that motivated the fix was observed in product forms before we suppressed native placeholders app-wide. The DIP conversion mirrors how m_imgWidth/m_imgHeight are derived (verified in 0.83.2 source). Needs upstream CI + placeholder-vs-typed comparison at >100% scale across box/font sizes, with and without explicit placeholder fontSize. Authored against 0.83-stable; happy to re-cut onto main.

Microsoft Reviewers: Open in CodeFlow

FaithfulAudio and others added 2 commits July 11, 2026 11:58
Two defects in the native placeholder path of WindowsTextInputComponentView:

1. CreatePlaceholderLayout() built LayoutConstraints (which are in DIPs) from
   m_imgWidth/m_imgHeight, which are *physical* pixels (frame * pointScaleFactor).
   The placeholder was therefore laid out in a box pointScaleFactor x too large
   and rendered at a different height than the typed text at display scales > 100%.
   Convert the constraint back to DIPs.

2. The NaN-fontSize guard computed
   `TextAttributes::defaultTextAttributes().fontSize;` as a discarded expression
   (missing assignment), so a placeholder with no explicit fontSize kept NaN
   instead of the default. Assign it to textAttributes.fontSize.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@azure-pipelines

Copy link
Copy Markdown
Contributor
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@FaithfulAudio

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree [company="Facilitron"]

@FaithfulAudio

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree company="Facilitron"

@FaithfulAudio

Copy link
Copy Markdown
Author

Compiled into a framework source build (binary-gated) and re-probed in our production app: no regression; the NaN-fontSize guard now actually assigns (a no-fontSize placeholder renders at the default size), and placeholder-vs-typed remain pixel-comparable at this geometry. After-shot: after

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