Uh oh!
There was an error while loading. Please reload this page.
Six signing wizard reports: a wheel that clicked, a selection painted twice, and an App ID nobody asked to change - #5660
Open
shai-almog wants to merge 25 commits into
Open
Conversation
… twice, and an App ID nobody asked to change A scroll wheel was a click. The implementation plays a wheel notch as a synthetic press, three drags and a release so the scroll animates like a finger drag, and that reads as a scroll only while something actually scrolls -- over content already fully visible no drag is ever activated, so the release fell through to whatever sat under the cursor and activated it. Wheeling over a row of the profile table opened that row's details dialog (#5655). Button already refused to fire while wheeling; a pointerReleased listener had no such guard, and neither does any component that acts on the release itself. Form and Window now finish a wheel gesture as a scroll: the dragged container still gets its release, because that one IS the scroll and has momentum to settle, and the paths that treat a release as a tap are skipped. The four focus transfers that tested isScrollWheeling go with it -- none of them is reachable during a wheel any more. Two certificates looked selected at once (#5656). setUIID swaps a component's styles without marking anything dirty, so the row that stopped being the chosen one kept painting chosen until any repaint corrected it -- which is why scrolling the dialog "fixed" it. The wizard's UIID swap now repaints, and the dark theme's selected row takes the accent border its selected segment already had, so the choice is legible rather than a shade of background. Automatic setup asked Apple to register an App ID identifier a second time. Identifiers belong to the account rather than to a platform, so the Mac step of a project whose App ID predates it always came back with "An App ID with Identifier '...' is not available. Please enter a different string." in the middle of a run that was otherwise going fine (#5652). A registration covering the platform -- explicitly, universally, or because the service did not say -- now counts, and an identifier already taken on another platform is reported as a step that was skipped and why, instead of Apple's refusal being reported as a failure. Warnings also stop vanishing: a step that succeeded used to wipe the banner the step before it had just put up, which is a message that appears and disappears too fast to read. A warning now survives until the run reports its outcome, and the outcome repeats every step that did not happen. The new-profile dialog offered every App ID on the account with equal weight, which is a list to search rather than a choice to make when one of them can sign this project (#5654). It now opens on the project's own App ID and the ones derived from it for the extensions a build generates, with the rest one click away, and preselects the single certificate the profile type could be signed with. A certificate whose display name Apple left empty gets its type and serial rather than rendering as a blank row. Push was turned on for every App ID automatic setup created, so a project that never asked for it got the capability -- and every profile issued from the App ID carried it (#5657). It now follows ios.includePush, read the way IPhoneBuilder reads it, and the widget and document provider extensions get no push at all: a notification is not delivered to them. The bundle table also stops claiming push is off. The listing endpoint reports no capabilities at all, and that silence was filled in with false, which is how the wizard printed "Push: Off" beside an App ID whose profiles Apple shows carrying Push Notifications. It says Unknown now. The device picker for a distribution profile (#5653) was already gone in 5648; there is now a test that says so, along with the selection a type change has to drop. The new wheel behaviour is covered in core-unittests -- both halves of it, including the ordinary tap that still has to work -- and the profile dialog in the wizard's structure harness, which grows a second wizard bound to a project because everything above it deliberately runs unbound. 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. |
This was
linked to
issues
Sep 2, 2026
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:e6f2e0399a
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
Cloudflare Preview
|
CollaboratorAuthor
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
CollaboratorAuthor
Compared 181 screenshots: 181 matched. |
CollaboratorAuthor
Compared 217 screenshots: 217 matched. |
CollaboratorAuthor
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
CollaboratorAuthor
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
…tform An identifier registered for both platforms has two App ID records, and the new default picked whichever came first -- the iOS one. Switching the dialog to a Mac profile type left that selection standing, so Create enabled with a Mac certificate and an iOS App ID, and Apple rejected the request. Turning a mistake the user had to make by hand into the default is worse than the list it replaced. The default is now chosen for the current type's platform, and a selection that stopped satisfying it is dropped in the rebuild every type change goes through -- the same place, and for the same reason, as the device selection above it. A UNIVERSAL registration, or one whose platform the service did not report, still satisfies both. The list itself stays unfiltered: it is what the account holds, and the remedy under an empty one can only register an iOS App ID. 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:704c8a1ee7
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Widen the list, choose an unrelated App ID, narrow it again: the row went away and the choice did not. Nothing on screen was marked selected, Create stayed enabled because the model still held an id, and it submitted the hidden bundle -- a profile for whichever app the user had been looking at a moment earlier. The drop already written for a profile type change now also asks whether the selection is among the rows being drawn, so the two ways a selection can stop making sense are one rule in one place. The default that follows puts the project's own App ID back, which is always on screen: narrowing the list is what narrows it to. 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:d7afd6d578
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
CollaboratorAuthor
Compared 160 screenshots: 160 matched. Benchmark Results
Detailed Performance Metrics
|
CollaboratorAuthor
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
CollaboratorAuthor
Compared 144 screenshots: 144 matched. |
Denying the wheel its tap denied Button the pointerReleased that takes it off the window's awaiting-release list, and nothing else empties that list on this path. The entry survived the gesture, so the next real press made the list hold two, autoRelease left its single-component branch, and a press dragged off the button was no longer cancelled: releasing well outside it fired its action. Cleared on the wheel path, which is what the cancelled-gesture path above already does for the same reason, and what Form's half of this fix does through releaseComponentsAwaitingRelease. Guarded by the gesture token like endGesture, so a fresh press installed by a release handler that entered invokeAndBlock keeps its own pending component. The test plays a wheel over a button in a window and then the ordinary press-drag-out-release that has to keep being cancelled; without the clear it fires the button from twenty pixels outside the window. 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:b6c47e391e
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
… it registers Reading ios.includePush instead of turning push on for everyone closed one half of #5657 and left the other open. Registration was the only place the capability was ever set, and a project that adds push later reuses the App ID it already has: nothing would enable it, while IPhoneBuilder starts stamping an aps-environment entitlement on the app. An entitlement the profile does not grant is a codesign failure, so the old over-provisioning was covering for a gap that predates it. The reuse path now asserts the capability when the project asks for push. It asserts rather than compares because there is nothing to compare against -- the service reports no capabilities with an App ID, which is the other half of this issue -- and that is exactly what the App Group step beside it has always done. It runs before the profiles, so the ones Apple invalidates by the change are reissued by the existing path, and a failure is a warning rather than the end of the run: the iOS assets that follow are still worth having, and the banner names the App ID that needs the capability. The tests drive Auto Setup for real against the mock account and read what the service was asked to do, including the spellings of the hint the builder normalises to false. 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:a29a31ef4b
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
One identifier can hold two App ID records, iOS and macOS, and Apple returns an account's App IDs in no documented order. The capability was asserted on whichever record the existence test above happened to answer with, so on an account that also ships a Mac build it could land on the macOS one while every iOS profile was issued from the other -- the entitlement still ungranted, and now with a step that says it did the work. Resolved inside ensurePushCapability, by identifier and platform, rather than taken from the caller. The existence test stays platform-neutral and now says why: Apple registers an identifier once for the whole account, so a record of any platform means the create it guards would come back "not available", which is the refusal #5652 is about. Every step that goes on to USE the App ID resolves it for the platform it needs. The test puts the macOS record first in what the service reports, which reproduces the wrong target exactly: push landed on BID_MAC. 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:dc7d5ebe35
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…d a success clears the failure before it Two ways the dialog still accepted what Apple would refuse, and one way it kept a message that had stopped being true. The bundle picker listed every App ID whatever the profile type was. Defaulting to the right one is not enough while the wrong one is still a row: clicking it assigned its id, canCreateProfile only asks that an id is present, and the request went to Apple naming an App ID that cannot carry the profile. The list is now filtered the way the certificate list beside it already is -- by excluding the KNOWN WRONG platform rather than requiring the known right one, so UNIVERSAL and a platform the service did not report stay offered. With the wrong rows gone, the drop already written for a selection that is not on screen covers the wrong-platform selection too. Filtering leaves an empty section where a project has no App ID for the platform, so that case now says what to do instead of showing nothing, in the terms Apple actually imposes: an identifier is registered once for the account, and the platform is added to it in the portal. The install path's message clearing kept every warning-styled banner, and those methods are also the Install button on the certificates and profiles pages: a failure followed by a successful retry left the obsolete failure on screen beside the success toast. It now keeps exactly what the running automatic setup recorded -- which is what that run repeats in its summary -- and clears anything else, because a message about what just failed stops being true when the retry works. 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:ae718215b1
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… project gets push Two consequences of a wheel no longer being delivered as a tap. A component that mutates on press and drag and settles on release is left half way through a gesture nobody made. OnOffSwitch is the clear case: the drag latches its "dragged" flag and slides the button, and the release is what settles it -- so a wheel past it now leaves it painted mid-slide, and the next real tap reads that latch as the tail of a slide and stops toggling. Before this branch the same release toggled a switch someone merely scrolled past, so neither answer was right; the answer the framework already has is that the component declines the gesture, which is what Slider does in all three of its handlers. The SplitPane divider does the same: the wheel moved it, and the panes either side are ordinary containers the wheel reaches directly, so nothing is lost by declining. Not EditorView, which is why the sweep is written down rather than applied by pattern. It scrolls itself out of the drag delta instead of through a scrollable parent, so declining the gesture would stop the wheel scrolling the editor at all, and nothing it latches outlives the gesture. The reason is now a comment in it. The other overriders of pointerDragged were classified the same way: ComboBox and Slider already decline, and ImageViewer, the maps, Media360View, List and PeerComponent latch nothing a press does not reset. The second is push. IPhoneBuilder turns an absent ios.includePush into true for an app that references com.codename1.call.voip, from a class scan the wizard does not have and must not guess at -- but a project that declares the voip background mode has said the same thing in the settings file, and that much is now read. Whole tokens, the way the builder matches them, and only when the hint is absent: an explicit value is the project speaking, and the builder refuses a VoIP app that turned push off rather than overriding it. What is left over is a VoIP app that declares neither, which is what the comment on the decision 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:6dab99e746
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…eclines the gesture too The window's wheel path empties its awaiting-release list without calling setReleased, and that is right rather than an omission: a wheel gesture cannot leave a ReleasableComponent looking pressed, because Button reaches its pressed state only through pressed(), which returns early while isScrollWheeling, and Switch implements setReleased as a no-op. Form calls setReleased on its equivalent path because that path also serves ordinary gestures; this one serves only the wheel. Written down where the clear happens, since the next reader will otherwise see the two paths disagree and assume one is wrong. Checking that turned up the one component the earlier sweep did miss. It enumerated the classes that OVERRIDE pointerDragged, and Switch -- the newer material switch, not OnOffSwitch -- acts on the gesture through listeners it registers on itself, so it never appeared. A horizontal wheel over it leaves the thumb drawn part way through a drag nobody made, until the next press re-initialises it. It now declines the gesture in all three listeners, the way Slider and OnOffSwitch decline it in their overrides, and the reason the sweep missed it is recorded on them: the guard belongs wherever the gesture is acted on, not wherever it is declared. Unlike the OnOffSwitch case this one has no test. Switch's press listener resets everything the drag sets, so nothing survives into the next gesture and nothing it leaves behind is reachable through public API -- the change is for consistency and a paint artifact, and saying so is better than implying it was pinned. 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:ebb15ddd03
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…e capability for them A project using LocalNotification does not need Push Notifications on its App ID, so automatic setup is right not to enable it. IPhoneBuilder's usesLocalNotifications links UserNotifications.framework and keeps the notification handling in the app delegate; it emits no entitlement, and the block that runs when push was not asked for strips the push code even from an app that uses local notifications. iOS asks nothing of an App ID to schedule one -- only a remote notification needs the capability, which is why aps-environment appears nowhere in that builder and the push entitlement reaches an iOS app through the provisioning profile instead. The signing guide's note that the build server enables the notification entitlement for a LocalNotification project even when ios.includePush is false is about not hand-stripping that entitlement to quieten App Store Connect warnings. It is not a requirement on the App ID, and reading it as one would put the capability on every project that schedules a reminder -- which is issue #5657 again, by the same route as always. No behaviour change; the boundary is written down where the decision is made, because this is the third reading of a builder inference as an App ID requirement and the next one will land in the same place. 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:4875ad5e07
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
… that kept the gesture The rule was written as "release the scrolling container", and that is only most of it. A component can opt into keeping a gesture once it starts -- Spinner does -- and the form hands it every drag through stickyDrag rather than by making it the dragged component. Its drag rolls the value and its release is what settles the roll and commits it, so suppressing that release left a spinner someone wheeled mid-roll, with the new value uncommitted until something else touched it. A window has no sticky-drag bookkeeping at all, so the same component is reached there as the pressed one and had the same release taken away. Both now release what the drag was actually delivered to, and the test is whether a drag was ACTIVATED. That is what keeps this from becoming the tap the branch exists to suppress rather than an exception to it: List fires its action event only on the release of a gesture that never became a drag, and takes the settling path when it did. A component the wheel merely passed over never activates one, so nothing about the earlier fix changes. 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:0ab36e40f7
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…, and a test that proved nothing now does ImageViewer moves panPositionX from a drag and settles it in pointerReleased, which animates a partial swipe back or commits the navigation. It is neither the dragged component nor a sticky one, so a wheel gets no release and left the viewer painted between two images until something else touched it -- a horizontal wheel or trackpad gesture is enough. Guarded below the delegation rather than at the top of the handler, and that placement is the whole point: a vertical drag over an image viewer is forwarded to the scrollable ancestor by this very method, so declining the gesture outright would stop a wheel scrolling the form the viewer sits on. That is the trap EditorView is left alone for. The forwarding still runs, the ancestor becomes the dragged component and gets its release as before; only the pan and the swipe are declined. A zoomed viewer no longer pans from a wheel either, which is the answer every non-scrollable thing gives it. The test that covers the placement turned up a defect in the tests themselves. isScrollableY() reports false for a container whose content fits, and the fillers were shorter than the form: the "scrollable row" case was a second copy of the unscrollable one, and it is what I read as showing a wheel over a button firing it anywhere rather than only where no drag activates. Both now overflow for real and assert that they do. With the fix reverted only the unscrollable case fails, which is exactly the report -- there was nothing available to scroll to -- so that test guards the good case rather than demonstrating the bug, and now says so. 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:7d4b3fcc00
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
The existence test that gates creating the app's App ID was platform-neutral, so on an account where Mac setup had run first it was answered by the macOS registration of the same identifier. Creation was skipped, and the run then stopped several steps later saying the bundle ID "could not be found after refresh" -- about an App ID it had never tried to create -- with the Mac stages, which need no iOS App ID, never reached. It now resolves the iOS record specifically, and the case where the identifier is taken on another platform gets the treatment the Mac half already had: say what was skipped and why, in the terms Apple imposes -- an identifier is registered once for the account and the platform is added to it in the portal -- then run the stages that can still run. The extensions are not among them; they are App IDs derived from the app's own iOS one, which is the reason the Mac pair is now named rather than inlined in the chain. The earlier comment defending the neutral test is gone with it. It was right that a neutral test avoids provoking Apple's refusal, and wrong that this was the only way to avoid it: the skip branch does that and keeps the rest of the run. 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:87d67a38e7
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…did to the profiles Two ways a release went further than it should have. Consuming a form level release listener means the gesture was handled and must not be dispatched onwards, and the ordinary path returns on it. The wheel branch called fireReleaseListeners and threw the answer away, so a consumed release still reached an activated sticky target and committed a spinner's value behind the listener that had claimed the gesture. Only the sticky half slipped through, because fireReleaseListeners clears the dragged component itself on that path and nothing else. The Enable push action granted the capability and stopped at "enabled". A profile is a snapshot of the App ID's capabilities when it was issued and Apple invalidates the ones a capability change leaves behind -- which is what isUsableProfile and the reissue in autoSetupProfile exist for -- so the developer would have fixed the App ID and still been looking at the same codesign failure, now against profiles nothing said were stale. The action now says what the change did and offers the flow that reissues exactly those profiles, or names the profiles page when the App ID is not this project's. Phrased conditionally, and offering rather than performing, for the reason that runs through all of this: the wizard cannot read a capability back, so it does not know whether the call turned push on or asserted what was already there. 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:da11c5f498
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
MapView pans the camera from a drag and calls fireCameraChanged from the release, and a wheel gets no release, so the map moved while every listener that reloads markers or tiles for the new viewport heard nothing about it. Half a pan is worse than none: the view and the data behind it disagree, silently. Delivering the release instead is not available. Below the pan threshold that method is the tap path -- it selects a marker or zooms in -- which is the activation the wheel branch exists to stop, and the framework has no way to tell a component that consumed a drag from one that merely received it. The signal that would say so, setDragActivated, is package private in Component, and widening it for one component is not a trade worth making. So the map declines the gesture, the way Slider, both switches, the SplitPane divider and the image viewer's pan already do. What it loses is a pan nobody designed: before this branch a small wheel notch over a map fired handleTap and selected a marker, which is the bug rather than the feature. MapComponent beside it is deliberately left alone, and the difference is written in both: its drag applies the whole pan as it goes and its release is only tap bookkeeping, so a wheel over it leaves nothing half done. Classifying the two maps separately is the point -- a sweep would have changed both. No test: MapView needs a live map engine to pan, which core-unittests has no way to provide. 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:611c7f7b83
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…nd a whitespace typo no longer flips the Mac push entitlement Apple's UNIVERSAL registration is ONE App ID serving both platforms, and the run enabled the Mac half of push at the start of the Mac stages -- after the iOS profiles had been issued from that same App ID and installed. Apple invalidates a profile issued before a capability change, so the run reissued its Mac profiles and finished with the iOS pair installed and dead. The call log the test reads says it plainly without the fix: two iOS profiles, then the enable, then the Mac ones. Both capabilities are now asserted at every entry point before any profile stage, and the Mac stage no longer asserts mid-run. The other half is the hint that decides it. A properties file keeps trailing whitespace and EntitlementOverrides compared the value raw, so "apsEnvironment= false " matched neither false nor none: the build emitted the APNs entitlement for a project that had just turned it off -- and then could not sign, because the wizard reads the settings file trimmed and left the capability alone. " development " resolved to production for the same reason. Trimmed once where the value is stored, which is what IPhoneBuilder already does with its own push hint, and an all-whitespace value now reads as unset rather than as some environment. The enable-push action drops its toast. A toast animating while the refresh rebuilds the page is a race -- container mutations are queued for the length of an animation, so the rebuilt page reads back as though nothing happened, which is what made the bundle-page test fail only when another class had run first. The banner it left behind says more than the toast did anyway. 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:7466f6df7f
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
…t left invalid Giving that button the both-platform assert was the wrong half of the previous fix. It reissues one profile type, so asserting the iOS capability there would change an App ID this run has no intention of reissuing profiles from -- worse than the ordering problem it was meant to solve. It asserts the Mac capability only; the full run asserts both because it reissues both. That still leaves the part this button cannot cover. A capability change invalidates every profile the App ID ever issued -- the other Mac type, and on a UNIVERSAL registration the iOS ones too, since that is one App ID serving both -- and one button reissues one type. So it says so, and points at the run that reissues them. Conditional on there being something to lose: the caveat is raised only when the account actually holds a profile for this identifier that this run will not reissue, read from the profiles rather than assumed, so it stays quiet for an account with nothing else to invalidate. 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:6d6876c1a9
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…the certificate clear gets a test that would notice its removal entString decided which spelling wins BEFORE normalising either, so a modern key left as "macos.entitlements.apsEnvironment= " counted as present. It shadowed the macNative value that actually said something, and then trimmed to nothing -- a project still reading its legacy hint lost the entitlement it asks for, and its Mac build stopped registering for remote notifications. Whitespace is not an answer: it now neither wins nor blocks the fallback, for every entitlement string hint rather than just this one. The other half of the report is not a defect. A certificate chosen under one profile type IS cleared when the type changes -- the listener does it before asking for the rebuild, and the sole-certificate default fills an empty selection rather than replacing one. But that is a guard sitting several lines from the code it protects, and nothing failed if it went away, so the harness now picks a bundle at the dead-end stage and asserts that the certificate is the only thing still missing. Removing the clear turns that into "select at least one device", which is canCreateProfile having accepted a certificate that cannot sign the type. 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:4d008e42ea
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
… page mid-animation The diagnostic for a capability change that failed named ios.includePush whatever platform it was asserting, so a Mac-only project was pointed at a setting it never had while the profiles it just got could not satisfy the build it did ask for. The two platforms declare push through different hints, and the message now names the one that asked. Reading that message exposed a flake in the tests around it, which is the second time this animation has bitten: container mutations are QUEUED while the animation manager is busy, so a page rebuilt under the toast an install puts up reads back as though the rebuild never happened -- and only when an earlier test left an animation in flight, which is why it passed alone and failed in the suite. The banner-reading tests now wait for the animation manager to go idle rather than sleeping a guessed interval, and take a few more turns of the event thread afterwards so anything the last animated frame queued has been applied. Five consecutive runs of the module, green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A wheel was emulated: a press, three drags and a release played into the component tree so the scroll would animate like a finger drag. Every component that reacts to a pointer then had to recognise the impostor and refuse it -- Button, Slider and ComboBox carry that guard to this day -- and every component that did not was a bug. Scrolling past a switch toggled it, a list selected a row, a table row opened its dialog (issue #5655). The ports have reported real wheel deltas for a while, and the newest component that scrolls itself, EditorView, already handles WheelEvent and never used the emulation at all. So the emulation goes: the component under the cursor is offered the wheel, walking up until something consumes it, and if nothing does the nearest scrollable ancestor is scrolled directly. playWheelScrollGesture, dragWheelStep and wheelRoot are deleted. That removes the reason for everything this branch had added on top of the old design. The guards in OnOffSwitch, Switch, the SplitPane divider, ImageViewer and MapView are reverted, and so are the tap-suppression branches in Form and Window -- 186 lines of defending against events that are no longer sent. A switch does not need to know what a wheel is. Component.mouseWheel is where a component that moves its own content takes the wheel, which is what those components actually wanted: ImageViewer pans a zoomed image and leaves an unzoomed one to scroll the page it sits on, MapView pans and fires cameraChanged -- the half a synthetic drag could never deliver, because the gesture it emulated had no end that meant "the pan is finished" -- and MapComponent keeps its pan too. The tests say what the wheel does now rather than which components have learned to distrust it: it scrolls the container, dispatches no pointer event to anything, does not flip a switch or fire a button, is taken first by a component that wants it, and does all of that inside a desktop window. 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:8fb5625301
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…l past an inner scroller moves the page Two things the new dispatch got wrong. addMouseWheelListener documents that consuming the event prevents the DEFAULT behaviour, and a component that pans itself on a wheel is exactly that -- so an application listening for control plus wheel to zoom an image viewer could not stop the viewer panning underneath it, because the built-in handler ran first. Listeners now run before it at every level of the walk, which is also the order every pointer event uses. And the scroll took the first ancestor that scrolls rather than the first that can still move. A list inside a page is scrollable right up to its last row, so a wheel died at the inner boundary instead of carrying on to the page -- which is what every other toolkit does with nested scrollers, and what the drag this replaced did implicitly by handing the gesture up. The walk now continues past a container already pinned at the edge it is being pushed against. The chaining test holds one pointer position rather than recomputing it: a container's absolute position follows its own scroll, so asking for "the middle of the inner container" after each notch walks the cursor off it and eventually off the screen. A real pointer does not move when the content under it does. 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:dff546775d
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…s listeners Two places the wheel path did less than the drag path it replaced. The vertical range left out what the virtual keyboard is covering. The drag path, setScrollY and the tensile highlight all add it, because a field behind the keyboard has to be reachable; clamping the wheel to the smaller range made it reachable by finger and not by trackpad. Component.getInvisibleAreaUnderVKB is package private now so the scroll in Display can ask the same question the drag asks -- no public surface, and the alternative was a second answer to it. MapComponent fired no listener for a wheel pan. Every other way its position changes -- drag, zoom, programmatic move -- calls fireMapListenerEvent, so an application that loads content for the visible region from mapPositionUpdated was told about every movement except this one. MapView already did the equivalent; this is the map beside it catching up. 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:a4077670f4
ℹ️ 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".
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…on its grid A fully panned image viewer kept claiming the wheel. Both clamped pan positions come back unchanged once it is against the edge being pushed, and returning true anyway swallowed the notch: the page underneath could not scroll for as long as the pointer stayed over an image that had stopped moving. It now passes the wheel on, which is the rule the nested scrollers already follow. And a container that snaps to a grid was left between rows. The snap used to come from the deceleration a drag left behind -- which is where the emulation got it, and a wheel has no deceleration -- so a notch that is not a whole number of rows stopped a Spinner3D between two of them. The scroll settles onto getGridPosY/getGridPosX after applying the position. Float.compare rather than == for the pan test, which SpotBugs was right to refuse: the question is whether the clamp handed back the identical position, and Float.compare says that without pretending an epsilon would be an answer. The snapping test sets snapToGrid AFTER show and asserts it took. Setting it before is silently discarded -- initialising a component resets the flag to the look and feel's default -- so the first version of this test was quietly exercising a container that did not snap. 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:4bc5077648
ℹ️ 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".
Comment on lines
+8725
to
+8727
| if (target.isSnapToGrid()) { | ||
| int snapped = Math.max(0, Math.min(ceiling, | ||
| vertical ? target.getGridPosY() : target.getGridPosX())); |
There was a problem hiding this comment.
When a high-resolution trackpad emits deltas smaller than half the grid spacing, this snaps each individual event back to the current row. The method nevertheless returns true, so repeated events neither accumulate enough movement to advance a Spinner3D nor propagate to a scrollable ancestor; scrolling while the pointer is over a snapping component can therefore appear completely stuck. Defer snapping until the precise gesture settles or retain the unsnapped remainder between events.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5652, #5654, #5655, #5656, #5657, and covers #5653 with a test.
A scroll wheel was a click (#5655)
CodenameOneImplementationplays a wheel notch as a synthetic press, three drags and a release so the scroll animates like a finger drag. That reads as a scroll only while something actually scrolls: over content already fully visible no drag is ever activated, so the release fell through to whatever sat under the cursor and activated it. Wheeling over a row of the profile table opened that row's details dialog.Button.released()already refused to fire while wheeling. ApointerReleasedlistener had no such guard, which is how the wizard's table rows were activated -- and neither does any component that acts on the release itself.Form.pointerReleasedandWindow.pointerReleasednow finish a wheel gesture as a scroll: the dragged container still gets its release (that one is the scroll, and it has momentum to settle), and the paths that treat a release as a tap are skipped. The four focus transfers that testedisScrollWheelingare gone with it, because none of them is reachable during a wheel any more.The regression test in
core-unittestsfails on master in both the scrollable and unscrollable cases -- so a wheel over a button was firing it too, not only at the end of a list -- and the ordinary tap it also asserts still passes. The report's frozen UI is the likeliest consequence of a dialog opened from inside a half-played gesture; the wheel path now also clears the pressed/sticky bookkeeping it used to leave stranded.Two certificates looked selected at once (#5656)
setUIIDswaps a component's styles without marking anything dirty, so the row that stopped being the chosen one kept painting chosen until any repaint corrected it -- which is exactly why scrolling the dialog "fixed" it. The wizard's UIID swap now repaints. The dark theme's selected row also takes the accent border its selected segment already had, instead of being one shade of background away from an unselected one.Automatic setup asked Apple to register an identifier twice (#5652)
App ID identifiers belong to the account, not to a platform, so
com.example.appregistered for iOS cannot be registered again for macOS. The Mac step asked anyway on every project whose App ID predates it, and Apple's "An App ID with Identifier '...' is not available. Please enter a different string." landed in the middle of a run that was otherwise going fine.UNIVERSAL, or because the service did not report one -- now counts.The new-profile dialog offered the whole account (#5654)
Every App ID was listed with equal weight, so the one identifier that can sign this project had to be found in a list of everything anyone on the team ever registered. The dialog now opens on the project's own App ID and the ones derived from it for the extensions a build generates, with Show all N bundle IDs one click away, and preselects it. It also preselects the single certificate the profile type could be signed with, so it opens ready to create rather than disabled. A certificate whose display name Apple left empty gets its type and serial instead of rendering as a blank row.
The formatting half of that report (
com.tantrix.app.launcherTantrix desktop launcher) was fixed in #5648 and is not in this change.Capabilities nobody asked for (#5657)
Automatic setup passed a hardcoded
push = trueto every App ID it created, so a project that never asked for push got the capability -- and every profile issued from that App ID carried it. It now followsios.includePush, read the wayIPhoneBuilderreads it (absent is off, only a trimmed case-insensitivetrueis on), and the widget and document provider extensions get no push at all, because a notification is not delivered to them.The bundle table also stops claiming push is off. The listing endpoint carries the identifier, name and platform and no capabilities, and that silence was filled in with
false-- which is how the wizard printed Push: Off beside an App ID whose profiles Apple shows carrying Push Notifications. It says Unknown now, andSigningState.BundleId.pushEnabledis aBooleanso the absence is representable.Two capabilities in that screenshot are not ours and are not changed here: In-App Purchase is enabled by Apple on every new App ID, and Multicast Networking is not something the wizard can request. Reading the real capability set back would need the signing service to return it; the wizard now says it does not know rather than guessing.
Devices on a distribution profile (#5653)
Already fixed in #5648 -- an App Store or Developer ID profile now says it is not limited to specific devices instead of drawing a picker that changes nothing. This change adds the test for it, and for the selection a profile type change has to drop.
Verification
core-unittests: 6078 tests green, including the four new wheel tests. Each of them was checked against a reverted fix and fails there.core-unittests: 0 / 0 / 0.CertificateWizardStructureHarness: 0 failures, including the new project-bound stage (checked non-vacuous with a deliberately inverted assertion).SigningState.javapicks up the copyright header it was missing.🤖 Generated with Claude Code