Repository navigation
fix(ios): clear a secure field with deletes before the caret tap - #4350
Conversation
A fill on an empty secure field still ran the clear, whose caret tap lands at the field's right edge. A show/hide control drawn there took the tap, the app swapped the input, and the fill failed with TEXT_INPUT_NOT_FOCUSED (e2e-inc/e2e#1631). A masked value that shows nothing, or a placeholder that is not a mask, is no longer cleared. Any other masked value gets deletes first, after the readiness repeat tap at the field's center when its focus is unconfirmed, since deletes into a field without focus record an XCTest failure. iOS clears a secure field on its first delete after it takes focus, and deletes from a caret at the end reach the rest, so the caret tap runs only when the read-back does not show the field empty. A placeholder that is itself a mask reads the same as text of its length, so it is not taken as empty: the deletes run and read back. Fixture tests put a toggle over the field's right edge. They cover a fill over prefilled text in the masked field and in a native secure field, a secure field that refused the focus tap, a refill of a focused field whose text reads like its bullet placeholder (no extra tap), and the first fill of an empty secure field, both bare (no deletes sent) and showing a bullet placeholder as its value, as React Native does.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
With both the masked-value and secure-entry fixture flags, the masked field wins, and the count label setup sent its setter to a field that has none. The setup now uses the secure-entry field it built.
The masked-value clear applies to any input whose value is a mask, not only secure fields. A masked input showing its placeholder is not cleared, except a secure field whose placeholder is bullets.
A replacement skipped the clear whenever the value read as the placeholder, so a plain masked field holding text as long as its bullet placeholder kept the stale text and the fill appended to it. On iOS a bullet-only value is no longer proof of an empty field, even when it equals the placeholder: such a field goes to the masked clear, which deletes and reads it back. Readability and verification reads are unchanged.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
An empty field with a bullet placeholder shows the same bullets as the first fill's text, so waiting for them proved nothing. The masked fixtures now show the field's text length, and the refill tests wait for it before refilling.
After the deletes, the edge caret tap is also skipped when the field reads back as a masked placeholder, not only when it reads empty.
|
Thanks for the careful fix. I found no code problems at fa2b299, but two things need settling before this is ready: the live evidence and one question on size. There are no conflicts. The one reported check does not run the runner XCUITests that cover this Swift change. The React Native run (21 steps, iOS 27.2/26.5) is described in the PR body but has no log. Please attach the fill command, device and OS, head SHA, and the runner lines Could the change be smaller? Of the 352 lines, about 99 are fixture ObjC and 162 are XCUITests, so the runner logic is roughly 85 lines. Would moving the mask-before-placeholder check into These comments no longer apply and can be resolved: placeholder skipping the clear, nil secure field, docs wording, earlier fill wait, caret tap docs. Not blocking:
|
The masked clear returned for an empty field before its focus check, so an empty secure field whose focus tap was refused was typed into without focus, which records an XCTest failure. The old edge tap had focused it. From review: the clear's mask-placeholder rule now applies on every platform, the read-back has three cases, an input that stops answering after the deletes gets no caret tap (its one burst, as for callstack#3060), and a test covers the caret-tap fallback for masked text the deletes can't reach. The rule stays out of textValueRead. There it turns a formatted read-back in a field that reports a bullet placeholder, such as a PIN field, from unconfirmed into TEXT_ENTRY_MISMATCH.
Masked text the deletes cannot reach that shows as many bullets as the placeholder reads as cleared, so fill types the new text next to it.
The masked clear logged its route only after the deletes, so a skipped empty field left no line in runner.log.
|
Thanks for the review. Pushed four commits; the head is Live run. iPhone 17 Pro simulator, iOS 26.5, agent-device
Steps and runner linesMoving the check into But the shared read also covers readable fields that report a bullet placeholder as their value while empty, such as a code field with a So I kept the rule in Not blocking:
Also fixed:
Checks:
The five threads you listed were already resolved. |
|
I found no code problems at 5690320. The earlier evidence request is now satisfied, and the earlier review's questions are resolved. The one CI check passes. It does not run the runner XCUITests that cover the changed Swift path, so the author's local XCUITest runs and the live React Native log cover that path. I did not run the runner XCUITests, the mutation set or check:affected. I confirmed by reading the code that the new tests would fail on the pre-change order at fa2b299. A few gaps remain, and none of them needs a change here. Removing the iOS guard changes macOS, visionOS and tvOS: they now clear an empty mask-placeholder field instead of skipping it. No fixture or live run covers that, and I did not verify the build claim. No test covers the case where the read after the deletes finds no input (#3060, mid-clear). The author says that branch is unchanged since fa2b299, and the diff agrees. The live run covers React Native only, so WebKit and Flutter secure fields that read "" while holding text are unchecked and would skip the clear. The review threads on the mask-placeholder read (#4350 (comment)), the counts label (#4350 (comment)), the commands.md wording (#4350 (comment)), the prefill wait (#4350 (comment)) and the documented read-back limit (#4350 (comment)) are all fixed at this commit. They no longer apply, so please resolve them. I know of no conflicts. Before merge, a maintainer needs to approve the CI run for this commit, which is waiting on that approval. |
thymikee
added
the
ready-for-human
Summary
On iOS,
fillalways cleared a secure field, whose value is unreadable even when empty. The clear starts with a caret tap 4 pt inside the right edge, where apps often draw a show/hide button. In e2e-inc/e2e#1631 that tap pressed the button, the app swapped the input, and the fill failed withTEXT_INPUT_NOT_FOCUSED.Now a masked field with unconfirmed focus gets the readiness retap at its center, and isn't cleared when it shows nothing or a placeholder that isn't a mask. A bullet placeholder isn't taken as empty: text of its length reads the same. Otherwise the clear sends deletes and reads back, and the edge tap runs only if masked text remains. The rule stays out of the shared
textValueRead, where it would make formatted fills into bullet-placeholder fields mismatch.9 files: runner, fixture, tests, docs, CLI help.
Validation
093341a60, head569032096.test-experiments.md): 14 mutants, all killed.check:affected --run --head 569032096: 14 of 14 pass.