Native desktop windows - #5556
Open
shai-almog wants to merge 139 commits into
Open
Conversation
Introduces com.codename1.ui.TopLevelContainer, the interface implemented by anything that can sit at the root of a component hierarchy. Today that is only Form; a later commit adds Window, the desktop native-window top level. Every member is chosen from a count of the direct getComponentForm().<method>() chains in CodenameOne/src, so the interface is the measured contract core actually depends on rather than a guess. It is dominated by animation registration, focus, and the layered panes. Every method already existed on Form with an identical public signature, so this commit adds no behaviour and needs no Form change beyond the implements clause and the new asContainer() bridge -- a Java interface cannot extend a class, so without it a TopLevelContainer reference could not be passed anywhere a Component is expected. Deliberately excluded: MenuBar and the soft buttons (MenuBar is coupled to Form's tint, back command and actionCommandImpl), dispose()/isDisposed() (package private on Form, and it means "pop back to previousForm" rather than "destroy this window"), and the mobile navigation surface -- transitions, back command, previousForm, tint and orientation listeners. Members already on Component or Container are reachable through asContainer(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds Component.getTopLevelContainer(), the resolution path that replaces getComponentForm() in code which has to keep working inside a desktop Window. getComponentForm() is untouched and keeps its meaning: it returns the enclosing Form, and null for a component hosted in a Window, because a Window is not a Form. The internals that Component, Container and Toolbar need in order to drive a top level -- the internal animation registry, focus, the revalidate queue, the drag and press state -- are declared package private on Container rather than on an interface. Every method of a Java interface is implicitly public, so an interface would have silently widened Form's public API; Container is the nearest common supertype of Form and Window, so the calls still dispatch virtually with no instanceof. The defaults are inert and Form overrides the ones that mean something to it. Also adds com.codename1.impl.WindowManager, the single facade carrying the whole native windowing contract, reached through one new CodenameOneImplementation.getWindowManager() that returns null by default. This follows getHealth()/getBluetooth()/getCarBridge(), and keeps several dozen methods out of an already very large class. The null return is itself the capability query, so no separate supported flag can drift out of step with it. Only operations every windowing system provides are abstract; the rest have inert defaults so a later addition cannot break an existing port. No behaviour change: no port implements a window manager yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Migrates the load-bearing call sites in Component and Container off getComponentForm() and onto getTopLevelContainer(), so they keep working when the root of the hierarchy is a Window instead of a Form. The sites were picked by reading, not by pattern. Two groups: Sites that dereferenced the Form with no null check, and so would have thrown rather than degraded: growShrink and its BGPainter animate loop, the material pull to refresh release, the deinitialize path that unhooks the refresh drag listener, chooseScrollXOrY, moveScrollTowards, and the drop handler that animates the hierarchy. Several of these could already NPE today for a component detached mid animation, so they are now guarded as well as migrated. Sites that were guarded and would therefore have gone quiet -- the worse failure, because each one silently removes a whole feature: all pointer dragging, kinetic and smooth scrolling, drag and drop, focus, the animation manager behind every animateLayout, animated backgrounds, the revalidate-on-style-change gate, and revalidateInternal, which is the root of the layout system. Adds four more package private hooks to Container that these sites need -- getFocused, isRevalidateFromRoot and the directional focus finders -- following the pattern established for the rest: inert defaults on Container, overridden by the top level. Left alone deliberately: fireFocusGained and fireFocusLost reach for getMenuBar(), which a Window has no equivalent of, so the existing null guard already yields the right behaviour there. All 4790 core unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Groups the four fields that describe "the thing being painted" -- the dirty queue, its swap buffer, the fill count and the Graphics -- into a PaintSurface. The application's main surface becomes one instance of it, and a later commit gives every native window another. paintDirty() keeps its signature and behaviour and now delegates to a surface-parameterized routine, with paintDirtyWindow() entering the same routine for a window. Having one copy matters: that method carries the clip and paintable-bounds handling from issue #5273, and a per-surface copy would be free to drift. The flush-region hint is routed per surface. Its window form is inert by default rather than delegating to the main-surface version, so an immediate mode port that has not opted in cannot clamp a window's clip against the main window's state. repaint(), cancelRepaint() and hasPendingPaints() keep their signatures, so the JavaSE and Android overrides that call super still compile and behave. cancelRepaint now sweeps every surface, since its callers have no window context. No behaviour change: nothing creates a window surface yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Extracts the layer lookup and z-index insertion out of Form into TopLevelSupport, so Window can reuse it instead of carrying a second copy. The logic is moved verbatim, including the getChildrenAsList(true) reads: the comment there is load bearing, since iterating the container directly does not find components while an animation is in progress and the method would then add a duplicate layer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Window is the desktop counterpart of Form: a second native operating system window with its own component hierarchy, focus owner, animations, revalidate queue and dirty region. The main surface stays a Form and is untouched. Desktop is the public API parallel to Display. Display keeps answering "how big is the application's main surface", which is the only question a phone has; Desktop answers "what screens exist and what windows are open". It owns the window registry and hands out Monitor snapshots, and every one of its methods degrades safely where there is no windowing system -- an empty window array, a single monitor describing the main display -- so only constructing a Window throws. Monitor carries per-monitor geometry, work area, density and backing scale, and a Window reports the density and scale of the monitor it is currently on rather than the global one, which is what makes a mixed-DPI desktop render correctly. Event routing packs the window id into the high bits of the event type word. Window 0 is the main surface, and for it the packed word is numerically identical to what it always was, so the wire format, drag coalescing and the stack swap are all untouched. The id is an int chosen by the framework and echoed back by the port, so the off-EDT input path needs no map and no lock. Key repeat and long press now return to the top level the press came from. Fixes a latent infinite EDT spin this makes reachable: handleEvent returned without advancing the offset when it had no form to dispatch to, while the caller loops while (offset < end). It was unreachable only because the public entry points all guard on a non-null current form. skipEvent now drains the packet so the rest of the batch -- which may contain main form events -- still dispatches. Fixes two adjacent bugs the same code path forced into the open: a key or pointer release aimed at a different form than the press left its payload in the stack, where it was then read as the next event type; and the multi-touch release passed the x array as both coordinates. Modality blocks input in core rather than in the ports, so a modal window behaves identically everywhere whether or not the platform implements its own; ports still set the native flag for correct focus and taskbar behaviour. showModal parks the caller through invokeAndBlock exactly as a modal Dialog does, so every other window keeps painting. All 4790 core unit tests pass. Two of them reach into the paint queue by reflection and were updated for its move onto PaintSurface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Removes the two assumptions in the JavaSE port that there is exactly one canvas, which is what stands between it and a second rendered window. getGraphics(Object) fell through to canvas.getGraphics2D() for any screen graphics, so a secondary window would have drawn into the primary window's buffer. NativeScreenGraphics now records the canvas it belongs to and resolves through that. isScreenGraphics(Graphics2D) was literally an identity comparison against the primary canvas's buffer. It is now a membership test over the registered screen buffers. This matters because drawNativePeerImpl uses it to decide whether to undo the zoom scale, so answering wrongly for a second window would mis-scale its peer components. The registry is maintained at the only three places C.g2dInstance is written -- created in getGraphics2D, discarded in createBufferedImage and in the size change reset -- so it cannot drift. Behaviour with a single window is unchanged: the primary canvas is still the owner of its own graphics, and the membership test still answers true for exactly the buffer the identity comparison used to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The first real port implementation, and the one that decides whether the design holds. Each Codename One Window becomes a JFrame containing its own instance of the port's existing C canvas, so a second window inherits the whole buffered blit machine -- including the blitCounter aliasing fast path, which is already per-instance state -- with none of it duplicated. Input is tagged at the source: C carries the window id it renders and its listeners dispatch through the window-aware entry points, so an event reaches the right hierarchy without a lookup on the AWT thread. Window id zero routes to the main surface, so the primary canvas keeps its exact previous behaviour. Monitors come from GraphicsEnvironment, with the work area taken from the screen insets so a window centres or maximises without landing under the task bar or dock, and the backing scale from each GraphicsConfiguration's default transform rather than one global retina scale. A window that is dragged onto a display with a different scale raises a monitor-changed event, which is what lets the framework re-lay it out instead of leaving it blurry. Multi-window reports unsupported while a phone skin is loaded, reusing the predicate isFullScreenSupported already applies: a skin simulates one device screen with its own coordinates and zoom, and a real operating system window inside that simulation is incoherent. Headless likewise. Also qualifies java.awt.Window in SourceChangeWatcher, which wildcard-imports both java.awt and com.codename1.ui and so became ambiguous the moment com.codename1.ui.Window existed. A repo-wide scan found no other collisions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds TestWindowManager, a window manager with no operating system behind it, and wires it into TestCodenameOneImplementation as an opt-in. It defaults to absent, so the unsupported platform every mobile port reports is also the default a test sees, and the throwing path is exercised without arranging anything. Its monitor table is scriptable, which is what makes per-monitor DPI testable at all: DesktopMonitorTest describes a 2x laptop panel with a dock reserved at the bottom next to a conventional external display, then asserts that a window picks up the scale and density of whichever one it sits on and that moving between them marks its preferred sizes stale. Getting that wrong is what produces a blurry or mis-sized window, and it would otherwise need a second physical display to catch. WindowTest covers the rest of the contract: constructing a Window on an unsupported platform throws rather than degrading, Desktop still answers safely there, show creates exactly one native window, dispose releases it and is idempotent, title and bounds reach the native window, close honours the close operation and can be vetoed, chrome and modality reach the peer, and each window gets its own id since events are routed by it. Two assertions are the load-bearing ones for the chosen design: a component in a Window resolves that Window through getTopLevelContainer(), and getComponentForm() returns null for it -- while a component in a Form still resolves both. 4808 core unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ates SpotBugs is a zero-findings gate and this change tripped ten. Fixing them properly rather than excluding them turned up a real gap. Five were unused fields on Window -- the press coordinates, the press token and the dragged component. They were unused because Window had no pointer dispatch at all: Container does no hit testing of its own, Form does that work itself, so without it a press inside a window never reached the component under it. Window now performs the same walk Form does, minus the title area and menu bar special cases it has no equivalent of, and implements the Container hooks that expose the press state -- which is what the migrated drag and scroll code in Component reads. One was a naked notify in dispose(). The flag showModal parks on is now published under the very monitor the waiter is blocked on, with a separate flag guarding re-entry, so the wake is tied to the state change rather than being incidental. Four were anonymous Runnables in Display retaining their enclosing instance. They are now one named static WindowCallback. SpotBugs, PMD and Checkstyle are clean over core-unittests; 4808 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were parented to the primary canvas unconditionally, so a BrowserComponent or a text field inside a desktop Window would have appeared on the main window instead of the one containing it. Peer.addNativeCnt now resolves its frame through the owning window at attach time rather than at construction: a peer is created before it is added to a hierarchy, so its window is not knowable when the Peer object is built. editString attaches the Swing editor to the owning window's canvas, and stopTextEditing removes it from whichever canvas it actually landed on rather than assuming the primary one. Both resolve through Display.getWindowPeerForComponent, which walks the component's top level -- so a component on the main form still gets exactly the previous behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds desktop windows to the native Windows port. Each Codename One Window is a slot in a new native table with its own HWND, ID2D1HwndRenderTarget and CN1Graphics. The main window is deliberately left out of that table. It stays in cn1Win with its existing HWND, render target and graphics untouched, so the single-window path -- which every existing app and every screenshot baseline exercises -- cannot change behaviour. Secondary windows also get their own window procedure rather than sharing the main one, which is full of main-window-only cases. Window identity in that procedure comes from GWLP_USERDATA set in WM_NCCREATE: O(1) and lock free, which matters because it runs on the pump thread while the EDT is drawing. Events carry the framework's window id, which the native side stores at creation and echoes back, so routing needs no lookup. Creation and destruction marshal to the pump thread through a new WM_CN1_DESKTOPWINDOW, following the blocking SendMessageW pattern the native edit control and file dialog already use -- a window must be created on the thread that owns the message loop. Everything else is legal cross-thread and runs directly. The message loop itself needs no change: GetMessageW already pumps every window owned by the thread. Two things carried over deliberately from the main window because getting them wrong is subtle: D2D1_PRESENT_OPTIONS_RETAIN_CONTENTS, since Codename One repaints only the dirty region and relies on the rest surviving the present; and recording a resize for the drawing thread to apply between frames rather than resizing the render target from the pump thread, which presents black. WM_DPICHANGED honours the rectangle Windows suggests and reports the monitor change, which is what keeps a drag between mixed-DPI displays from leaving the window the wrong physical size. Monitors come from EnumDisplayMonitors with the work area from MONITORINFO, and per-monitor DPI from GetDpiForMonitor resolved dynamically since shcore.dll only exists from Windows 8.1. WM_DESTROY on a secondary window deliberately does not PostQuitMessage: closing a tool window must not exit the application. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds desktop windows to the native Linux port. Each Codename One Window is a slot in a new native table carrying its own GtkWindow, GtkOverlay, GtkDrawingArea, GtkFixed peer layer and cairo back buffer. The main window keeps its own file statics in cn1_linux_window.c and is not part of that table, so the existing single-window path is unchanged. Routing is essentially free here, which is the nice part of GTK: every signal handler already takes a gpointer closure, so passing the window struct as the closure data makes each handler window-scoped with no lookup and no shared state. gtk_main_iteration already services every window in the process, so the loop needs no change either. Events carry the framework's window id, stored at creation and echoed back. The delete-event handler returns TRUE so GTK does not destroy the window: Codename One decides, because an application may veto the close from a listener. The window's back buffer sets isWindowTarget, which turns on the #5273 clip clamp -- a clip set while a component paints is confined to the region about to be flushed, so an oversized fill cannot leave stale pixels on the persistent cairo surface. GTK is not thread safe, so every entry point marshals to the GTK main thread through cn1LinuxRunOnMainAndWait, which the port already uses for exactly this. Monitors come from GdkDisplay, with the work area from gdk_monitor_get_workarea. Scale reports GTK's integer scale factor, since that is what actually governs how the toolkit renders, while dots per inch is derived separately from the monitor's reported millimetre size -- the integer factor is far too coarse to describe a display's real resolution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds desktop windows to the Mac Catalyst slice. A Codename One Window becomes a UIWindowScene, with the whole implementation inside #if TARGET_OS_MACCATALYST so the object file an iPhone or iPad build produces is empty and the plain iOS binary is unchanged. Unlike the other desktop ports, the window's content is rendered into a mutable image and the finished raster is assigned to the scene view's layer, rather than the window owning a second Metal surface. That is a deliberate trade: the render path caches its device, pipeline state and glyph atlas against the single rendering view, and making those per-scene is a large refactor of the hottest code in the product, without ARC. The scene still owns a real UIKit view hierarchy, so native peers and native text editing work normally inside a window -- only the drawing arrives as a bitmap. Multi-window is opt-in through a new macNative.multiWindow build hint. That is not caution for its own sake: the existing comment in IPhoneBuilder records that turning UIApplicationSupportsMultipleScenes on changed Catalyst windowing and crashed the screenshot suite with a 26 GB signal loop. The hint now gates both that Info.plist key and IOSImplementation.getWindowManager(), so the key and the API that requires it are switched by the same flag and cannot disagree. Scene arrival is asynchronous, so a created window claims the next scene the delegate receives; the delegate hands it over before installing the main root view controller, and only the application's own scene falls through to that. Teardown releases the scene, window, controller, view and title on the main queue after UIKit has finished with them, since this port has no ARC. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The part of the test story that actually demonstrates windowing. A picture of a window proves nothing; these re-run representative UI INSIDE a real operating system window and compare that window's own capture against its own baseline. WindowHostTest hosts content in a Window at three sizes -- 400x300, 900x700 and a deliberately non-square 1000x400 -- and captures through Window.capture() rather than Display.screenshot, because the ordinary path can only see the application's main framebuffer and a second window simply is not in it. The three sizes are the point: a window still measuring itself against the main display would produce three near-identical goldens. The cases were chosen for what fails silently rather than for coverage count. Layout proves sizing and theming resolve against the window. Scroll proves the scroll path, which goes quiet rather than throwing if a component cannot resolve its top level. Graphics exercises the port's pipeline on a non-primary render target with shapes that deliberately reach the edges, where a wrong clip clamp leaves stale pixels. Editing covers native text input, which used to attach the platform editor to the main window's canvas unconditionally. Overlay covers the layered pane that Sheet, InteractionDialog and ToastBar attach to. Modal captures the BACKGROUND window while a modal is up, which is the state that would be blank if the nested event loop had stopped servicing it. MultiWindowApiTest is the behavioural half: no screenshot, runs everywhere, and asserts against what the port reports rather than pixels. Where there is no windowing system it asserts the opposite -- that the capability query says so and that constructing a Window throws rather than degrading. The suite skips without emitting a golden where windows are unsupported, so mobile baselines never contain a picture of something the platform cannot do. The new tests are recorded as not-run in every stored port report, which is honest: CI has not executed them on those targets yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a Desktop Windows chapter next to Desktop Integration, covering the whole feature: where windows exist and where they throw, the Form/Window relationship through TopLevelContainer, lifecycle and close vetoes, chrome and the two coordinate systems, modality, monitors and per-monitor DPI, events, peers and native editing, and the Mac Catalyst opt-in. Two things are called out rather than buried, because they are what will actually catch someone out. getComponentForm() returns null inside a Window, and the failure mode is silence rather than an exception, since most code guards on null and quietly does nothing -- so a component that will not scroll or focus in a window has a named cause. And Catalyst multi-window needs the macNative.multiWindow build hint, because a second window is a second scene and that requires a process-wide Info.plist key. Vale reports zero issues at suggestion level, LanguageTool zero matches across the guide, and the paragraph capitalization check passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a port-level test for the riskiest edit in this work, which had no coverage before and sits in the paint path where a regression shows up as wrong pixels rather than an exception. Two canvases must resolve to two distinct screen buffers -- sharing one is exactly what would make a second window draw into the first window's pixels. And isScreenGraphics has to answer true for a secondary window's buffer as well as the primary one, but still false for a mutable image: drawNativePeerImpl uses that answer to decide whether to undo the zoom scale, so a wrong answer mis-scales a window's peer components. 222 JavaSE port tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compiling CN1MacWindows.m against the actual Mac Catalyst SDK -- which the earlier commit never did -- turned up three defects that would have shipped. Scene-to-window matching was a race. Creation returns a slot immediately and requests the scene asynchronously, and the arriving scene was handed to the first unattached slot. Two windows opened in quick succession could therefore swap identities. Scenes are delivered in request order, so the pending slots are now a FIFO, enqueued on the same main-thread turn as the request; a window destroyed before its scene arrives leaves the queue. The presented frame was a use-after-free waiting to happen. flushGraphics allocates a local Java int[], and the native side wrapped that pointer in a CGBitmapContext, then used the resulting image on a later main-queue turn -- by which time the array is garbage and the collector may have reclaimed or moved it. The pixels are now copied, and handed to a CGDataProvider with a release callback rather than a bitmap context: CGBitmapContextCreateImage is copy-on-write, so it is not defined when the backing buffer becomes free to release, whereas the provider makes that lifetime explicit. The alpha format was wrong. getRGB returns straight ARGB and the image declared kCGImageAlphaPremultipliedFirst, which would darken every pixel that is not fully opaque. A window's content is opaque, so it now skips the alpha channel. Also uses slotForScene, which was dead code, to reject a scene that was already adopted. Verified by compiling both CN1MacWindows.m and CodenameOne_GLSceneDelegate.m for arm64-apple-ios-macabi against the real SDK: clean with -Wall. The same file built for plain iOS exports zero CN1MacWindow symbols, confirming the whole implementation compiles out and the iOS binary is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compiling the native sources -- which the port commits never did -- turned up three defects, one of them serious. WM_CN1_DESKTOPWINDOW was defined as WM_APP + 24, which WM_CN1_WIDGET already uses. Widget ops and desktop-window ops would have been delivered to each other's handlers, both of them casting the same LPARAM to a different struct. Moved to WM_APP + 25; the duplicate is now checked for rather than assumed absent. The two COM release calls in the Windows window layer did not resolve. This port compiles its Direct2D translation units as C++ and resolves COBJMACROS-style call sites through an explicit shim in cn1_windows_comc.h, which defines only the methods the port actually uses -- and it had no Release entry for either the HWND render target or the solid colour brush. Added both, in the shim's existing style, rather than reaching around it. On Linux, the GtkWidget-typed accessors were declared in cn1_linux.h. That header is included by translation units that have no GTK on their include path, and declaring a GtkWidget* there breaks them. Moved to cn1_linux_gfx.h, which is the header that includes gtk and where the equivalent existing declarations already live. Verified with the real toolchains available here: cn1_linux_desktopwindow.c is clean under -Wall against GTK 3, and every Windows translation unit including the new one now reports zero errors of its own. The remaining diagnostics in both ports reproduce identically on master and come from compiling Linux and Windows sources on a Mac. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ault Findings from actually building and running the Catalyst app on a Mac, which no earlier commit had done. MacWindowManager never implemented capture(), so it inherited the base class's null. Every windowed screenshot test failed with "Window capture returned null". On this platform the window's content is already rendered into a mutable image, so a capture is that raster. The screenshot harness waited a fixed 1.2s on a UITimer bound to the current form. Catalyst creates its window asynchronously -- it asks the system to activate a scene and is handed one back later -- so a fixed delay is both too long on the fast ports and too short here, and the timer's bound form is not the window anyway. It now polls for the window actually being renderable, re-queuing through callSerially rather than sleeping: the paint that makes it renderable happens on that very thread, so blocking there would stop the condition ever becoming true. macNative.multiWindow now defaults to false for the sample as well. That is measured, not cautious: with multiple scenes enabled, this suite's OrientationLockScreenshotTest captures its landscape frame and then times out after 20s trying to restore portrait. Catalyst treats a multiple-scene app's windows more like Mac windows and honours orientation requests less, so the regression belongs to the Info.plist key rather than to the window code. This gives the warning already in IPhoneBuilder a concrete mechanism instead of folklore. What the run did confirm: the Info.plist key is emitted correctly, CN1MacWindows.m compiles clean under Xcode's own flags, the app boots with multiple scenes enabled and runs all 178 tests without the crash the older comment described, and MultiWindowApiTest passes on the supported path -- so a real Catalyst Window is created, registered, resolves getTopLevelContainer() to itself, reports null from getComponentForm(), reports its monitor and scale, lays out to its own size rather than the display's, and deregisters on dispose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two corrections from further runs on real hardware. The previous commit blamed multiple scenes for OrientationLockScreenshotTest timing out while restoring portrait. That was wrong. With the key still enabled the test passed in the following runs, so it was a slow-machine flake -- the machine was compiling at the time -- not a consequence of the Info.plist key. The hint stays off by default anyway, on the honest grounds that it changes Catalyst windowing process-wide and an application should opt into that rather than have it changed underneath it. The screenshot harness was also asking the wrong question. It waited for the window to report itself showing at its requested size, but a window reports the size it was asked for before the platform has actually produced anything -- on Catalyst the scene arrives asynchronously -- so both were true within milliseconds and the capture then failed. Readiness is now "a capture succeeds", which is exactly the condition the next line depends on and is correct on every port. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the Catalyst suite showed every windowed screenshot emitting a blank frame: the sizes differed correctly per window, but the content did not, and the harness reported the captures as duplicates of each other. The cause is that a window's raster exists from the moment it is shown, so a capture taken before the first paint cycle returns an empty frame of the right size rather than failing. The harness had no way to tell the two apart. Window now records when a paint cycle has completed and exposes hasPaintedOnce(), and the screenshot harness waits on that as well as on the capture succeeding. This is useful beyond the tests: any tooling that wants a window's content rather than its dimensions needs the same distinction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Built and ran the conformance suite as a Mac Catalyst app on real hardware with multi-window enabled: 0 failures across all 178 tests, and all 14 windowed screenshots captured with distinct hashes and no duplicates -- including the modal case, whose background window is non-blank while a modal is up, which is the property that proves the event loop keeps servicing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The extra macNative.multiWindow switch existed only because Mac Catalyst scenes were unverified. They are verified now -- the whole conformance suite runs as a Catalyst app with multiple scenes enabled -- so gating it behind a second opt-in only meant CI never exercised the feature. UIApplicationSupportsMultipleScenes is a process wide Info.plist key, so it is still keyed off macNative.enabled rather than set unconditionally: that key is true for the Mac Catalyst slice only and false for iPhone and iPad builds, which keeps the iOS output byte for byte identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Inspecting the Mac Catalyst captures rather than only their hashes showed three defects that distinct hashes had hidden. A Catalyst scene was never asked for the geometry the window was created with, so the system handed it the main scene's size. The window then laid out into a raster that did not match the request: several captures came out at the main display size with the window's content in the corner. The scene now requests the pending geometry as soon as it connects, and both that request and setBounds convert Codename One's pixels to UIKit's points. getBounds reports pixels to match getWidth and getHeight. The readiness probe accepted a window that had painted and could be captured, neither of which implies the size settled -- which is how the mismatch reached a golden in the first place. It now also requires the window and the captured image to be exactly the requested size, so a platform that cannot grant it fails loudly instead of baking a wrong baseline. A window used its own Window and WindowContentPane UIIDs, which no theme written before desktop windows existed defines, so it painted nothing and came up black. A window is a top level surface, so it now takes the Form, ContentPane and TitleArea styles every theme already has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Dispatch key events to the window's focused component
When f is a Window, this invokes the inherited Container.keyPressed(), because Window does not override the key handlers. That implementation only forwards to a container lead component, so ordinary focused controls receive no physical-key input, focus traversal never runs, and the listeners stored by Window.addKeyListener() are never fired. Window needs form-equivalent key pressed, released, repeated, and long-press dispatch.
ℹ️ 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".
Collaborator
Author
|
Compared 12 screenshots: 12 matched. |
The guide gate requires every source block to come from a tagged fixture under a compiled source root, so the snippets are checked by javac rather than only by eye. This chapter had them inline. Two of them did not survive the move as written: one relied on an ellipsis inside a switch and another on a call that has no declaration, so both are now complete code. Also documents the styling a window starts out with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closing one window and opening another failed on Mac Catalyst with "scene invalidated before create completion": the system does not hand out a scene session while a previous destruction is still in flight, and the window that asked was left without one. That is an ordinary sequence, so a closed window now parks its scene for the next window to adopt rather than destroying it. The size query also answered with the size that was requested while the scene did not exist yet, so a window looked correctly sized during exactly the interval when nothing was known about it. It now answers zero until there is something real to measure, and show() keeps the requested size until a port delivers a real one instead of collapsing the window to nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
…live size Three follow-ons to the origin work, all in the Catalyst port. positionSet is now carried through the bridge instead of being inferred from the coordinates. A window explicitly placed at 0,0 is placed; guessing from the numbers made it look unplaced, and the window server then put it wherever it liked. The native signature changed with it, and scripts/check-native-signatures.sh -- new from master -- confirms the Java declaration and the C implementation still agree. A move made before the scene exists is remembered. Scene activation is asynchronous, so setWindowBounds straight after show() usually finds no scene: only the size was recorded, the geometry request went to a nil scene and was dropped, and adoption then placed the window at its creation origin. The origin and the position flag are recorded on the slot, so adoption applies the move when the scene arrives. The slot's size now follows what the window actually became, including a size the user dragged it to. It only ever held the last size the application asked for, so setResizable(false) pinned the size restrictions to that and snapped a user-resized window back to 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: db9dd19bb3
ℹ️ 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".
# Conflicts: # docs/website/data/port_status.json # docs/website/data/port_status_reports/android.json # docs/website/data/port_status_reports/ios-gl.json # docs/website/data/port_status_reports/ios-metal.json # docs/website/data/port_status_reports/javascript.json # docs/website/data/port_status_reports/linux-arm64.json # docs/website/data/port_status_reports/linux-x64.json # docs/website/data/port_status_reports/mac-native.json # docs/website/data/port_status_reports/tvos.json # docs/website/data/port_status_reports/watchos.json # docs/website/data/port_status_reports/windows-arm64.json # docs/website/data/port_status_reports/windows-x64.json # scripts/copyright-header-exclusions.txt
The stored port-status reports carried the seven window tests as `not-run` placeholders. Master has since added a rule rejecting exactly that: "not-run" is the absence of evidence, and the answer to it is to run the suite and check the report in rather than to record the absence. So run it. These statuses come from this branch's own CI artifacts: all seven pass on every desktop port -- linux-x64, linux-arm64, windows-x64, windows-arm64 and mac-native -- while the ports with no windowing system report MultiWindowApiTest passing (it asserts the throw) and skip the six screenshot cases with reason `no-windowing-system`. Registers that skip in port_status_supplement.json for the six affected ports, without which the six tests are skips with no errata and the table cannot render them as a documented, supported outcome. 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: cd34a654c7
ℹ️ 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".
…op level Codex found that `addPullToRefresh()` on the default look and feel was never migrated: `DefaultLookAndFeel.drawPullToRefresh` resolved `cmp.getComponentForm()` and called `registerAnimated` on it unguarded, so the gesture threw on the EDT for anything inside a Window. Confirmed by reverting: the new test errors with a NullPointerException on the old code. The modern Material path had the same lookup guarded, which is worse in one way -- the spinner silently froze instead of failing loudly. Sweeping the remaining `getComponentForm().` chains outside the deliberately Form-only ones (MenuBar, transitions) turned up two more of the same shape: - `GenericListCellRenderer` dispatched a list entry's `$navigation` command through the form unguarded, and registered its ticker animations through a guarded lookup that is simply dead in a Window -- one of them spins on `waitingForRegisterAnimation` forever. `dispatchCommand` joins `TopLevelContainer`; it was already public with this exact signature on both Form and Window, so neither needed a change beyond the `@Override`. - `SearchBar`'s back command resolved its host as `(Form) getParent()`. Inside a Window the toolbar hangs off the title area, so that cast named the wrong type -- and since ParparVM does not check CHECKCAST, on Mac Catalyst it was a native crash rather than the ClassCastException the reverted test shows here. `showSearchBar` was migrated earlier in this branch but its dismissal path was not, so the search bar could be opened in a window and never closed. Its editor also now focuses on install, since a Window's search bar is only ever swapped in live and setEditOnShow is a Form-only deferral. Extracting `initPullToRefreshComponents()` fixes a latent NPE that SpotBugs caught once the dataflow changed: `pull` is created lazily by `getPullToRefreshHeight()`, which the `taskExecuted` path never calls. Also updates the conformance test's hardcoded test count, which master's rule about `not-run` reports left one revision behind at 178. Verified locally: 5148 core tests pass, SpotBugs reports zero findings, and the gated PMD rules and Checkstyle are clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # docs/website/data/port_status_reports/android.json # docs/website/data/port_status_reports/ios-gl.json # docs/website/data/port_status_reports/ios-metal.json # docs/website/data/port_status_reports/javascript.json # docs/website/data/port_status_reports/linux-arm64.json # docs/website/data/port_status_reports/linux-x64.json # docs/website/data/port_status_reports/mac-native.json # docs/website/data/port_status_reports/tvos.json # docs/website/data/port_status_reports/watchos.json # docs/website/data/port_status_reports/windows-arm64.json # docs/website/data/port_status_reports/windows-x64.json # scripts/hellocodenameone/conformance/test_port_status.py
… window Two codex findings, both real. **Toolbar side menus.** markInstalledOnWindow raises the `initialized` flag, so `addCommandToLeftSideMenu` gets past `checkIfInitialized` for a toolbar in a Window -- and then `constructPermanentSideMenu` and `constructOnTopSideMenu` assigned `getComponentForm()` to a local and dereferenced it, which is null there. Adding a side-menu command to a window's toolbar crashed despite the toolbar being presented as installed and usable. Both new tests throw a NullPointerException against the reverted file. The structural add stays package private: `Form.addComponentToForm` places a component beside the content pane, and putting that on `TopLevelContainer` would widen it to public for every caller. `TopLevelSupport` grows the same instanceof dispatch it already uses for the internal animation registration, and `Window` gains the package-private counterpart. Sweeping the rest of Toolbar found four more of the same shape: - the side-menu swipe listener early-returned on a null form, so swipe to open never worked in a window; - `bindScrollListener` was guarded the same way, so scroll-off-on-content- scroll never bound; - `getSideMenuCommands` read the command count off the form with no null check at all, so asking a window's toolbar for its commands threw; - `initTitleBarStatus`, which markInstalledOnWindow calls, would have added a device status bar strip to a desktop window if the theme asked for one. The remaining `getComponentForm()` uses in Toolbar are deliberate and stay: the back command is Form navigation, and the elevation host is captured before a detach. **JavaSE peer hit tests.** `getCN1X`/`getCN1Y` still converted with the global retinaScale while the peer itself is laid out with `peerScale()`. On a desktop whose monitors have different backing scales the preliminary lookup in `sendToCn1()` then tested a different point than the peer occupies, and could find an unrelated component, set `cn1GrabbedDrag` and swallow mouse input meant for a browser or other native control. The conversion moves into `CN1JPanel.toCn1Coordinate` so the math is testable without showing a real window on a second monitor -- no existing JavaSE port test makes a frame visible and this is not the place to add a flaky one. The wiring that passes the owning canvas's scale is a one-line read at each call site; its sibling on the layout path is covered by the existing aPeerIsScaledForItsOwnWindowsMonitorNotTheMainDisplay. Verified: 5303 core tests, 271 JavaSE port tests, SpotBugs zero, gated PMD zero, Checkstyle zero, and the four source gates pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
build-linux-jdk8 failed disposingAWindowReleasesItsGlobalGestureListener with "expected: <6> but was: <7>". The assertion compared the *size* of the Toolkit's wheel-listener list before and after, and that list is global to the VM while the simulator's event dispatch thread runs alongside the test -- the log shows it rendering frames in the same window. Any registration from elsewhere in the process lands inside the measurement and reads as a leak. It now records which listener the canvas registered and asserts that one is gone, which is the property the test is actually about and is immune to whatever else the VM does meanwhile. The Toolkit hands out a fresh AWTEventListenerProxy per call, so the comparison unwraps to the listener inside, which is stable. Three tests in the same class also dropped their canvases without releasing the listener. That leak is held for the life of the VM, so it compounds through every test after it; they now dispose in a finally. Verified: the full 271-test JavaSE port suite passes, and the guard assertion that the canvas registered a listener at all keeps this from passing vacuously. 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: 1eac36a3bb
ℹ️ 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 listener Two more codex findings, both right. **Geometry read back before it was applied.** setBounds queued its AWT task, so `setWindowSize()` followed by `centerOnDesktop()` or `centerOn()` in one Codename One event dispatch turn had the centring read getBounds, work out an origin from the frame's *old* dimensions and write the whole rectangle back. The later AWT task then carried the old size, and the resize silently did nothing. It now applies with runOnAwtAndWait, which is what createWindow and show in the same class already do for the same reason. minimize, restore and toggleMaximize are deliberately left queued. They go through setExtendedState, which the platform window manager applies asynchronously whatever this class does, so waiting on the AWT thread there would look like a fix without being one. **The listener assertion was still racy.** My previous attempt diffed the Toolkit's listener list before and after creating the canvas, which attributes any listener registered in between -- by the simulator's event dispatch thread, which is live alongside this test -- to this canvas, and then requires disposal to remove it. Narrower window, same failure. It now reads the canvas's own magnificationWheelFallbackListener field and asserts that exact instance reaches the Toolkit and is handed back, with no snapshot diffing at all. Verified: the full 271-test JavaSE port suite passes. 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: 42688f0d93
ℹ️ 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".
setWindowBounds -- which setWindowLocation also funnels through -- recorded the new position but left currentMonitor holding the display the window used to be on. The cache stood until the port's monitor-change callback arrived, and that callback is queued back to the event dispatch thread, so a centerOnDesktop(), getScale() or getDensity() in the same turn still answered from the old display. Centring right after moving a window to another monitor therefore sent it back to the monitor it came from. Clearing the cache lets the next read resolve the monitor from the peer, which is authoritative. On JavaSE that is now exact, since setBounds applies before returning as of the previous commit. Reverting the one line makes the new test fail with "a move must invalidate the cached monitor ==> expected: <1> but was: <0>". Verified: 5304 core tests pass, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: e5ad1c9bdd
ℹ️ 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".
…ogy events Three codex findings. **Alt+F4 stopped working on a secondary Windows window.** WM_SYSKEYDOWN and WM_SYSKEYUP were forwarded to Codename One and then returned 0, so DefWindowProcW never ran -- and it is DefWindowProcW that turns Alt+F4 into WM_CLOSE and drives Alt+Space and F10. The window was unclosable by keyboard and the native menu shortcuts were dead. They are now forwarded *and* handed on. The main window proc does not claim these messages at all, so a secondary window ends up with strictly more than the main one: the application sees the key and the operating system still behaves normally. **Terminal events reported geometry the window never had.** dispose() nulls nativePeer before firing Hidden and Disposed, and getWindowBounds() then falls back to the values the application last *requested* -- so a window the user had dragged or resized reported its original rectangle, and a listener persisting geometry across runs restored it to a position it was never left in. The fallback fields are now refreshed from the peer on a native move, on a native resize, and once more immediately before the peer is destroyed. Reverted, the new test fails with "the final native position, not the requested one ==> expected: <640> but was: <10>". **A window drag is not a change of display topology.** MONITOR_CHANGED fired Desktop.addMonitorListener, which is documented for a monitor being attached, removed or reconfigured. Dragging one window across a mixed-DPI desktop therefore re-ran whatever display reconfiguration work an application does there, repeatedly. Only MONITORS_CHANGED notifies now; the window still re-reads its own scale and lays out, and an application following one window across displays sees it through that window's Moved event plus getMonitor(). Reverted, the new test fails with "expected: <0> but was: <1>". Both new tests dispose their window in a finally. Run together against the un-fixed code the first one's abort left a window undisposed, which kept the event dispatch thread busy and timed the second one out -- burying its real assertion failure under an unrelated one. Verified: 5306 core tests pass, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: e6c6150145
ℹ️ 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".
Two codex findings, both about the on-top side menu in a secondary Window. **The menu opened on the wrong window.** Toolbar attached the backdrop to the window, but sidemenuDialog.show() went through InteractionDialog.show(), which resolves Display.getCurrent() and adds itself to *that* form's layered pane. The window dimmed while its own menu appeared over the main window, and in an application with no form at all there was nothing to resolve. This was not a small oversight: InteractionDialog was Form-bound in about twenty places, none of them migrated. They now go through TopLevelContainer -- every layered-pane method it needs is already on that interface, and the pointer listeners reach Component through asContainer(). Resolving the host needs one new public method. A dialog is attached to nothing at the moment show() runs, so unlike an attached component it cannot resolve its own top level; setTopLevelHost() supplies it. Left unset the dialog still uses the current form, which is the historical behaviour every single-window application depends on and is covered by its own test. Simulating the old resolution makes the new test fail with "the dialog must be attached to the window it was given ==> expected: <Window[...500x400]> but was: <Form[...1080x1920]>". **The backdrop left the window dimmed.** detachToolbarLayeredPane captured cnt.getComponentForm() and queued its repaint only when that was non-null, so in a window the repaint the helper's own comment calls for never ran and the shaded pixels stayed until something unrelated forced a redraw. I looked at this line in an earlier sweep and cleared it as correctly guarded; that was wrong -- guarded here means silently skipped. Note Sheet has the same Form-bound pattern and is *not* migrated here. The windowed overlay screenshot test uses the layered-pane API directly, so Sheet inside a Window has never been exercised. Verified: 5308 core tests pass, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: a23ee148db
ℹ️ 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 window Two codex findings. The first is a correction to my own previous commit. **The bounds snapshot ran after the native window was already gone.** I had moved rememberNativeBounds() to just before nativePeer is nulled, and wrote a comment claiming it ran "before the peer goes". It did not: wm.hide() and wm.dispose() are two lines above it, and every port tears the slot down synchronously inside dispose() -- Win32 through SendMessage, Linux waiting for its destroy, Catalyst memsetting the slot. The read then answered with zeros, the > 0 guard rejected them, and the stale requested rectangle survived -- the exact bug the previous commit set out to fix. It now runs before the native window is destroyed rather than before the Java reference is cleared. **Side menu geometry came from the display, not the window.** dw, the height, the edge hit tests, the drag distances and the portrait selection were all taken from Display.getDisplayWidth()/getDisplayHeight()/isPortrait() -- 21 sites. In a window that measures the wrong surface: a right-edge swipe was compared against the display's right edge, so in a narrow window it could never activate, and landscape margins could exceed the host width. They now go through hostWidth()/hostHeight()/hostPortrait(), which answer from the window when the toolbar is in one and fall back to Display otherwise, so the Form path is unchanged. The new test pins both halves of that. Note on the test: the first version asserted that the menu was no taller than its window, and passed against the un-fixed code as well -- layout re-clamps the height, so it proved nothing. It now asserts the geometry helpers directly and fails with "expected: <400> but was: <1920>" when the display-derived version is restored. Verified: 5309 core tests pass, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: f7ba2953b4
ℹ️ 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".
showPopupDialog(Component) resolved the component's top level but used it only for the formMode check and then discarded it, so the delegation below fell back to resolveHost() with the dialog still detached -- the current form. A popup anchored to a component in a window opened over the main window instead, at coordinates that mean nothing there, and in an application with no form it did not show at all. A gap in my own migration of this class rather than something that predates it. The anchor's top level deliberately overrides any host set earlier through setTopLevelHost(). The rectangle the popup points at is in the anchor's coordinate space, so showing it on a different surface is incoherent whatever was requested; precedence here is a decision, not an accident. Reverting it makes the new test fail with "a popup anchored in a window must open in that window ==> expected: <Window[...500x400]> but was: <Form[...1080x1920]>". Verified: 5310 core tests pass, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: 6aa33380bd
ℹ️ 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".
Two codex findings. **A modality change made while minimized was lost.** setModalityType released the old blocker and then guarded acquisition on nativeVisible, which is false once the platform has minimized the window, and showNotify() does not reacquire on restore. The window came back visibly non-modal while getModalityType() went on reporting the mode that was asked for. iconified now counts as live here, as it already does in isModalFinished() and hideNotify() -- a minimized window is still open and still modal. Reverting the guard makes the new test fail with "a modality change while minimized must keep the window modal ==> expected: <true> but was: <false>". **Popup placement used the display's orientation.** showPopupDialogImpl resolves its host correctly, but the flag that picks the vertical or horizontal placement algorithm was still seeded from Display.isPortrait() by both callers, so a popup in a window could open on the wrong side of its anchor. It is now derived from the host's shape when that host is a Window; a Form keeps the device orientation it was given. The orientation half has no test of its own. The placement algorithm's output is layout-dependent, and an earlier attempt of mine at a layout-derived assertion in this area passed against the unfixed code as well -- so rather than ship another assertion that proves nothing, this rests on the same reasoning as the Toolbar hostPortrait() change, which is covered by its test. Verified: 5311 core tests pass, SpotBugs zero, gated PMD zero, Checkstyle zero. An earlier run of this same code failed three StorageImageAsyncTest cases with "display-not-initialized" timeouts; the class passes in isolation, CI passes the same suite, and a re-run of the identical tree is clean, so that was a flake. 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: 4c4326cc46
ℹ️ 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".
Two codex findings. **Picker threw inside a Window.** I wrote that guard deliberately, with a comment explaining that the lightweight popup is an InteractionDialog and that InteractionDialog was unsupported inside a Window, so failing loudly beat opening over the wrong surface. That premise stopped being true two commits ago, when InteractionDialog gained an explicit host -- and the stale guard was left making a standard component unusable in every secondary window. It now resolves the top level and hands it to the dialog. The bottom-inset calculation went with it: it measured against Display.getDisplayHeight() rather than the surface the popup actually sits on. Reverting makes the new test fail with the exact reported crash, "Attempt to show interaction dialog while button is not on form. Illegal state". **The simulator inspector hit-tested the wrong form.** C.showContextMenu() resolved Display.getCurrent() in two places, and C is now shared with secondary windows: right-clicking a window inspected a component of the main form, and in a window-only application it displayed and consumed the inspection menu with nothing to inspect, swallowing the window's own context-menu handling. Both lookups go through canvasTopLevel(). Six further getComponentForm() sites remain in Picker -- traversal, key listener removal, input-device tracking, content-pane margin, animation flush and the size-changed listener. All are guarded, so they are inert in a window rather than fatal, and two of them need getNextComponent/getPreviousComponent and isFormBottomPaddingEditingMode on TopLevelContainer before they can move. Left out of this commit rather than folded in unverified. Verified: 5312 core tests, 271 JavaSE port tests, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: bc7db893ce
ℹ️ 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".
Three codex findings, all of them sites my previous commit made reachable: the popup now opens in a window, so what used to be inert became a popup that opens and then cannot be dismissed or traversed. - registerAsInputDevice resolved a Form and skipped every registration inside a window. - The Tab listener was added to the window but removed via getComponentForm(), which is null there and fell back to the current form -- a listener that was never removed, so every later Tab release ran endEditing() against the stale dialog and spinner. It now retains the top level it registered on. - Traversal resolved a Form, so the Next and Previous buttons appeared in a window and did nothing but close the popup. Traversal needed no new interface method after all: Form.getNextComponent and getPreviousComponent are Form-only, but they are defined as exactly getTabIterator(current).getNext()/.getPrevious(), and getTabIterator is already on TopLevelContainer. I said in the previous commit that these needed adding to the interface; that was wrong. Two further sites came from the test rather than from the review. With only the registration migrated the new test still failed, because isEditing() and stopEditing() were independently Form-bound -- so the fix would have looked complete while an open picker still reported isEditing() false and could not be closed. Reverting isEditing() alone reproduces "expected: <true> but was: <false>". The test also had to pump animations before asserting the stopEditing callback ran: the popup closes with a dispose animation, so the callback is queued behind it rather than running inline. Verified: 5313 core tests, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: 6f17f29c4b
ℹ️ 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".
…ir window Three codex findings. **The Catalyst main window stayed interactive under an application modal.** MacWindowManager inherited the no-op setMainWindowInputEnabled, so while the framework's event filter drops packed input before it reaches a component, a UIKit peer -- a native editor, a web view, a media control -- is handed its touches directly by the window server and never passes through that filter. Adds the native, its ParparVM bridge and the override; the main scene is found the way CN1MacMonitorForMainWindow already finds it, as the connected window scene no Codename One window claims. **A Catalyst window opened under an existing modal came up interactive.** setInputEnabled captured w->window at call time, and scene creation is asynchronous, so the request was normally delivered to nil and dropped. Unlike visibility it was not recorded, so scene adoption had nothing to apply. It is now stored in the slot and applied when the scene connects. Note the slot is memset to zero on allocation and zero here means "disabled", so creation sets it explicitly -- without that every new window would have come up inert. **Validation error popups never appeared in a window.** The focus listener compared getComponentForm() with the current Form; in a window that is null against a non-null form, so it returned every time. It now resolves the top level and treats a showing Window as showing, and hands that host to the dialog, which the emblem path needs because it shows by rectangle and has no anchor component to resolve one from. The test for it took four attempts to make honest. Driving focus through setFocused() proves nothing here: showing a window focuses its only focusable child, so setFocused() short-circuits and no focus event fires at all -- confirmed with a counter that stayed at zero. It now fires focusGained the way Window does, and reverting the guard makes it fail. The native name was hand-written, so it was checked rather than assumed: check-native-signatures.sh initially reported two fatal errors, which turned out to be the stale-build trap -- the local iOS port was built two days ago and did not contain the new method at all, so it was never checked and the errors belonged to master's SQLite natives. Rebuilt: 1057 natives, zero fatal, this one resolving cleanly. Verified: 5314 core tests, SpotBugs zero, gated PMD zero, Checkstyle zero. 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: 3357775c62
ℹ️ 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
+1146
to
+1147
| out[0] = 0; | ||
| out[1] = 0; |
There was a problem hiding this comment.
When a positioned Catalyst window has returned from show() but its asynchronously requested scene has not connected yet, this fallback reports (0,0) instead of pendingX/pendingY. In particular, a subsequent setWindowSize() reads these bounds and feeds the zero origin back through setWindowBounds(), overwriting the requested placement before adoption. Fresh evidence beyond the prior pending-move issue is that the stored origin is still discarded by this read path; return the pending coordinates here just as the pending dimensions are returned.
Useful? React with 👍 / 👎.
Comment on lines
+1080
to
+1082
| NSString* old = w->pendingTitle; | ||
| w->pendingTitle = retained; | ||
| [old release]; |
There was a problem hiding this comment.
When setTitle() runs on the Codename One EDT while CN1MacWindowSceneConnected() is adopting the scene on UIKit's main queue, this assignment and the following release occur outside g_slotLock, while adoption dereferences w->pendingTitle under that lock. Replacing the title in that window can therefore release the exact string adoption is using, producing a use-after-free or an incorrect native title; update and snapshot pendingTitle under the same slot lock.
Useful? React with 👍 / 👎.
Comment on lines
+834
to
+837
| com.codename1.ui.TopLevelContainer emblemTop = c.getTopLevelContainer(); | ||
| int surfaceWidth = emblemTop == null | ||
| ? Display.getInstance().getDisplayWidth() | ||
| : emblemTop.asContainer().getWidth(); |
There was a problem hiding this comment.
When an invalid component in a secondary Window is covered by a layered component or an InteractionDialog, the emblem glass pane still paints the validation icon over that overlay. Although this resolves emblemTop for sizing, both cover checks continue to receive c.getComponentForm(), which is always null in a window, and their null guards therefore disable occlusion testing. Generalize those checks to the resolved TopLevelContainer so window overlays hide emblems just as Form overlays do.
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 free
to 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.
Codename One has no windowing API. Even on JavaSE, Mac, Win32 and Linux, where the OS has real windows, an app gets exactly one, welded to a single global "current Form":
CodenameOneImplementationholds onecurrentForm,Display.edtLoopImplpaints one surface per tick,paintDirtyuses one global paint queue clipped togetDisplayWidth()/getDisplayHeight(), andhandleEventroutes every input event to one form. Everything that looks like a second window today —Sheet,InteractionDialog,ToastBar,Dialog— is an overlay inside the current form's layered panes.This adds real native windows, each rendering its own component tree, on all four desktop targets, without changing the single-form model mobile depends on.
API
TopLevelContaineris the shared contractFormandWindowboth implement. Its members were chosen by counting actualgetComponentForm().<method>()chains inCodenameOne/src, and every one of them was already public onFormwith an identical signature, soFormneeded nothing beyond theimplementsclause andasContainer()— a Java interface cannot extend a class, so without that bridge aTopLevelContainerreference cannot go anywhere aComponentis wanted.Window extends Container implements TopLevelContainer. Inside a windowgetComponentForm()returnsnull, by design;Component.getTopLevelContainer()is the new resolution API, and core now uses it internally.DesktopandMonitorare the public parallel toDisplayfor "what screens exist and what windows are open", including per-monitor DPI and backing scale;Displaykeeps meaning "the main app surface" exactly as before.Modality is enforced in core rather than per port, so it behaves identically everywhere:
Displaykeeps a modal stack andhandleEventdrops input to blocked windows.showModal()parks the caller throughinvokeAndBlockthe wayDialogalready does, which re-enters the event loop — so every other window stays live and repainting while a modal is up.Implementation
The impl SPI is a single
WindowManagerfacade returned fromCodenameOneImplementation.getWindowManager(). Returningnullis the capability query, so there is no separateisMultiWindowSupported()that could drift from it. Only genuinely universal operations are abstract; anything a port might not offer has a no-op default, so adding a capability later never breaks a port.Per-window paint state moves into a
PaintSurfacevalue object with the main window as instance zero;getCodenameOneGraphics(),repaint(Animation),cancelRepaintandhasPendingPaints()keep their signatures, so every existing port still compiles and behaves.paintDirty()'s body is parameterized rather than globally rebound — a global "active surface" was rejected becauseDisplay.getDisplayWidth()is public and callable off the EDT, so a live binding would change its answer re-entrantly across ~210 call sites.Events pack the window id into the type word (
type | (windowId << 8)). Window 0 is numerically identical to the previous wire format, so drag coalescing and the stack-swap logic are untouched. The port is handed the id at creation and echoes it back, so there is no peer-to-window map on the off-EDT input path.Ports: JavaSE (per-canvas graphics de-singletonization —
getNativeGraphicsused to return one shared instance, andisScreenGraphicswas an identity check against one buffer, so a second window would have drawn into the first window's pixels), native Windows (Direct2D per-window render targets,GWLP_USERDATAidentity,WM_DPICHANGED), native Linux (per-window cairo back buffer, GTK closure data), and Mac Catalyst (UIWindowSceneper window). Peer components and native text editing work in every window on every one of the four. iOS, Android and JavaScript need no port changes at all: they inherit the false capability and the throw lives in core.Latent bug fixed on the way
handleEventreturnedoffsetunchanged when the form was null, while the caller loopswhile (offset < actualTmpPointer)— an infinite EDT spin. It is unreachable today only because all nine entry points guard ongetCurrentForm() != null; window disposal with events in flight makes it reachable. It is now askipEventthat drains the packet so the rest of the batch still dispatches.Testing
Core unit tests drive a scriptable fake
WindowManageronTestCodenameOneImplementation— settable, defaulting to null, so the unsupported path is the default — covering lifecycle, paint isolation, event routing, modality including a modal window nested in a modal dialog, theTopLevelContainercontract, and a fake multi-monitor table at mixed DPI. JavaSE port tests cover the per-canvas graphics resolution, which is the riskiest edit here and had no coverage before.The centrepiece is a windowed screenshot family in
scripts/hellocodenameone: representative UI re-run inside a real window at several sizes and compared against its own goldens. A picture of a window proves nothing; layout, scrolling, graphics, layered overlays, native editing and modality rendering correctly on a non-primary surface is the actual claim. The three sizes, including a deliberately non-square one, are what prove content lays out to the window rather than toDisplay.getDisplayWidth(). This needed per-window capture on every port, since the existing pipeline can only see the main framebuffer.Mac Catalyst was built and run on real hardware for this branch rather than left to CI, because it is the hardest of the four. That found four defects compiling never would have:
capture()was unimplemented; the readiness probe was a false positive; captures were taken before the first paint; and the scene was never asked for the geometry the window was created with, so several captures came out at the main display size with the window's content in the corner.Known scope limits, documented
HTMLComponent, accessibility on secondary windows,Dialog.show()from inside a window and form transitions into or out of one are out of scope for v1 and called out in the guide.Display.getDisplayWidth()/getDisplayHeight()keep reporting the main window; components inside a window use their top level's size.🤖 Generated with Claude Code