Phone number verification: the code field, and the hint that fills it - #5642
Phone number verification: the code field, and the hint that fills it#5642shai-almog wants to merge 30 commits into
Conversation
A phone number is verified by sending a code to it and asking the user to type the code back. The typing is the part that has always been worse than it needs to be, because both platforms will hand the code over -- iOS offers it above the keyboard, Android's autofill offers it on the field -- to a field that says it holds a one-time code, and Codename One had no way to say that. TextArea.ONE_TIME_CODE is that statement. It carries no behaviour of its own and costs nothing on a platform that cannot offer a code; what it buys is the alternative it replaces, which is reading the SMS, which on Android means holding READ_SMS -- a permission Google Play restricts to messaging apps. OtpField is rebuilt around it. The boxes used to be one editor each, and that is the one shape a code cannot arrive in: a code is a single value, both platforms enforce maxSize as a hard native length filter, and a six character code offered to a one character field is truncated to its first digit. So the boxes became drawings and one field behind them holds the whole code. A code offered by the platform, a paste and a keystroke now all arrive the same way, through one editing session that never tears down between digits. That field is an EditField rather than a TextField, which is what makes the boxes possible at all: the pure editor renders its own text, so the component can draw six boxes where a native peer would have drawn one control over them. It draws nothing but the caret. PhoneNumberField and PhoneVerification are the rest of the screen. The country table is generated from libphonenumber's region metadata rather than written by hand, and names are looked up as "Country.<ISO>" in the theme's bundle before falling back to English. PhoneVerification owns the two stages and the things that are always rewritten with them: the wait before a resend is offered, the way back to a mistyped number, and a response that arrives after the user has moved on, which is dropped rather than applied to a screen it no longer owns. Nothing here sends a message. The service that does is the application's, and so is the check that a code is correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The constraint is only worth setting if the ports turn it into the platform's own offer, and each platform expresses it differently. iOS in two places, because a Codename One field is edited by one of two native paths. The overlay editor a TextField gets sets textContentType on its UITextField and UITextView, after the base type rather than before it, so a numeric code field is still declared a code. The pure editor -- which is the one OtpField uses -- has no UITextField to set it on, so CN1TextInputView grows the trait itself and clears it when a session ends, because the view outlives one session and a stale content type would follow the caret into the next field. Declared as NSString rather than UITextContentType so the header carries no availability of its own; the value is what is guarded. Android has no trait to set. Its autofill framework asks a View, and the pure editor's view is the rendering surface, so the surface claims the hint while a code field holds the session and drops it afterwards -- and answers autofill() by replacing the field's contents, because the platform is saying what the value IS rather than typing into what is there. The overlay editor, having a real EditText, just carries the hint on it. AUTOFILL_HINT_SMS_OTP is spelled out because the constant is newer than the SDK this port compiles against; the string is the contract. The browser has had a token for this for years: autocomplete="one-time-code", which is also what puts the code in iOS Safari's suggestion bar. It goes ahead of every other case including SENSITIVE, because a code is exactly the value a browser should offer and worthless to a dictionary afterwards. Windows, Linux and macOS ignore the flag, which is the whole point of a hint: there is no code arriving on those platforms to offer. ApplyAutofilledText is a named class rather than an anonymous one. An anonymous class in AndroidImplementation takes a number from the same sequence as every other one in the file, so adding one renumbers the ones below it and the cast-semantics baseline stops matching methods nobody touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
64 tests against the test implementation cover what can be asserted without a rasterizer: the country table and the E.164 parse, the two stages and the one-shot response, and the paths a code takes into the field -- typed one character at a time, pasted whole, or offered whole by the platform, which all arrive through the same commit. The rendering test is in maven/javase instead, against the real port and a real rasterizer, because the thing worth checking cannot be seen from core: the value is drawn by six boxes while the field that owns it draws nothing, and against an implementation whose graphics draw nothing at all that is indistinguishable from a component that renders nothing. So it measures ink -- in every box, and more of it in a filled field than an empty one. Two assertions in the old test file are gone because they described the old shape: the boxes were the editors, so a test could type by calling setText on one. They now display the value rather than holding it, and the test types through the field. The country table assertions caught a wrong assumption while they were being written: +1242 is not the Bahamas' calling code. Assigned country codes are prefix-free -- 242 is a North American area code, part of the national number -- and there is now a test asserting that property of the table, since the parse leans on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both native themes style the new UIIDs. The code boxes derive from TextField so they read as a row of slots rather than bare digits, and the dark overrides are colours only -- a dark override that redeclares 'border' without repeating the radius is what NativeThemeLightDarkConsistencyTest exists to catch. The .res files are regenerated from the CSS rather than hand-edited; the sync workflow rebuilds them on master anyway, but a PR whose CSS and .res disagree would test the old theme. The guide section lives in Authentication and Identity rather than a chapter of its own, because verifying a number is a way of signing somebody in. It is explicit about the division of labour -- the application's server sends the message and judges the code -- and about the permission the hint saves, since that is the reason to reach for it rather than for an SMS reader. The agent skill gets the same, shorter: an agent writing a verification screen in a generated project should reach for OtpField and know that reading the SMS itself is the wrong answer. The sample runs the whole flow against a fake server that accepts any number and one code, so the client half can be driven without a backend. On a device it is also how the platform's offer gets exercised: send yourself a code while its second stage is showing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The feature was documented as a section inside Authentication and Identity, which is where a reader who already knows it exists would look and nowhere a reader who does not would. Biometric Authentication has its own chapter for the same reason, and the material outgrew a subsection anyway. The chapter says the thing the API cannot: who does what. Codename One does not send the message and does not judge a code, and a reader who misses that will look for the send call. It also gives the per-platform table for the one-time-code hint, the permission the hint saves and why that matters on Android, the parts of the flow that are otherwise rewritten by hand on every verification screen -- the disabled button, the ignored second answer, the dropped superseded one -- and the server-side rules that no client can enforce: rate limit sends, expire codes, never return one. Two limits are stated rather than left to be discovered: the number field does not strip a national trunk prefix, because a leading zero is a prefix in one country and part of the number in another, and isValid checks a shape rather than existence, which is all a client honestly can. Seven snippets, all compiled by the demos module like every other Java block in the guide. Authentication and Identity keeps a short pointer, since verifying a number is still a way of signing somebody in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The iOS port builds with CLANG_ENABLE_OBJC_ARC=NO, so a copy property's backing ivar is the class's to release, and this one was the only object ivar in the class that dealloc did not name. In practice the view is a singleton holding a UIKit constant string, so nothing measurable leaked. It is still the rule the rest of that dealloc follows, and the next object property added beside it would have inherited the omission. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeea388bcc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Native fidelity (Android, Material 3)54 pairs compared -- median 95.6%, worst 91.3% ( Distribution --
Geometry vs native (bbox offset / size ratio / center offset / corner radius) -- gated separately from the visual score
Side-by-side comparisons (worst first)
|
All three are the same shape -- state belonging to a request or a field that survives past the thing it belonged to. A pending request survived leaving the code stage. The generation counter only moved when a new request started, so backing out to fix the number left the old one valid: a verification that came back a second later fired verified for a screen the user had walked away from, and a resend that came back dragged them into a code stage they had just abandoned. Both are now reproduced by tests that fail without the change. A stage transition retires whatever is still out, which is what the flow already claimed to do and only did for the case where the user sent again. An autofilled value bypassed the field's own filter. It went in through replaceRange, which is a raw document edit, rather than commitText, which is what a platform uses to say "the user entered this" and is the path a field filters. An autofill service that keeps the message's separators would hand over "123-456", and an OTP field would hold a seven character value it would never have let anyone type -- never reaching the length that completes it, so automatic verification never fired. Selecting the field first keeps the replace semantics that were the point of using replaceRange. The one-time-code hint outlived its field on Android. Tapping from a code field straight into another field reuses the same EditText through switchToTextArea, and the hint was only ever set, never cleared, so the next arriving code could be offered on whatever the user tapped into. It is now cleared as deliberately as it is set -- restoring AUTO rather than NO, because an ordinary field IS autofillable and turning that off would stop a password manager filling the username and password fields that are the whole point of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Cloudflare Preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26aeddc22d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Three separate mistakes, each now covered by a test that fails without the fix. The caret was drawn at the boxes' absolute coordinates. Painting runs with the ancestors' offsets already applied to the Graphics -- which is why every component here draws at its own getX() -- so those offsets were counted twice and the caret landed outside the box, and outside the clip, for any field that was not at the origin. It was invisible in the only place it had been looked at: a field at the top of a bare form, where the translation is zero. The rendering test now puts the field under a heading inside a padded container and measures ink inside the active box, which is zero before the fix and a caret after it. Composed text bypassed the field's filter entirely. An IME, handwriting and dictation all build text as a composition, and a composition writes to the document directly; so does the commit that finalizes one. A numeric code field could therefore hold letters, or more characters than it has boxes, which shows up as a code that can never be complete and a verification that never fires. Both doors now go through the same limit as typing, with the composed range accounted for so the room is measured against what the text replaces. The browser was given a numeric keypad for codes that are not numeric. OtpField(length, false) exists for alphanumeric codes, and inputmode="numeric" gives a mobile browser a keypad with no route to a letter -- not awkward but impossible. The hint says what the value IS; the constraint beside it says how it is typed, and only NUMERIC gets to choose the keyboard. iOS and Android were already right about this: they key the number pad off the base constraint, and only the browser inferred one from the hint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5e608aa09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The class documentation showed 050-123-4567 producing "+972501234567". It does not: the leading zero is a digit, so it is kept, and the number is "+9720501234567". The paragraph immediately below said as much -- that the field does not strip a national trunk prefix -- so the example and the prose disagreed, and the example is the half people copy. The same wrong number was in the guide's snippet. Both now show the two cases separately: separators dropped, trunk prefix kept, and what the second one actually returns. A test pins it, because a documented behaviour with no test is how the example drifted from the code in the first place. The behaviour itself stands. Stripping a leading zero would normalize Israel by corrupting Italy, where it is part of the number, and the per-country rule that tells them apart is metadata this field deliberately does not carry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30ce24af0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Hit testing was inherited, and what it measures is the field's own text laid out from its left edge -- metrics for a rendering that exists nowhere on the screen, because the boxes are the rendering and the layer holding the text draws nothing but the caret. With the boxes centred, every tap fell to the right of that phantom layout and answered with the end of the code, so tapping the third box to fix a digit put the caret after the sixth and the correction landed on the wrong one. A test walks all six boxes; before the change the first already answered 6. The offset is clamped to the text, which the review did not ask for and the caller needs: pointerPressed assigns this straight to the caret without clamping, so a tap on an empty box would otherwise put the caret past the end of the document. Clamped, it means what a user means by tapping an empty box -- carry on where I left off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edfa0bc976
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both halves of the autofill bridge talk to the editor from Android's UI thread while the editor belongs to the EDT, and neither said so. A filled value was captured on the UI thread and applied a hop later on the EDT, with nothing checking in between that the session it was filled into still existed. Between those two moments the user can move to another field or leave the screen, and the value would then be written into a field nothing is bound to and fire its listeners -- an OtpField's completion listener submits a code, so a late fill could verify a code for a flow already left. The rest of this bridge re-checks the bound client in exactly this way; this callback now does too. Reading the field's value for an autofill query walked the live document: a length, then a range out of it. Two reads of something another thread is editing. Clamping does not help, because the buffer underneath can be restructured between them -- the failure is a crash inside somebody else's autofill query, which is a poor place to have one. It now answers from the immutable state snapshot the bridge already maintains for cross-thread reads. A value one edit out of date is the right trade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0f2744136
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…own field The review asked for a hit test that copes with the boxes being reversed on a right-to-left form. They should not be reversed. A code is digits, digits read left to right in every locale, and the platforms' own code fields do not mirror them -- BoxLayout reverses its children when its parent is RTL, so on a Hebrew or Arabic form the first digit was drawn on the right and the whole code read backwards. Teaching the hit test to follow that would have made taps agree with a display that was already wrong. The row opts out of the mirroring instead, which fixes what is drawn and leaves the caret and the hit test able to take the boxes in order. A test on an RTL look and feel now pins both, and fails on the box order alone without the change. The flag is re-applied after initLaf because that is where it is assigned; setting it in the constructor would not survive being added to a form. The autofill guard needed a second half. Checking that the session is still the one that was captured is not enough, because the hint that invited the fill lives on the surface view and is applied and cleared on Android's UI thread while the session it describes changes on the EDT. For a moment after the user moves from a code field to an ordinary one, the view still advertises smsOTPCode over a session that is something else, and a fill delivered in that window would land a code in whatever they tapped into. Both the entry point and the queued application now also ask what the current session advertises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65bb2d9baf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…tion A code is not language, and the keyboard was treating it as one. TextInputConfig enables correction and capitalization by default and EditField passes them straight through, so an alphanumeric code field got both on every port that honours the config. The platform applies them BEFORE the value reaches the field, so accept() never sees the original: the user types the code they were sent, the keyboard capitalizes its first letter, and the server rejects a code that was right when it left their hands. Off now for the code field, and off in the iOS overlay editor for any field carrying the constraint, which is what the documentation tells people to do to their own fields. A digit code was always safe from this by virtue of its keypad; one that takes letters was not. setCountries(null) left a country selected that the restored list does not carry. The review asked for the same treatment the non-null case gets, which selects the first entry -- of the full table that is Afghanistan, and silently moving somebody from +1 to +93 is worse than the inconsistency it tidies. The same ISO code is preferred where the full list has it, so the ordinary case keeps its selection, and the fallback is the device's own country, which is where the field started. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ed8a82fa1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Countries are equal when their ISO codes are, so a replacement list can hold a different object for the same country -- the built-in United States beside an application's own. Checking only that something matched left the old object selected, so the field would dial a code the selector no longer offered. It now adopts the entry from the list, which is what the null branch already did after the previous round; the two halves of this method disagreed for one commit. Where nothing matches, the first entry of the list the application supplied stays the fallback. That is a defensible default in a way the first entry of the full table is not, which is why only this branch takes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c9f8a8164
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50c64394fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…form
String.toUpperCase folds with the device's locale. In Turkish "i" becomes a
dotted capital I, so findCountry("il") looked for "İL" and the table has "IL":
the documented case-insensitive lookup returned null for every ISO code
containing an i, on every device set to Turkish. Verified rather than reasoned
about -- under tr_TR the JDK really does answer "İL", and equalsIgnoreCase
really does still match, because it compares character by character and that
mapping carries no locale.
Both lookups now use it, and picking the country from the device locale takes
the tail after the separator instead of matching suffixes on a folded string,
which is what made the old one need the fold at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6291ba619f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… what case means The same Turkish fold, one layer further in, and I had talked myself out of this one an hour ago on the grounds that both sides of the comparison went through the same toLowerCase. They do not start from the same case: the capital I of "Israel" folds to a dotless i while the i the user types is already lowercase and stays dotted, so on a Turkish device the country could not be found by typing its first letter. Measured this time -- "Israel".toLowerCase() really is "israel" here and "isrsel" with a dotless i there. Folding character by character uses the Unicode mapping, which is the same everywhere. An ASCII-only fold would also have worked for Israel and would have stopped matching Aland Islands by its own first letter, which is the sort of fix that trades one locale's problem for another's. The predicate is now a named method so it can be tested without opening a dialog, which is why the search had no test until now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1946a333f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ill writing An input method builds text before committing it, and the boxes were showing it as it arrived -- which is right -- but the completion listener fired on it too, which is not. A composition that passes through six characters submitted a code the IME was still editing; PhoneVerification is busy from that moment, so the corrected code was ignored until the premature attempt came back and failed. Completion now waits for the value to be final: a commit, or a composition finished without one. Both are handled, because finishComposing changes no text and nothing else would tell the field to look again. The provisional flag is this field's own rather than the editor's composing range. The range says what the platform last marked, and a commit does not necessarily clear it; what this needs to know is whether the value has been finalized, which is a different question with a different answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9964576128
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The review asked for Pitcairn to be added, on the grounds that it uses +64 and a device reporting PN therefore falls back to the wrong country. The premise is right and the remedy is not, so the reasoning now lives beside the table where the next reader will find it. The table is generated from libphonenumber's region metadata, and that is the point: two hundred and forty-five calling codes typed out by hand would contain mistakes nobody would find until somebody in that country could not sign in. Eight ISO 3166 codes have no metadata there -- AN, AQ, BV, GS, HM, PN, TF, UM. One stopped being a country in 2010, most of the rest have no permanent population, and none has a numbering plan of its own; Pitcairn's numbers are reachable through New Zealand's +64 rather than through anything assigned to PN. Adding the one that got noticed, with a code invented for it, while leaving the seven that did not, would trade a documented boundary for an undocumented guess. What the review is right about is that the fallback is arbitrary, so it now says so, and says what an application serving such a place should do instead: pass its own list, which setCountries exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7806908e4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…with a test The review reported that a commit leaves the editor's composing range set, so a second committed fragment would replace the first and a code delivered in fragments could not be entered. The mechanism it describes would be serious, and the reasoning is sound from reading commitText alone, where nothing clears the range in view. It is not what happens. The commit replaces the composed range through the ordinary document-change path, and that path clears the composition; measured rather than argued -- after a commit the range reads -1,-1 and a second commit appends. I had started writing the fix before checking, which would have added redundant clearing to shared editor code that every pure editor runs. What the report deserves is the evidence, so EditorViewInputTest now asserts that a second commit follows the first, and the comment at the call site says where to look. The same correction applies to a claim I made a few commits ago justifying this field's provisional flag: the flag is right, but not for the reason I gave. It records what the delivery was, not what the document says. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c39e252c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Everything typed into the number field was treated as a national number, so a
value that already had a calling code got a second one. Pasting +972501234567
with Israel selected produced +972972501234567, and the field invites exactly
that: it carries the phone constraint, which is what makes the platform offer
the device's own number -- in international form.
The same assumption reached setE164. Given a number no offered country can
express -- a narrowed list, or a code outside the table -- it stored the whole
international value as if it were national, so setE164("+447911123456") with
only the United States offered read back as +1447911123456: a different number,
and a plausible looking one.
Both are the same rule now. A value that says which country it is for is used
as it stands and the selector is not applied to it; the selector applies to a
national number, which is what it is for. The national part of an international
value is what follows its own calling code, and isValid measures what getE164
would actually send rather than what the selector would have contributed.
The three numbers above are the three assertions, and each of them fails
without the change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six index loops that only ever used the element, and an import left behind when the digit row stopped being a box layout. Both are on the forbidden list, so build-test (8) failed on them; it had not reached that step before, because every earlier run of it was cancelled by the next push. Run locally this time rather than pushed and watched: the whole report now has no forbidden violation, SpotBugs none, Checkstyle none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 200724859b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…r contains The autofill value was read before the checks that validate it. The client, the config and the state are three fields assigned separately on the EDT, so taking the state first and validating afterwards can pair one field's text with the next field's configuration -- and the pairing that matters is a password field's text with a code field's hint. The state is now read after the guards, and the client re-checked after the state, so nothing is returned for a session other than the one that was checked. The review proposed a single session snapshot, which would be a better shape for the whole input bridge and a larger change than this method has any business making; the property needed here is narrower. setCountry accepted anything, so an application that had narrowed the picker could still select a country the picker does not list -- a selector showing one country over a list containing another, and a calling code the user was never offered. It refuses now, loudly, because it is a mistake worth hearing about where it is made. It refuses rather than substituting the list's own entry, which is what the review suggested. Countries are equal by ISO code, so substituting would silently throw away an application's own Country -- a name of its own, or a calling code it had reason to override -- for a case that is a programming error either way. Both halves have a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35a3516ed3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s the field A commit replaces the composed range in preference to the selection, so selecting the whole field is not enough to replace the whole field while an input method is mid-word. A code filled in at that moment landed inside the composition and kept whatever surrounded it: with "99" typed and "12" being composed, accepting "123456" produced "991234" -- the right length, the wrong code, and long enough to submit itself. That is the number the test asserts, and it is what comes out without the change. Ending the composition first makes the commit take the selection branch, which is the whole field, which is what a filled value means. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Compared 157 screenshots: 157 matched. Benchmark Results
Detailed Performance Metrics
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5262b7f10f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The documented way to mark a field as holding a code is NUMERIC | ONE_TIME_CODE, and two consumers compared the whole constraint rather than its base type, so the hint beside NUMERIC turned a digits-only field back into one that took anything: TextField.validChar in lightweight editing, and CodenameOneView's input type on the legacy Android view, which stripped PASSWORD by hand and let every other modifier fall through to the text keyboard. This is older than the hint -- PASSWORD, SENSITIVE, NON_PREDICTIVE, USERNAME and UPPERCASE all defeated those two comparisons the same way, and stripping one of six by hand shows what was meant. Both now mask the base type, as every other consumer of a constraint in the framework already does. That does change behaviour for combinations that predate this branch: a NUMERIC | PASSWORD field now takes digits in lightweight editing, where it used to take letters. That is the constraint being honoured rather than a new rule. All 5807 core tests pass. And a number cannot start with a zero after the plus: E.164 reserves that digit, so no calling code begins with it. Both the flow's plausibility check and the field's validity check refused to notice. A leading zero in a NATIONAL number is a trunk prefix, which the field keeps on purpose, so the rule applies only to the international form -- and the test says both halves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2c69fce19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The review asked for the pending generation to be retired on deinitialization, by the same argument that carried for going back to the number stage. It does not carry, and the difference is worth writing down because the evidence looks the same from a distance. Going back is the user saying they are done waiting for that answer. Deinitialization says nothing of the kind: it happens whenever the component stops being displayed, including when an application shows a progress screen over the wait -- a reasonable thing to do, and one that would then never hear the result. Silently losing a verification the server did give is worse than the two things dropping it would prevent, and both of those are mild. A detached component moving to its code stage is the state it should be in when it is shown again, and a listener firing late belongs to the application, which can remove it if it has moved on. The reasoning sits beside deinitialize and a test pins the behaviour, so the same evidence read again does not quietly reverse it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eb4fea20a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Caught by CI rather than locally because the gate compares a range of commits and the file was still untracked when I ran it -- so it was checked after being committed, which is the only order that means anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1442bca00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…o yet iOS delivers an edit through UITextInput as a range replacement, and the bridge calls the client's replaceRange directly -- reaching neither the typed-text hook nor the commit, both of which this field filters. So on the platform the field is mostly aimed at, a replacement could put a letter in a numeric code or more characters than there are boxes, which shows up as a code that can never be complete. That is the fourth entry point and, going by the interface, the last: typing, committing, composing, replacing. Separately, requestCode took ownership of the number before the server had agreed to send anything. A custom layout can call it from the code stage with a different number, and when that send was refused the screen still described the number that had worked while the component had already adopted the one that had not -- so the code on screen would have been checked against a number no code was sent to, and getPhoneNumber named it. The number is held until the send succeeds now. Each of the three assertions fails without its change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Compared 181 screenshots: 181 matched. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1915f93365
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (index > 0) { | ||
| boxes[index - 1].startEditingAsync(); | ||
| boolean full = text.length() == length; | ||
| if (full && !complete) { |
There was a problem hiding this comment.
Fire completion after replacing a complete OTP
When a complete value is replaced by another full-length value in one edit, such as through select-all paste, autofill, or a second setText(), complete is already true and no intermediate partial-value notification occurs, so this guard suppresses the completion listener even though the code changed. Standalone flows that auto-submit from this listener consequently never receive the corrected attempt.
Useful? React with 👍 / 👎.
| public void replaceRange(int start, int end, String text) { | ||
| int replaced = Math.abs(end - start); | ||
| int used = getText().length() - replaced; | ||
| super.replaceRange(start, end, owner.accept(text, owner.length - used)); |
There was a problem hiding this comment.
Clear provisional state on iOS range replacement
When iOS performs UITextInput.replaceRange while marked text is active, CN1TextInputView.cn1EditRange() clears its native _markedRange and sends only tiReplaceRange, so no later finishComposing() callback is guaranteed. This override clears the editor's composing range through super.replaceRange() but leaves provisional true, causing the data-change callback—and potentially subsequent hardware or range edits—to suppress OTP completion indefinitely; clear the provisional flag when this replacement finalizes the native mark.
Useful? React with 👍 / 👎.
Native fidelity (iOS Modern, Metal)68 pairs compared -- median 95.0%, worst 83.5% ( Distribution --
Geometry vs native (bbox offset / size ratio / center offset / corner radius) -- gated separately from the visual score
Side-by-side comparisons (worst first)
|
















































































































































































































































Client-side phone number verification: entering the number, entering the code, and letting the platform hand the arriving SMS code over so the user never types it.
Codename One does not send the message and does not judge a code. Both belong to the application's own server, and nothing here changes that.
The hint
TextArea.ONE_TIME_CODEis a statement about what a field holds. Both platforms will offer an arriving code to a field that declares it, and neither offers anything to one that does not -- which left reading the SMS as the only route, and on Android that meansREAD_SMS, a permission Google Play restricts to messaging apps.TextField)UITextContentTypeOneTimeCodeon theUITextField/UITextView, applied after the base type so a numeric code field is still a code fieldOtpFielduses)CN1TextInputViewcarries the trait itself, cleared when a session ends so it cannot follow the caret into the next fieldsmsOTPCodewhile a code field holds the session, and answersautofill()by replacing the valueEditText, plus no-suggestionsautocomplete="one-time-code", ahead ofSENSITIVENo permission, no entitlement, no native dependency, no build hint, no BuildDaemon change.
OtpField, rebuiltThe existing widget could not receive a code at all. Its boxes were one editor each, and both mobile ports enforce a field's
maxSizeas a hard native length filter, so a six-character code offered to a one-character field was truncated to its first digit.The boxes are now drawings, and one
EditFieldbehind them holds the whole code. A typed character, a paste and a platform-offered code all arrive the same way, through one editing session that never tears down between digits. The pure editor is what makes this possible: it renders its own text, so the component can draw six boxes where a native peer would have drawn one control over them.Behaviour change:
OtpField.getBox(int)now returns the box that displays a character rather than the editor that holds it. Code that calledgetBox(i).setText(...)to drive the value stops working silently. The class shipped in 7.0.24x and was never in the guide, so real exposure is probably nil, but it is a break rather than an addition.The rest of the screen
PhoneNumberField-- country selector plus number field, producing one E.164 string. The country table is generated from libphonenumber's region metadata rather than written by hand; names are looked up asCountry.<ISO>in the theme's bundle before falling back to English.PhoneVerification-- the two stages and the things otherwise rewritten by hand on every verification screen: the button disabled while a request is out, the resend countdown, the way back to a mistyped number, a second answer to one request ignored, and an answer to a request the user has moved past dropped rather than applied to a screen it no longer owns.Plus both native themes, a developer-guide chapter, the agent skill, and
Samples/samples/PhoneVerificationSamplerunning against a fake server.Verification
android,ios,codenameone-maven-pluginandcore-unittests(which analyses the core classes).maven/javase-- ink measured in every box, and more of it in a filled field than an empty one. Against the test implementation, whose graphics draw nothing, a component that renders nothing looks identical to one that works..mfiles compile to arm64 object code against the real iOS SDK using the generated project's own headers;nmconfirms thetextContentTypeaccessors and theUITextContentTypeOneTimeCodereference are emitted rather than compiled away.Not verified here
A full-workspace
xcodebuildof the generated sample fails before reaching Objective-C, in Swift targets this branch does not touch --CN1WatchWidgets/CN1WidgetBundle.swift(accessoryCircularneeds iOS 16) and the watch target'sCN1SurfaceBridge.swift(CN1SurfaceAttributesnot in scope). That is why the iOS check above is per-file compilation rather than an end-to-end build. Not investigated.🤖 Generated with Claude Code