Turn on the desktop native themes, and fix what turning them on exposed - #5861
Open
shai-almog wants to merge 62 commits into
Open
shai-almog wants to merge 62 commits into
shai-almog wants to merge 62 commits into
Conversation
Four things a desktop form needs that the framework either lacked or built and never wired up. **Tab and Shift-Tab move focus.** The traversal machinery is years old -- TabIterator, getNextComponent, preferredTabIndex -- and no key was ever connected to it, so its only callers were the "next field while editing" paths in TextEditUtil and Picker. Connecting Tab to that iterator would have been wrong, and the reason is worth recording: its filter opens with `getTabIndex() >= 0`, Component.preferredTabIndex defaults to -1, and TextArea is the ONLY class in the framework that ever calls setPreferredTabIndex(0). So the iterator holds text areas and nothing else -- correct for next-field-while-editing, and for Tab it would walk between a form's text fields and skip every button, checkbox and slider between them. The desktop order is therefore its own: same walk, same comparator shape, but filtered on focusability, which is what a pointer can reach and so what a keyboard must reach. An explicit preferredTabIndex still sorts first; zero counts as unnumbered rather than as first, because that is what setTraversable(true) writes and reading it as a position would put every TextArea ahead of its own label. Wraps at both ends so the keyboard cannot strand itself. **Escape cancels.** On a Dialog it does what the window's close control does, which was already written twice as "the back command, or dispose when there isn't one" -- now written once, as Dialog.cancel(), with both former sites calling it. On a plain Form it fires the back command and does nothing at all when there is none: Escape must never be able to exit an application. Window honours its close operation, so one that refuses the close button refuses this too. Enter needed nothing: ports already map it to GAME_KEY_CODE_FIRE and keyReleased already fires getDefaultCommand() on GAME_FIRE. Both keys are gated on isDesktop(), and the gate is asserted in both directions -- a gate nobody tests from the other side is not a gate. **desktopTitleBarMode had no reader.** All three desktop native themes have carried this constant since they landed, and nothing ever read it: Form went to Display.impl.getDesktopTitleBarMode(), which is sourced from the build hint. The constant was dead text. It cannot simply be consulted either, because that method is documented to answer a usable mode and so cannot say "nobody asked" -- it answers "toolbar" both for a port with no opinion and for a project that deliberately chose the legacy look. Hence getConfiguredDesktopTitleBarMode(), which reports exactly that distinction: build hint first, then the theme constant, then the port. JavaSEPort's static drops its "toolbar" default for null and coalesces at each reader, which also stops injectDesktopThemeConstants from writing that default over a theme's own constant. **The four interactive-scrollbar UIIDs are seeded.** LookAndFeel.initScroll picks DesktopScroll/DesktopScrollThumb and the horizontal pair when interactiveScrollBool is on, and UIManager.resetThemeProps seeded only the four mobile ones. A theme that turned the constant on without defining all four therefore drew its track and thumb out of the blank default style: an invisible scrollbar, still reserving its gutter, with nothing reporting a problem. Seeded like their mobile counterparts plus the two things the desktop bar needs and the mobile one does not -- a gutter wide enough to grab, and thumb hover/pressed styles so the highlight exists before a theme styles it. Guarded like every other seed here, so the desktop themes still suppress them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… had All three exist in every desktop toolkit and had no Codename One equivalent, which is why a desktop form built out of the existing components reads as one undifferentiated column of controls. **Separator** -- NSBox in separator mode, MenuFlyoutSeparator, GtkSeparator. Applications drew their own out of a Label with a bottom border or a fixed-height Container with a background colour. Those look approximately right on one platform and wrong everywhere else, because what differs between platforms is exactly the part they hard-code: the thickness, the colour and the air either side. All three come from the Separator UIID here -- border where there is one, background colour otherwise, margin for the air, and separatorThicknessMM for the thickness. Never thinner than a pixel: a theme may ask for a hairline, a hairline rounds to zero millimetres worth of pixels on a dense screen, and zero makes the rule invisible rather than thin. Not focusable, so the new desktop traversal skips it. The constant is read as a string like every other *MM constant (the theme format has no float accessor) and a malformed one falls back rather than throwing out of a paint. **GroupBox** -- NSBox with a title, GtkFrame with a label widget, WinUI headered content. Two UIIDs: GroupBox styles the frame and GroupBoxTitle the caption. Nothing hard-codes where the caption sits relative to the top edge, because the three platforms disagree about it; a theme that wants it inset into the edge uses a negative top margin. An empty caption hides the strip entirely rather than leaving a gap nothing explains. Adds route into the content pane, including the constrained form -- the two-slot BorderLayout underneath is an implementation detail, and BorderLayout.SOUTH from a caller means "below the other controls in this group", never "outside the box". **Stepper** -- NSStepper, NumberBox with spin buttons, GtkSpinButton. The nearest existing thing is Slider, which is a different control for a different job: a slider is for a value whose exact number does not matter and a stepper is for one where it does. Every hand-built composite got the same two things wrong, so both are handled here: the field cannot hold a number this control could not produce (out-of-range typing is corrected under the caret, and empty or unparseable text is left alone so clearing the field to retype it does not watch it fill itself back in), and the button that would step past a bound is disabled rather than accepting a press that does nothing. Fires only when the value actually moved. The value is an int, deliberately. A fractional stepper is a real control on some platforms and doing it properly needs a format, a locale and a parse policy; guessing those is worse than not offering them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…wo behaviours The three desktop native themes shipped with a scrollbar derived from the mobile one and nothing defined for any surface Codename One still draws itself. Both are fixed here, and both were the kind of gap nothing reports: the controls simply look wrong. **The scrollbar was `cn1-derive: ScrollThumb` and a transparent track.** That is a desktop scrollbar with no gutter, no minimum thumb length and no highlight, in themes whose whole job is to look like the platform -- while the MOBILE themes (ios-modern, android-material) carried the full desktop treatment. Backwards. Each theme now sizes its own gutter from the platform's figure (12px Fluent, 15px Aqua overlay, 13px Adwaita), insets the thumb inside it, and gives the thumb its hover and drag colours. Two things about the states are worth recording, both measured rather than assumed by reverting one theme and reading the compiled `.res` back: - The highlight states are `.selected` and `.pressed`, NOT `.hover`. LookAndFeel's InteractiveScrollThumb returns getSelectedStyle() under the pointer and getPressedStyle() while dragged. A `.hover` rule here compiles and is never painted. - `cn1-derive` emits the whole state family by copying the base, so the old stub produced `sel#bgColor = 8a8a8a` against `bgColor = 8a8a8a` -- a highlight that is present in the resource and invisible on screen. Worse in dark mode: derive is flattened against the LIGHT parent, so the dark thumb came out 8a8a8a, the light mobile grey, rather than the theme's own 9a9a9a. The test therefore asserts the two values DIFFER; asserting the key exists passes on exactly the defect being fixed. **Nothing defined the surfaces CN1 still draws on a desktop.** On Windows and macOS the menu BAR is the platform's own, but the right-click menu, the overflow menu, the tooltip and the dialog's command area are ours -- and the whole of GNOME's headerbar mode is. PopupContentPane, CommandList, Command, TouchCommand, Tooltip, TooltipDialog, DialogCommandArea, DialogButton and DialogButtonDefault were undefined in all three, so each fell through to UIManager's blank default: black text on white, on a dark window. Same for ToolbarSearch (written by SearchBar), the Accordion pair (seeded by the framework with a line border and phone metrics) and the Tabs trio (only the tabs* constants existed). Plus the three new components' UIIDs: Separator, GroupBox/GroupBoxTitle, Stepper/StepperField/StepperButton, and Link. **interactiveScrollBool and defaultNativeWindowModeBool are now theme constants.** Both are behaviours a native desktop theme IS, and the theme is the right seam for them: these three files install only on the desktop, so an application still on the legacy theme is untouched and no port-side isDesktop() gate is needed. Dialogs therefore open as real operating system windows by default under these themes; anchored popups never do, and the constant is ignored where there is no windowing system. Aqua adds no hover rules, which is asserted rather than remembered: AppKit restyles none of these controls on rollover, the captured reference says so, and eighteen .hover rules were already removed from that theme for the same reason. Its scrollbar knob is the deliberate exception, because NSScroller does darken -- through sel#/press#, like every other interactive thumb. Every coloured addition has a $Dark counterpart, and that is asserted too: the derive flattening above is exactly how a dark-mode rule goes missing without anything noticing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ned one Component.addContextMenuListener has been here for a long time -- it fires on a secondary mouse button, a stylus barrel button or a long press -- and nothing ever turned it into a menu. Every application that wanted one built its own popup, which is why none of them looked like the platform. ContextMenu is the popup, styled through PopupContentPane, CommandList and Command, which the desktop themes now define. Component.setContextMenuCommands is the short way in: give a component its commands and the menu opens by itself. When the commands depend on what was clicked -- which row, which cell -- a listener plus ContextMenu.show is still the way. Three details that are decisions rather than mechanics: The listener and the commands are resolved in ONE walk up the tree, not two. A row inside a table that carries its own commands must not be overruled by the table's listener merely declining to consume, and two passes would do exactly that. The buttons carry the command's NAME rather than the command itself. Button(Command) fires the command from inside its own action event, which runs it while the menu is still showing -- so a command that opens a form or another dialog would put it underneath a menu still holding the popup layer, and the menu would outlive the screen it belonged to. Here the menu is disposed first and the command dispatched after, which is the order Dialog already uses for its own command buttons. It never becomes an operating system window, even now that the desktop themes ask for that by default: setNativeWindowMode(false) per instance outranks both the static default and the theme constant. The reasons are the ones already written on Dialog.showPopupDialogImpl -- the rectangle it points at is in its host's coordinate space, a separate window would never receive the click meant to dismiss it, and it would steal focus from its opener every time it appeared. Empty means nothing opens, in both directions: setting null or an empty array removes the menu rather than leaving an empty one, and ContextMenu.show refuses to open one. A menu with no items is a rectangle the user has to dismiss to learn it was empty. resolveContextMenuOwner is the routing without the showing. The showing is a modal popup that parks its caller until the user dismisses it, so a test that asserted the routing through it would simply hang -- which is what the first version of these tests did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MenuBar.updateCommands hands the commands to setNativeCommands and RETURNS -- it draws no soft buttons, because on a platform with a real menu bar drawing them too would duplicate every command. CodenameOneImplementation.setNativeCommands is an empty method. So on a platform without a menu bar, asking for COMMAND_BEHAVIOR_NATIVE sent every command to a method that discards them and drew nothing: the application simply had no commands, and nothing anywhere in that path could tell that from "the platform handled it". Latent for as long as the constant has existed, because nothing asked for it. The desktop native themes now do -- `commandBehavior: Native` is right for the three platforms they model -- and two of the ports that will install them, Windows and Linux, implement no native menu bar at all: no setNativeCommands, no WindowManager.setCommands, no menu native source. Installing those themes without this would have deleted the commands from every Windows and Linux desktop application, silently. isNativeCommandsSupported() is the missing question, and setCommandBehavior is where it is asked. That method already normalises a behaviour the platform cannot honour -- BUTTON_BAR becomes SOFTKEY on a non-touch device -- so NATIVE becomes DEFAULT on a platform with no menu bar in the same place, and every reader downstream is fixed at once rather than one at a time. JavaSE and the iOS/macOS ports answer true on the desktop, where they really do build a JMenuBar and an NSMenu; everything else keeps drawing what it has always drawn. Form.isDesktopHideToolbar() needs the same question asked separately, because it is driven by desktopTitleBarMode rather than by commandBehavior. Hiding the Toolbar takes away the side menu, which is the only place the commands are drawn, so a port that cannot take them natively keeps its Toolbar. The title still goes to the OS title bar there, because that part works everywhere -- the result is the legacy look, not a broken one. Asserted in both directions, and the guards verified to bite by removing each one and watching the matching test fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three desktop native themes have been built, measured and gating since they landed, and no port installed one. Windows and Linux staged AndroidMaterialTheme -- a phone design language on a desktop, complete with Material ripples, a hamburger side menu and a fading touch scrollbar -- and macOS defaulted to the modern iOS theme, which is the same mistake with a different phone. That was sequencing, not oversight: the flip restyles every screen and reseeds each port's committed screenshot baselines. It was deferred until those could be reseeded alongside it, which is what this change does. Three one-line flips, exactly where the deferral notes said they were: - maven/windows/pom.xml stages WindowsFluentTheme.res - maven/linux/pom.xml stages GnomeAdwaitaTheme.res - MacOSBuildHints.getThemeMode()'s unset branch answers "aqua" The JavaSE desktop default is deliberately NOT flipped. resolveDesktopNativeTheme keeps answering legacy when nothing asks, because that default reaches every desktop application ever built with Codename One, not only ours -- and an application that wants the platform look already has desktop.themeMode and the cross-platform nativeTheme=native to ask with. The three native ports have no such history: none has shipped. hellocodenameone gains desktop.titleBar=native and desktop.interactiveScrollbars=true, which is what the Maven archetype and the initializr already put in every new project. The app's own theme.css sets includeNativeBool: true, so it inherits whichever theme the port stages with no CSS change. DesktopModeScreenshotTest used to switch desktop mode on for itself and switch it back off in done(), because it was the only screen in the suite that ran with the desktop chrome. It is not any more, and keeping the local opt-in would now HIDE a regression rather than demonstrate a feature: whatever the test turned on for itself would look right even if the suite-wide settings had stopped working. Its baseline is also the record of a real difference between the ports. macOS and the Java SE build have a menu bar, so the Toolbar is hidden and the commands move into it, out of the raster. Windows and Linux have none yet, so the Toolbar stays and draws them -- Form. isDesktopHideToolbar() will not hide the only place the commands exist. Both are correct, and the two baselines are what say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hanged COVERAGE.md's Desktop section has been a verbatim copy of the iOS section since 3959d68. That commit set out to remove a block the merge had duplicated and removed the wrong copy: what sits under "Desktop: Windows Fluent, macOS Aqua, GNOME Adwaita" today is UIButton .glass, UITabBar and UIPickerView. So the file has had no record at all of which desktop widgets are covered, which is the one thing that section exists to say. Restored from 2ceb4f0, the last commit that had it, and updated. What the update records: - Which theme each port installs now, and that the Java SE default is deliberately still legacy -- that one default reaches every desktop application ever built rather than only ours. - The constants the desktop themes declare, and why they are theme constants rather than port hooks: the three files install only on a desktop, so nothing needs an isDesktop() gate. - That commandBehavior: Native is safe on a port with no menu bar because setCommandBehavior normalises it away, which it did not before. - The native menu bar on Windows and Linux, added to the honest gap list rather than left to be discovered. README.md's Layout block listed ios-modern and android-material only, and its Rebuilding section the two themes those produce -- the three desktop ones have been missing from both since they landed. Added, along with a section on the behaviour a desktop theme declares, including the two traps this change had to find by measuring: the interactive scrollbar thumb highlights through .selected and .pressed rather than .hover, and cn1-derive on those UIIDs produces a highlight identical to the resting colour. The guide gains the keyboard conventions, the context menu, the three new components, and the note that Windows and Linux keep their Toolbar because they have nowhere native to put the commands -- which is a difference a developer will see and should not have to rediscover. Vale and LanguageTool both clean on the two chapters, checked against the committed versions first to be sure the findings were mine rather than inherited. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The desktop matrix scored nine controls, all of them form fields. Everything that makes a window look like a desktop window -- the scrollbar, the menus, the chrome -- was unmeasured, including the interactive scrollbar that is the headline feature of this whole area. Thirteen rows added: ScrollBar, Separator, GroupBox, Stepper, LinkButton, SearchField, ListRow, Tabs, Toolbar, Disclosure, MenuBar, MenuItem, Tooltip. All at the existing 240x56 tile, because the tile size is a constant in each of the three standalone capture apps and nothing here needed a bigger canvas. A Dialog row does, which is why there isn't one; it is in the gap table with the prerequisite named. The CN1 scrollbar tile is the bar and nothing else. `LookAndFeel.drawVerticalScroll` paints the theme's track and thumb across any component, so the tile is the same thing the reference apps build -- a scrolling container would put its content in the comparison too. Its hover and drag states come from OVERRIDING `isVScrollThumbHover()` and `isVScrollThumbGrabbed()`, which are the two public methods `drawScroll` asks to choose between the unselected, selected and pressed thumb styles. That renders exactly the pixels a real hover produces, with no test-only hook added to the framework and no synthetic pointer to get wrong. Three rows are not scored on every platform, and that is the honest answer rather than a gap. A reference has to be renderable into a view: - macOS cannot capture a scrollbar. Measured, not assumed: an NSScroller reports usableParts=allScrollerParts, knobProportion 0.4, isHidden=false and a 17x56 frame, and renders NOTHING through NSView.cacheDisplay -- detached and inside a real NSScrollView, in both .legacy and .overlay styles, with AppleShowScrollBars=Always already set by the capture script. The tile comes back holding one colour, the backdrop, every time. Same class of limitation as Aqua vibrancy, and cacheDisplay is the path that needs no Screen Recording consent, which a hosted runner cannot grant at all. - NSMenu belongs to the window server, so the menu rows are Windows and GNOME. - The AppKit and GTK tooltips are separate windows; a WinUI ToolTip is an ordinary Control, which is the only reason that row exists. Two rows lost a state rather than compare different things. A native expander's expanded state reveals its content, so "expanded native" against "selected-styled CN1 header" is not one measurement -- DesktopDisclosure scores its resting header. And a menu item's highlight is hover on both platforms that have one, so DesktopMenuItem uses hover like every other row rather than a selected style nothing drives. The macOS reference is verified end to end here: 92 tiles, zero blockers, every new tile reviewed by eye. That review is what found three defects the app's own blockers had already flagged -- an NSTableRowView with no intrinsic size in either axis laying out to 240x0, an NSStackView whose fittingSize had no width so the stepper showed its chevrons and no field, and a CGColor read from a dynamic NSColor freezing at the wrong appearance so the light toolbar tile was painted with the dark window background. check-fidelity-spec.py's label table gains all eight new text-bearing rows, which is the check that caught all six of the first wave rendering different strings on the two sides. Its scraper needed two fixes to do it: follow a `MakeXxx()` factory arm the way it already followed MakeComboBox, and take the first NON-EMPTY literal -- the macOS disclosure builds an empty-titled button for the triangle before the label that says "Details", and match one read the empty string. Verified to bite by drifting the GNOME group box label. No goldens yet, deliberately: they come from the runner that scores them, never from a developer's Mac, which has a chosen accent colour, a chosen appearance and custom fonts. The desktop legs will report the new pairs as missing_expected and fail until fidelity-desktop-native-ref.yml is dispatched and its output reviewed and committed -- a new row failing is correct, and is not the same as being skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither port had one: no setNativeCommands, no getDesktopTitleBarMode, no menu native source. So the desktop themes' `commandBehavior: Native` and `desktopTitleBarMode: native` both had to be normalised away there, and the commands stayed in the Codename One Toolbar. Correct, and not what a desktop application looks like. Windows gets a Win32 HMENU hung on the application window with SetMenu; Linux gets a GtkMenuBar packed above the drawing area. Both take the row format IOSImplementation.setNativeCommands already writes for the macOS menu -- "<menuHint>\t<label>\t<shortcutKeyChar>\t<shortcutModifiers>\t<commandId>" -- so the three ParparVM desktop ports share one encoding rather than each inventing its own alongside Command's placement constants. Decisions worth recording, because none of them is forced by the API: **Where About, Preferences and Quit go.** Neither platform has an application menu, so the macOS mapping does not transfer: About goes under Help and the other two under File, which is where a Windows or GNOME user looks. That is why neither hint table is a copy of the macOS one. **Win32 menu ids are not command ids.** They are 16 bit and share a space with control notifications, so the ids handed to Win32 come from a private base and are mapped back. A Codename One command id is a 32-bit counter that would start colliding with WM_COMMAND's control notifications the moment it passed 0xFFFF. The high word is also checked before the range, so a notification whose control id happens to land in the range is not mistaken for a menu item. **Accelerators are drawn by hand on Windows and by GTK on Linux.** Win32 draws nothing itself -- the text after a tab IS the accelerator display -- so a shortcut not spelled into the label would respond and show nowhere. GTK's accel group draws it and binds it in one call. **Both rebuilds are BLOCKING hand-offs to the UI thread**, and that is load bearing twice over. SetMenu and GTK are not legal from the EDT, so a marshal is required either way -- but stringToUTF8 returns this thread's scratch buffer, which the next conversion on this thread overwrites, so a posted message would read it after it had moved on. A blocking send cannot: nothing else runs on this thread until it returns. **The Linux window gained a GtkBox between the window and the overlay**, and it stays EMPTY until commands arrive. A box with one child that expands lays the overlay out exactly as it was laid out as the window's direct child, so an application that publishes no commands renders identically to before -- which is what keeps the screenshot baselines of every such app untouched. **A selection goes back through the ordinary event queue** as CN1_EVENT_MENU_COMMAND, on the same number in both ports so the two desktop wire protocols do not drift apart. That is what puts the command on the EDT rather than on the pump or GTK thread. Each port keeps one superseded generation of its command map, because a selection can be drained after the form that published it has been replaced -- without it, the command a user clicked on the way out of a screen resolves to nothing and silently does not run. The C and C++ here have never been compiled: neither toolchain exists on the machine this was written on, so CI is the first verifier. What could be checked was: scripts/check-native-signatures.sh resolves all 483 Windows and 481 Linux natives including the two new ones, and both natives sit INSIDE their file's extern "C" block -- the exact mistake this PR series already shipped once in cn1_windows_window.cpp, where the name was right, the verifier passed, and only a real Windows build reported the undefined symbol. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 12 screenshots: 12 matched. |
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
The first capture run answered two questions this could only guess at, and both answers narrow a row rather than widen it. **A WinUI ScrollBar has no highlight state a capture can ask for.** None of PointerOver, UncheckedPointerOver, CheckedPointerOver or MouseOver is a visual state of that control, and neither is Pressed or Dragging -- the capture app's blocker said so by name rather than writing six tiles identical to normal. GTK can state it: PRELIGHT and ACTIVE are what the CSS pseudo-classes resolve from, and the captured GNOME hover and pressed tiles are genuinely different from their normal one. So the resting bar is one row scored on Windows and GNOME, and the highlight is a second row scored on GNOME alone -- which is the only place all three of "the reference renders", "the reference can be put in the state" and "the state is visible" are true at once. **A WinUI ListViewItem draws its own pointer-over chrome.** It goes through ListViewItemPresenter, which paints rather than exposing a visual state, so hover is dropped from the row. Selected is a real property on all three and is still scored. Neither is a theme problem and neither is fixable by tuning, which is why both are recorded in the gap table with what was measured rather than left to be rediscovered by the next person who wonders why the scrollbar highlight is not scored on Windows. macOS and GNOME captured cleanly on that run -- 92 and 104 tiles, no blockers -- so this is the only change the run asked for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neither is reachable on a healthy run, which is exactly why they would have sat there. **Linux created a fresh GtkAccelGroup on every rebuild** and added it to the window. The menu items go away with the bar, but the groups do not: a window would end up holding one group per form that had ever published commands. Created once and reused. **Windows leaked a popup whose title failed to widen.** The popup was created before the label conversion and only appended to the bar afterwards, so a failed conversion left it allocated and unreachable -- DestroyMenu on the bar cannot reach a menu that was never appended to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Cloudflare Preview
|
Both found by scoring the CN1 side against the first CI capture and then looking at the tiles, which is the step the golden protocol asks for and the only one that catches this class of problem: the numbers were plausible, and one of them was measuring a control cut in half. **NSTabView was clipped through its own tab strip.** Left to its fitting size it is taller than the tile, so the pill came out sliced along its top edge. Full height now, like the group box. A reference that is cut in half measures nothing, whatever the number underneath says. **A selected NSTableRowView drew the unemphasized grey.** Outside a focused table that is the default, and grey is what macOS shows for a selection in a window the user is NOT working in -- not what a selected row looks like while they are. The CN1 row is correctly accent-filled, so the comparison scored a correct style against a reference in the wrong state: 67%. isEmphasized makes it the accent fill in both appearances. Neither is a theme finding and neither would have been visible from the score alone. Also corrects DesktopModeScreenshotTest's note, which said Windows and Linux have no native menu bar. They do now, so the commands leave the raster on every desktop port and the Toolbar-stays branch is asserted by DesktopChromeTest rather than by a screenshot no port produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A WinUI TabView is a document-tab control and puts a close affordance on every tab. A Codename One Tabs has no such thing, so the tile compared two tabs against two tabs plus two buttons and charged the difference to the theme. IsClosable false on both items. Found by looking at the captured tiles, which is the review step the golden protocol asks for -- the score alone would have read as "the Fluent tab styling needs work". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
112 new tiles across the three desktop sets, captured by fidelity-desktop-native-ref.yml run 35370740065 on the hosted runners -- never locally, so no developer's accent colour, appearance or installed fonts are baked in. Every new tile was reviewed by eye before this commit, which is the step that found the four reference bugs fixed in the commits above it. Set sizes now match their manifests exactly: macOS 88, GNOME 104, Windows 100. macOS and GNOME carry fewer rows than Windows, and that is the recorded per-platform scoping, not a short capture -- NSMenu and both tooltips are window-server surfaces, and AppKit cannot render a scrollbar into a view at all. Reproducibility, checked by comparing this run against the previous one: - GNOME reproduced BYTE-FOR-BYTE across two independent runs, all 104 tiles including the manifest. So nondeterminism is not a property of the suite. - macOS differed on exactly the four tiles the reference fixes targeted and nowhere else, 84 of 88 identical. - Windows differed on three tiles beyond the intended one, and all three are the documented residual: 2-3 pixels, +/-1 in a channel, on an anti-aliased edge. The README said that residual was the slider thumb; it is now measured on the tooltip's rounded border too, so it is recorded as a property of anti-aliased edges on that runner rather than of one control. One file is deliberately NOT in the Windows set. The capture also writes Button_normal_light.png, the BitBlt self-check that proves the Mica backdrop reached the window; it is excluded from tiles_written because it is not a tile, has no CN1 counterpart, and would make the golden count disagree with the number of pairs the gate can score. No baselines here. Those are recorded separately, from the runs that SCORE these, so the commit that defines the goldens and the commit that defines the ratchet stay two reviewable changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Exit 139, partway through the dark pass, several widgets after the menu bar tile had been written -- and the two runs before it completed all 104 tiles from the same binary. That shape is a deferred free: the damage is done at teardown of one widget and surfaces later. The menu bar was the only new widget handing a GObject back to GTK and then dropping its own reference. GtkPopoverMenuBar keeps the model it was built from and rebuilds its items from it while it lives, and the popovers it creates hold the submenu, so unreffing both made the lifetime depend on GTK's teardown order rather than on ours. The models are now held for the life of the process: two objects in a tool that writes a hundred PNGs and exits, against a capture that fails one run in three. That is a diagnosis from the crash's shape rather than from a backtrace, because there was no backtrace to read -- which is the second half of this change. A fatal-signal handler now prints the frames and re-raises, so the shell still sees the real signal and the job still fails, and the next occurrence says where instead of only that. backtrace_symbols_fd rather than backtrace_symbols: it writes straight to the fd and allocates nothing, which is what makes it safe from a signal handler. The build gains -g and -rdynamic, without which the frames print as bare addresses. If it recurs the handler will say so plainly; it will not be quietly retried. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
MacOSStubThemeTest required the generated stub to call setIosMode("modern"), and the theme
flip broke it -- correctly, since the default is aqua now. But the literal was never what
the test was guarding. Its own comment says so: the point was that iOS7Theme.res declares no
@darkModeBool, so a stub defaulting to it leaves an application with no dark mode however
carefully it asks, and every dark screenshot comes out light. "modern" was simply the theme
that happened to satisfy that.
So it is now two tests. One pins the value, because the value is a decision worth pinning.
The other names the property -- the default is never the theme with no dark mode -- so the
next person to move this default is told what it has to keep rather than reading a literal
and guessing. Aqua declares the constant too, which DesktopNativeThemeContentTest asserts
against the compiled resource rather than the CSS.
The flip also made a latent hazard load bearing, so that gets a test as well.
installNativeTheme() asks for its resource by name at run time and falls back to the legacy
theme WITHOUT SAYING SO when the lookup returns null -- the file's own comment warns about
it. While the default was modern, iOSModernTheme.res was the only theme anyone staged, so
nothing could go wrong; with the default on aqua, a MacOSAquaTheme.res that fails to reach
buildinRes is a silent revert to the iOS 7 look on every macOS build. stageThemeResources
copies every .res it finds, so the flip is safe today, and now something says so.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 172 screenshots: 172 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 157 screenshots: 157 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 172 screenshots: 172 matched. Benchmark ResultsDetailed Performance Metrics
|
The desktop themes set defaultNativeWindowModeBool, and the Windows port's screenshot suite
then captured two of LightweightPickerButtons' four placements, found the two it did capture
byte-identical, and timed out waiting for the rest.
The lightweight Picker positions its popup by hand -- setX/setY/setWidth/setHeight, then
show(top, bottom, left, right) -- and native window mode documents exactly those margins as
ignored. So in a window the popup comes out centred and every placement variant collapses
onto the same picture.
This is a bug in the native-window-dialog feature rather than in the theme default: an
application calling Dialog.setDefaultNativeWindowMode(true) hit it just as hard. ComboBox and
InfiniteProgress had already opted out by hand, so the hazard was known -- it had simply
never been reachable, because nothing defaulted the mode to true. Six more sites had not:
TooltipManager, both Toolbar side menus, both Picker popups, Validator and
FloatingActionButton.
There is no choke point to fix it at. Dialog.usesNativeWindow() already exempts menus and
anchored popups, but AbstractDialog is an interface whose own comment forbids new members --
the core is Java 5, so a new method breaks every implementation and throws AbstractMethodError
on the compiled ones. So each site says so itself, and a test enumerates them.
That test reads SOURCE, and the first version of it did not:
The obvious runtime test walks the current form looking for a popup that wants a window.
It passed with every fix removed. Reaching one of these popups means pressing the control
that owns it, which parks the caller, so the popup does not exist while the test can look.
A probe that cannot fail proves nothing.
What it counts is constructions against opt-outs, per file, because the bug that started
this was the SECOND of a pair being missed -- and it immediately found a third: Validator
builds two, and the one I had fixed was not the field initialiser that is used until a
constraint supplies its own message.
It also strips comments first, because BubbleTransition shows a dialog in a javadoc EXAMPLE
and counting that reported a file with no dialogs in it as an unclassified builder. A check
whose findings have to be filtered by hand stops being read.
A second test fails when a new framework source starts building one of these and is in
neither list, so the classification stays a decision rather than a default. Seven existing
sources were classified by it: the file choosers, crash reporter, signature pad, share sheet
and country list are dialogs the USER operates and correctly take the default, and MenuBar is
exempt by mechanism because usesNativeWindow() already refuses a window for a menu.
Also lands the fidelity baselines from run 35371376318 -- 86 macOS, 102 GNOME, 98 Windows
pairs, all with geometry, means 84.0 / 83.8 / 85.3 against 84.6 / 85.9 / 82.1 for the
nine-row matrix. And drops DesktopSeparator, because the gate refused to baseline it and was
right to: a 1px hairline a few levels off the surface does not clear the comparator's content
threshold, so the tile is 98.2% backdrop holding exactly TWO colours, geometry comes back
{"empty": true}, and shape and size agreement reach 1.0 because two empty masks agree
perfectly. Windows light scored 96.45% on a comparison that had looked at nothing. The
component is still themed and still asserted; it is the overlay comparison that cannot see it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
Apple Watch (watchOS / Core Graphics)Compared 223 screenshots: 221 matched, 2 updated.
|
…open Found by looking at a captured screenshot rather than at a score. TabsTheme_light on the Linux port came back as three plain boxes with hard borders and no indication whatever of which tab was open -- and the DesktopTabs fidelity row had scored 35-68%, the worst in the set, which I had written down as "CN1's tabs do not look like a native tab control". That diagnosis was wrong. Tabs writes the `Tab` UIID onto every tab button and marks the open one with that button's own SELECTED style. These themes styled `SelectedTab` and `UnselectedTab`, which nothing in the framework writes at all -- I added them on the assumption that the names meant what they say. Dead rules. With nothing styling `Tab`, the strip fell through to UIManager.resetThemeProps, which seeds `Tab.sel#derive: Tab`. That seed makes the selected tab derive from the unselected one, so the two are pixel-identical by construction. A tab strip that cannot show which tab is open is a usability defect, not a fidelity gap, and no amount of tuning the score would have found it. `Tab` is now styled per platform, with the selected treatment each one actually uses: a card fill on Fluent, the accent pill on Aqua, an accent rule under the label on Adwaita. Plus `TabbedPane`, `TabsContainer` and `TabsContainerHost`, which `Tabs` also names and which no desktop theme defined either. The test asserts the DIFFERENCE, not the key, and for the same reason the scrollbar highlight does: `Tab.sel#` is present in every compiled theme whether or not anything styled it, because the framework seeds it. Present-and-equal IS the defect. Verified to bite by setting the selected rule back to the base colours and watching it fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 150 screenshots: 150 matched. |
Its javadoc says the golden "shows the dialog centred on the window with the window's content dimmed behind it, which is only possible if both are on the same surface" -- that is the HOSTED path: the window's layered pane, the scrim, isTopmostHostedDialog, a real amount of code. The desktop themes set defaultNativeWindowModeBool, so the dialog now opens as a separate operating system window, leaves the captured raster entirely, and the golden becomes an empty host window. Correct behaviour for that mode, and no longer a test of anything. Found by scanning the reseed captures for screens that had gone uniform rather than by reading a diff: Window-Dialog-900x700 came back 99.7% a single colour, and the control run over the committed baselines showed it had not been near the top of that list before. The other Window-* screens were already sparse for their own reasons, which is why the control mattered -- high uniformity is normal in this suite and only the CHANGE is a signal. Pinned to the hosted path, so it keeps covering the code it was written for. An application can still ask for that mode, so this is a supported configuration and not a workaround. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scoring the macOS leg after the Tab styling landed separates two things the single 35-68% number had been hiding. The DEFECT is fixed: the selected tab is now visibly selected, and the light row moved 68 -> 82. Confirmed by looking at the tiles rather than by the number -- CN1 now draws a filled "One" against a plain "Two". What remains is SHAPE, and it is not a tuning problem. CN1 draws a left-aligned row of tabs; AppKit draws a centred rounded pill inside a grey track. The dark row is still 49%, and the geometry moved FURTHER from native -- height ratio 0.53 -> 0.25 -- because the styled tab is shorter than the unstyled full-height box it replaced. Recorded as wanting a per-platform tab shape rather than more colour tuning, so the next person does not read the improved light score as the row being done. The run also reported seven sub-1% drops on TextField and GroupBox and a handful of AccentButton width drifts. Those are NOT acted on: the baseline was recorded on the CI runner and this scoring was local, which is the disagreement goldens/README.md already documents in the other direction -- a Mac-recorded baseline failed the gnome gate on eighteen pairs over a slider one pixel taller on Linux. A sub-1% delta between a local render and a runner-recorded baseline is that, not a regression. The baseline re-record itself therefore waits for the dispatched CI scoring run, from the runner that scores it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CI scoring run confirmed the Tab fix is real and large -- GNOME dark 35.73% -> 73.09%, light 64.67% -> 75.75%, with no score regressions anywhere. It also showed the geometry moving the wrong way: width ratio 1.0 -> 0.64 and the centre offset 12.5px -> 43.6px. That is a consequence of the fix rather than a separate bug. A Tab is transparent until it is selected, so once the tabs were styled the only content in the tile was the selected tab's fill and the two labels -- the comparator's bounding box shrank to the part that paints. GtkNotebook and a WinUI TabView both draw a divider under the tab strip, and neither theme had one. Adding it fixes the look and the measurement together, which is the only kind of change worth making to a geometry number: drawing the line the platform actually draws, not padding something out until a ratio improves. Aqua deliberately gets no such rule. NSTabView's pill sits on the bare window background, and for that theme a narrower bounding box is CORRECT -- its native reference is a centred pill, not a full-width strip, so the ratio moving away from 1.0 is the CN1 side getting closer to the reference rather than further. Worth recording from the same run: the seven sub-1% "regressions" my LOCAL scoring reported on TextField and GroupBox do not appear on the runner at all. That is the local-versus-runner disagreement goldens/README.md documents, measured from the other side this time, and it is why the baseline re-record has to come from the runner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… for it Both loops exit as soon as the number of PNGs on disk reaches EXPECTED, and EXPECTED counts the GOLDENS in the reference directory. Any run that captures more screenshots than there are goldens therefore reaches the threshold while earlier captures are still arriving -- and that is every run that adds a test, because the new one has no golden yet. The directory is snapshotted mid-stream and everything that had not landed is reported as "Actual screenshot missing (test did not produce output)", naming tests that ran perfectly. Measured on tvOS run 35395173010: 22:34:33 [cn1ss] Test 'DesktopMode': Actual screenshot missing (test did not produce output). 22:34:33 [cn1ss] Test 'Media360Panorama': Actual screenshot missing (test did not produce output). 22:35:04 [cn1ss-ws-server] test=DesktopMode png_bytes=163304 status=ok 22:35:04 [cn1ss-ws-server] test=Media360Panorama png_bytes=403545 status=ok Both were delivered, intact, thirty seconds after being declared missing. Six new captures with no goldens had pushed the count over EXPECTED six screenshots early. This is also a bootstrap trap, which is the part worth fixing rather than working around: a new golden cannot be seeded on these two ports, because the wait ends before the capture that would seed it arrives. The count now has to have STOPPED CHANGING as well as reached EXPECTED. While captures are still streaming it keeps rising and the condition never fires; once it plateaus, the existing two confirmations give the final writes their flush window. Costs at most one more eight-second poll on a normal run. Both scripts, because both had the identical loop and a fix in one would have left the other reporting the same false missing screenshots. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six at 3840x2160 from run 35395173010. Same profile as every other port -- chrome ~4000 colours, widgets ~1650, scrollbar ~770 at 94% one colour -- so none is a blank capture. tvOS renders these through the iOS-derived theme, as Catalyst does. They are committed for the same reason: this suite gives every port a golden for every test it runs, which is why DesktopModeScreenshotTest has one in all seven directories rather than only the desktop three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
161 captures: the 155 the run produced plus the six new desktop tests. Held back through three earlier rounds because ten animation filmstrips were capturing six empty cells, which would have been reseeded as the new truth. They are not any more: AnimateLayout [665, 823, 822, 837, 858, 866] distinct colours per cell SmoothScroll [1106, 1364, 1208, 1372, 1391, 1360] StickyHeader [1225, 1474, 1358, 1342, 1384, 1362] ... all-blank filmstrips: 0 (was 10) AnimateUnlayout ends [665, 734, 602, 542, 541, 518] rather than climbing, and its Linux and Windows goldens do the same thing -- [43, 37, 21, 9, 1, 1] and [7, 6, 5, 2, 1, 1]. An unlayout animation empties the screen, so its last frames are legitimately blank. That is why the guard added with the fix requires ALL SIX frames flat and not any one of them: a per-frame rule would fail these three tests on every port forever. Five goldens are deliberately NOT replaced. The macOS artifact carries only captures that failed comparison -- cn1ss deletes a passing one outright -- so BrowserComponent, LottieAnimated, MotionShowcase, SVGAnimated and VideoIODecodedFrames are absent because they PASSED. Each was resolved to its test in port-status-macos.json and confirmed "pass" before being left in place; absence alone is still refused. Reviewed by comparing every changed file against its predecessor for the failure this suite produces. Twelve looked suspicious and all twelve are explained, none by "it is probably fine": nine graphics-* tiles lost ~200 colours because their antialiased TITLE moved into the OS window under desktop native title-bar mode while the drawing itself is pixel-identical, and MainActivity, RichTextArea and PullToRefreshSpinner read flatter only because Aqua's background is a large flat field -- all three still carry their content, at 751-936 colours per cell in PullToRefreshSpinner's case against the old golden's 1020-1203. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six each from run 35395173010, at the two ports' own geometries -- 1179x2556 for the phone, 416x496 for the watch. Healthy on both: chrome ~4500 / ~1550 colours, widgets ~1700 / ~1100, scrollbar ~830 / ~255 at 90% one colour, which is right for three thin thumbs on a plain field and not a blank capture. These are the last two of the eight golden directories the three tests reach. Only Android is left, and only because its captures could not be retrieved from CI at all until the upload fix earlier in this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
FadeTransitionTest came back from run 35407477083 as: failed: FadeTransitionTest produced 6 frames and every one of them is a single flat colour. The animation host painted its background and none of its children. That is the guard added with the layout fix, firing on a case the fix itself missed, on the first run after it existed. Worth stating plainly: the sweep was wrong and the check caught it, which is the entire reason the check is there. The sweep was wrong in a specific and avoidable way. It looked for files calling BOTH setWidth and layoutContainer, on the assumption that a broken call site would have a layoutContainer to correct. AbstractTransitionScreenshotTest never calls layoutContainer at all -- it builds two off-screen forms, sizes both with the raw setters and relies entirely on the incidental invalidation -- so it matched neither half of the pattern and was invisible to a grep shaped that way. Enumerating every file that calls setWidth and looking at what each one does with it finds it immediately; that is the enumeration this should have been from the start. Thirteen tests share this base. Only FadeTransitionTest went fully blank because paintBookendDirectly draws the first and last frames outside the form path, so the others kept some content and stayed under the all-six-frames threshold -- they were rendering four empty middle frames and would have gone on doing it. Both forms get layoutOffScreen, after their content is built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t reach Six at 320x640 from run 35407476926 -- the first Android run after the upload fix, and the first time these captures have ever left a runner. Before it the Android leg wrote them to artifacts/ and uploaded only the logs, so the set was unreachable from CI by construction. Healthy on the same measures as every other port: chrome ~1830 colours, widgets ~1000, scrollbar ~582 at 93% one colour. That completes ten of the eleven golden directories. The last is scripts/ios/screenshots, the OpenGL lane, which deliberately does not run on pull requests -- it costs ~40 minutes on a serialised macOS chain -- but does run on master pushes and nightly against 143 live baselines. Merging without its goldens would turn master red, so it has been dispatched explicitly rather than discovered after the fact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate checks added and MODIFIED files, and this one had no header -- like the nine in the earlier sweep, it predates the rule. Missed locally because the check ran before git add: --base/--head compare commits, so a change still in the working tree is not in HEAD and is not examined. Run it after committing, or the answer is about the previous commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…on itself The old golden showed no Toolbar at all. The new one shows the hamburger and the title, which is the correct rendering for this port and the direct consequence of a change already made in this branch: DesktopModeScreenshotTest used to switch desktop mode on for itself and off again in done(), so its capture showed desktop chrome on every port regardless of what that port actually reports. It does not any more -- the suite-wide settings decide, and keeping the local opt-in would have hidden a regression in them rather than demonstrated a feature. Mac Catalyst answers CN.isDesktop() == false: it is an iPad application hosted on a Mac, not a desktop port, and none of the three ports that install a desktop native theme is Catalyst. So the Toolbar stays and its commands live in the side menu, which is exactly what the test's own class comment says a non-desktop port must show. Not the same change as the JavaScript port's DesktopMode, despite the shared name: there the difference was a 30-pixel strip down the right edge and nothing else, because the JavaScript port keeps its Toolbar too and only gained the desktop scrollbar. Here the whole frame moves, because the toolbar itself comes back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The twelve tests sharing AbstractTransitionScreenshotTest all moved, and only on macOS. That is the shape the whole layout bug has had throughout: the incidental invalidation these captures relied on came from attaching the Toolbar, macOS is the port that hides it under desktop native title-bar mode, and the other ports never lost it. Linux, Windows and Android all passed this same round without a single transition golden changing. What changed is content arriving where there was none. Per grid cell, before and after: FadeTransition [327, 449, 456, 455, 475, 521] -> [645, 695, 755, 768, 794, 838] SlideHorizontalTransition [327, 451, 458, 456, 477, 521] -> [645, 804, 807, 871, 881, 838] FlipTransition [327, 437, 459, 259, 475, 521] -> [645, 770, 792, 423, 794, 838] Every cell gains, and the minimum across all twelve is now 319 distinct colours with no all-blank frame anywhere. These twelve were not caught by the blank-filmstrip guard and could not have been: only FadeTransition went fully blank, because paintBookendDirectly draws the first and last frames outside the form path, so the other eleven always had two real frames and sat under the all-six threshold. They were rendering four empty middle frames and their goldens recorded it. The guard is deliberately not tightened to catch them -- a per-frame rule would fail AnimateUnlayout and its two siblings on every port forever, since an unlayout animation legitimately ends blank. Seeded from run 35409267568, the run that reported them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… last set Six at 1179x2556 from the dispatched run 35407477609. Healthy on the same measures as the other ten: chrome ~5000 colours, widgets ~1800, scrollbar ~520 at 90% one colour. This set exists only because it was gone looking for. The OpenGL lane deliberately does not run on pull requests -- it costs about 40 minutes on a serialised macOS chain and Metal is the default backend -- but it does run on master pushes and nightly, against 143 live baselines. Merging without these six would have turned master red on the first push after the merge, with nothing on the PR having ever said so. The lane reported exactly three failures, all of them these missing references, and nothing else. FadeTransitionTest passed here, which is the expected answer: the off-screen layout bug needed a port that hides its Toolbar under desktop native title-bar mode, and iOS is not one. All eleven golden directories now carry the three new tests: linux/screenshots, linux/screenshots-arm, windows/screenshots, macos/screenshots, mac-catalyst/screenshots, javascript/screenshots (12 -- two themes), ios/screenshots, ios/screenshots-metal, ios/screenshots-watch, ios/screenshots-tv, android/screenshots Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The twelve transition tests moved on tvOS too. I said earlier that only macOS moved; that
was premature -- Linux, Windows and Android had reported and tvOS had not.
It is the same fix with a different visible result, and the difference is worth recording
because "nothing appeared" makes it look like noise. The colour count per grid cell is
unchanged, in two of the three sampled tests to the exact number:
FadeTransition old [1303, 1182, 1359, 1343, 1449, 1495]
new [1303, 1182, 1359, 1343, 1449, 1495]
but ~5% of pixels differ with a delta near full range, in a band from y=42 to y=1629. Same
palette, moved content: a layout shift, not antialiasing.
That is what the fix does here. These captures size a form with the raw setters and paint it
into a frame; without the invalidation the form stayed laid out at the DISPLAY size, which on
this port is 3840x2160, and was painted into a frame a fraction of that. It now lays out at
the size it was given. macOS lost its children entirely because the whole content pane was
mis-sized; tvOS kept them and drew them to the wrong proportions. Same cause.
Checked by eye before reseeding rather than accepted on the diff numbers: the structure is
identical -- title, row, action bar, body -- with the geometry corrected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…l invalidation Same twelve tests, same cause, and now the pattern is complete enough to state: moved macOS, tvOS, Mac Catalyst unchanged Linux x64, Linux arm64, Windows, Android, JavaScript, iOS GL, watchOS The ports that moved are the ones whose off-screen host Form was not being invalidated by something else. The rest were already getting the layout by accident and forceRevalidate is a no-op there -- which is the strongest evidence available that the fix restores a layout rather than inventing one: seven golden sets did not shift by a pixel. Catalyst's shift is the smallest of the three, 2.09% of pixels on FadeTransition and 0.33% on MorphTransition, with per-cell colour counts within a few of their previous values. macOS lost its children outright, tvOS drew them at display proportions into a smaller frame, and Catalyst -- whose display and frame are closest in size -- is off by the least. The size of the symptom tracks the gap between the display and the frame, which is what a mis-sized layout predicts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…aches iOS Metal moves too. The classification in the previous two commits was built on whichever legs had reported at the time and was twice too narrow; this is what the evidence supports now, with every port that has run the fix accounted for: moved macOS, tvOS, Mac Catalyst, iOS Metal unchanged Linux x64, Linux arm64, Windows, Android, JavaScript, watchOS unknown iOS OpenGL Metal's per-cell colour counts are identical to the old ones -- 1536, 1430, 1616, 1619, 1667, 1801 on FadeTransition, the same six numbers -- while 11.19% of pixels differ. Same palette, moved content, which is the tvOS signature and not the macOS one. iOS OpenGL is listed as unknown rather than unchanged, and that is the part with a consequence. Its dispatched run finished BEFORE the transition fix existed, so its clean result says nothing about the fix; its twelve transition goldens will move on the first master push after this merges, on a lane that does not run on pull requests. It needs a second dispatch against a head that has the fix, and those goldens seeded, before this is safe to merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 154 screenshots: 154 matched. Benchmark Results
Detailed Performance Metrics
|
…fter merge Twelve, from the second dispatch (35414496010) against a head that actually has the layout fix. Exactly the twelve transition tests and nothing else, which is the answer the first dispatch could not give: it ran before the fix existed and reported clean, and clean there meant only "these goldens match the old rendering". This is the whole reason the lane was dispatched by hand. It does not run on pull requests, so its twelve would have moved on the first master push after the merge, on goldens no PR check ever looks at. The failure was predicted from the fix and confirmed by dispatching for it rather than discovered afterwards. Same signature as Metal, tvOS and Catalyst -- per-cell colour counts within a few of their old values, 12.88% of pixels moved on FadeTransition and 1.91% on MorphTransition, nothing blank. Final classification, every port now measured against a head carrying the fix: moved macOS, tvOS, Mac Catalyst, iOS Metal, iOS OpenGL unchanged Linux x64, Linux arm64, Windows, Android, JavaScript, watchOS In the same dispatch, build-ios-metal, build-ios-tv and build-ios-watch all passed, which is those three reseeds confirmed on a clean run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 166 screenshots: 166 matched. Benchmark Results
Detailed Performance Metrics
|
…pture Run 35431774593 is the first GNOME reference capture since the dangling widget pointer was fixed. It completed, wrote all 104 tiles, and produced a capture-manifest.json that parses with no blockers -- where the previous run wrote its tiles and then died inside write_manifest, leaving a truncated file. That is the fix verified rather than assumed. The tiles are what a separator reference should be: 240x56, two colours, a 240-pixel rule. light #fafafa background #dddddd rule dark #242424 background #454545 rule This closes a gap with the same shape as the OpenGL one, and it is worth naming because it is not obvious: scripts-fidelity-desktop.yml scores on PUSH TO MASTER and on dispatch, never on pull requests. gnome-adwaita was the only set missing a golden for a row the yaml declares, so the first master push would have scored a declared row against nothing -- green PR, red master, on a suite no PR check runs. One measurement to act on separately: the theme declares Separator color #d8d4d0 and GTK actually draws #dddddd. Close, not equal. Left alone here deliberately -- Separator is rendered inside DesktopWidgetsTheme on the Linux screenshot set, so changing it means rebuilding the .res and reseeding screenshot goldens, which is a change with its own blast radius and does not belong in a commit whose job is to add a reference. The number is now measured and recorded, which is what the row was added for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@codex review |
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. |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 8701d4eb57
ℹ️ 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".
…nged From the review on #5861. Every one was checked against the code before being accepted -- a finding is a claim, and implementing a fix for a bug that is not there is its own defect. All eight held up, and each now has a test that fails without its fix. **One Escape popped two screens (P1).** Escape is not merely a desktop convention, it IS the back key: JavaSEPort.getBackKeyCode() returns VK_ESCAPE and Display.init assigns that to MenuBar.backSK. So the press ran the back command through the new desktop path and the release satisfied menuBar.handlesKeycode(backSK) and ran it again. The release is now suppressed when the press was consumed. The flag is static rather than per form because a Dialog disposes on the press and the release is delivered to its OWNER -- which is exactly the case where the second back lands on the wrong screen. **Native menu selections skipped the form (P1).** They called Command.actionPerformed directly, so Form.addCommandListener listeners, an actionCommand() override and the pop guard never ran. Survivable while the Toolbar was still on screen and the same command was reachable through it; this PR hides the Toolbar, which makes the native menu the only route. Now dispatched through the owning Form. Fixed on macOS/iOS too, not just the two ports added here -- that path had the same gap and this PR turns Aqua on by default, so it now matters there as well. **Disabled commands were still activatable from a native menu.** The row format carries no enabled flag, so the native item is created enabled. Extending the format means a sixth field and a matching change in three native parsers; the guard that refuses to run a disabled Command is the half that prevents the damage, and is in all three ports. The item still looks enabled -- recorded in the code rather than left implied. **JavaSE could lose every command.** Form.getDesktopTitleBarMode consults the theme's desktopTitleBarMode when the project configured nothing, but JavaSEPort resolved "toolbar" regardless. A theme asking for native chrome therefore hid the Toolbar while setNativeCommands returned without installing a menu. Both now read the same effective mode. **Stepper: three defects.** It fired a change event when clamping left the value where it already was (10 in a 1..10 stepper, typing 11), against the rule setValue itself follows. Its button arithmetic overflowed in int before clamp could act, so incrementing near Integer.MAX_VALUE jumped to the bottom of the range -- now saturating in long. And a negative range was untypable, because TextField.validChar answers digits only for NUMERIC; the minus is now accepted, and only when the range contains a negative value. DECIMAL was not the answer: it also admits '.' and ',', which this control cannot represent. **ContextMenu opened an empty modal for an all-null array.** Length was checked, content was not, while build() skips nulls -- so a rectangle the user had to dismiss to discover it was empty. The class already documents that an empty menu opens nothing. **The fallback scrollbar had three identical states.** sel and press both derived from the unchanged base, so the highlight the fallback exists to guarantee never appeared. Opacity rather than colour, because `foreground` there is pure black in light mode and pure white in dark -- "darker on hover" has nowhere to go. **GroupBox(null).getTitle() returned null** despite the method documenting otherwise; setTitle(null) already normalised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… a wait that was still wrong **Shift-Tab did nothing on Windows and Linux.** Form.moveFocusByTab asks Display.isShiftKeyDown for the direction, and neither port overrode it -- they inherited the core's `return false`, so Shift-Tab traversed forwards exactly like Tab. Only JavaSE and the macOS port reported modifier state. Both now expose a currentModifiers native using the bit values the macOS port already uses (1 shift, 2 control, 4 alt), so the Java side of all three desktop ports reads one encoding. The two natives are asked rather than pushed with the event, but for different reasons, and the difference is the interesting part. Windows calls GetKeyState, which answers for the message being processed. Linux latches the mask from the GTK key event, because GDK carries it on the event itself -- which is sufficient here: Shift-Tab asks whether Shift is held while handling the Tab, and the Tab event carries that. Neither tracks a modifier pressed in isolation, and nothing asks. check-native-signatures.sh resolves both: 484 natives against 524 C definitions on Windows, 482 against 526 on Linux, exit 0. A wrong name there compiles, links, and ships the feature inert, so this is checked rather than assumed. **Windows menu accelerators were decoration.** appendAccelerator writes "Ctrl+S" into the label and Win32 draws that text and nothing else; the pump has no accelerator table and never calls TranslateAccelerator, so the advertised shortcut did nothing. Matched in WM_KEYDOWN instead of building an HACCEL, which would have to be rebuilt and destroyed alongside the menu and threaded through the pump where this needs one call. An exact modifier match is required and a bare keypress can never match, so typing is untouched. Linux needed nothing: it already registers a real GtkAccelGroup, so GTK both draws the accelerator and responds to it. The finding was Windows-only and it was right about that. **The tv/watch capture wait was still exiting early.** My earlier fix made the count-based exit require a stable count, which is not enough: EXPECTED counts GOLDENS, so any run that captures more than there are goldens reaches it with captures still in flight, and this script says in as many words that trailing tests run for minutes without producing a PNG. A sixteen-second plateau is not evidence of anything -- measured on tvOS run 35395173010, two screenshots arrived thirty seconds after the comparison had already run. The suite's own CN1SS:SUITE:FINISHED marker is now the primary signal, confirmed twice for a drain window, with the count demoted to a backstop that needs an eighty-second plateau before it is believed. That is what the review asked for and it was right. **Not fixed, recorded in the code instead:** Tab does not reach this handler while a native text editor holds the keystroke on Windows or Linux, so tabbing between text fields -- the commonest desktop case -- still traverses nothing there. Closing it means intercepting Tab inside each port's native editor and committing before forwarding. That is surgery on the text-input path of two ports, neither of which can be exercised from here, and that path has a history of swallowing every keystroke when it is got wrong. It is left for a change that can be run on both rather than written blind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44255c2ae9
ℹ️ 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 more Five findings, three of them on changes made in the previous round. That is the review doing its job, and the menu one is a fix that looked right and was not. **The native-menu owner was the wrong form (P1).** Last round routed native menu selections through the owning Form, capturing it with Display.getCurrent() when the menu was published. That form is the wrong one. MenuBar publishes from addCommand/removeCommand -- verified, it is the only caller -- which runs while a form is being BUILT, before show() makes it current, and showing it does not republish. So the captured host was the previous form, or null, and the routing this was meant to restore still did not happen. Resolved at selection time instead, where the form whose menu is on screen is simply the current one, and confirmed rather than assumed: a command the current form does not carry is run directly rather than dispatched through a form it does not belong to. Moved into CodenameOneImplementation.dispatchNativeMenuCommand as one choke point for all three desktop ports, along with the disabled-command guard that was duplicated three ways. The Windows, Linux and macOS paths now call one method. **Escape ran the back command before the pop guard could veto it (P1).** The guard exists to stop a pop before it happens; consulting it afterwards means the veto lands once the command has navigated away or discarded the state it was guarding. MenuBar's hardware-back path gets this right, and on the desktop Escape IS that key. escapePressed now checks first and carries ONE event through the dispatch, so a command that consumes it suppresses the form-level routing -- building a second event meant consumption could never be seen there. **An all-null context-menu array was treated as a menu.** ContextMenu.show learned to reject one last round; the component path did not, so a right click was consumed, nothing opened, and an ancestor that did have a menu never got to answer. Filtered when stored, so `length > 0` means what it says at every use rather than at one of them. **Separator painted over themes that draw the rule with a background.** Android Material and iOS Modern both define the Separator UIID through background-color and set no foreground, and the component is exactly as thick as the rule -- so filling with the foreground repainted the whole visible area in the text colour. An opaque background is now left alone, the same rule the border case above it already follows. **A Stepper could be left showing "-".** Permitting the sign so a negative range is typable means the field legitimately holds "-" mid-edit. When editing ended, parsing failed and the method returned, so the control showed "-" while getValue() reported the old number for as long as the form was up. The edit-completion path now restores the value; the data-change path still tolerates it, or typing "-5" would be undone at the first keystroke. Every one was reproduced before being changed: reverting all four testable fixes fails the four new tests with exactly the symptoms described -- "expected <3> but was <->", "expected <null> but was <[null]>", and the guard and consumed-event cases each firing once when they must not fire at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
UnnecessaryFullyQualifiedName, seven times, all in the block added last commit. CodenameOneImplementation already imports Command, Display, Form and ActionEvent; the new code was written with the fully qualified names the PORT files use, where they are needed because those files import none of this. Verified against a regenerated report rather than the previous one: core-unittests' target/pmd.xml, timestamped after this change, has zero violations. Worth noting how that report reads, because it is easy to get backwards -- it carries 821 `suppressedviolation` entries (NOPMD-annotated, deliberate) and a naive "tag ends with violation" match counts all 821 and reports a disaster. The element that matters is `violation`, and there are none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d for
Consequence of the Separator fix, and the proof that it was a real defect. Two rows in
DesktopWidgetsTheme change and nothing else in the image does:
y=186, y=329 #000000 -> #cac4d0 (light)
#000000 -> #49454f (dark)
#000000 was the default TEXT colour -- the component filled its whole visible area with the
foreground because it is exactly as thick as the rule. #cac4d0 and #49454f are what
android-material/theme.css declares for Separator and for its dark override, so the rule now
renders as the theme specifies instead of as a black bar.
The same will follow on every port whose theme draws this rule with a background rather than
a foreground: ios-modern does exactly what android-material does, so the iOS family and the
JavaScript port will report the same two rows and be reseeded from their own runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same two rows, same cause as the Android reseed, this time against ios-modern:
y=155, y=288 #000000 -> #c6c6c8 (light)
#000000 -> #38383a (dark)
Both are what ios-modern/theme.css declares for Separator and for its dark override, and
nothing else in either image moves.
This is the port that actually installs the iOS-derived theme. Worth recording alongside the
ports that did NOT move, because the prediction in the Android commit was too broad: the
JavaScript and iOS legs both passed unchanged. A theme only shifts if it declares an OPAQUE
Separator background -- with no such rule the background stays transparent, the new
early-return never triggers, and the paint path is identical to before. Predicting from
"which theme does the port use" was the wrong question; "does that theme declare the rule as
a background" is the right one.
moved Android, Mac Catalyst
unchanged Linux, Windows, macOS (desktop themes use color:), JavaScript, iOS
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
I said earlier that iOS passed unchanged by the Separator fix. That was wrong, and wrong for a specific reason worth recording: the green check I read was "Test iOS native test scripts", a different workflow from "Test iOS UI build scripts", which is the one that compares screenshots. Two similarly named checks, and I took the wrong one as the answer. Both change, with the two-row signature every affected port shows: screenshots-metal #000000 -> #c6c6c8 light, #38383a dark (ios-modern, as Catalyst) screenshots-watch #d2c6cc -> #f5e9ef light, #0a2634 dark The watch is not the black-to-grey case the others are, because its screen is 416x496 and its palette is its own -- the rule blends differently there. What matters is the same: two rows move and nothing else in either image does. tvOS passed and is deliberately left alone. It renders the same suite at 3840x2160 and its comparison matched, so whatever its separator resolves to there, it already matched its golden. Corrected scope, every port now measured rather than predicted: moved Android, Mac Catalyst, iOS Metal, watchOS unchanged Linux, Windows, macOS, JavaScript, tvOS Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Makes the three desktop native themes actually native. PR #5845 built, measured and gated
them and deliberately switched nothing on; this turns them on and fills what turning them on
exposed.
The two bugs this found
commandBehavior: Nativewas a silent way to lose every command.MenuBar.updateCommandshands the commands to
setNativeCommandsand RETURNS without drawing soft buttons -- correctwhere there is a real menu bar, and on a platform without one it means they go to an empty
method and are never drawn at all. Nothing downstream can tell that from "the platform handled
it". Latent until a theme asked, which the desktop themes do, and which the Windows and Linux
ports cannot honour. Fixed in
setCommandBehavior, whereBUTTON_BAR->SOFTKEYalreadynormalises an unsupportable behaviour.
COVERAGE.md's Desktop section was the iOS section.3959d6886cset out to remove ablock the merge had duplicated and removed the wrong copy, so the file has had no record of
which desktop widgets are covered since. Restored from
2ceb4f0f9cand updated.What changed
Scrolling. All three themes derived their scrollbar from the mobile one --
DesktopScrollThumb { cn1-derive: ScrollThumb; }and a transparent track, so no gutter, nominimum length and no highlight -- while the mobile themes carried the full desktop
treatment. Each theme now sizes its own gutter from the platform's figure and gives the thumb
its hover and drag colours. Two things measured rather than assumed: the highlight states are
.selectedand.pressed, never.hover(a.hoverrule there compiles and is neverpainted), and
cn1-deriveemits the whole state family by copying the base, so the old stubproduced
sel#bgColoridentical tobgColor-- a highlight present in the resource andinvisible on screen. The four UIIDs are also seeded in
resetThemePropsnow, so a theme thatturns the constant on without defining them no longer draws an invisible bar that still
reserves its gutter.
Menus.
PopupContentPane,CommandList,Command,TouchCommand,Tooltip,TooltipDialogand the dialog command area were undefined in all three themes, so each fellthrough to
UIManager's blank default -- black on white, on a dark window. Plus a realright-click menu:
Component.addContextMenuListenerhas fired for years and nothing everopened one.
Dialogs.
defaultNativeWindowModeBoolon the three themes, so a dialog opens as a realoperating system window. Anchored popups never do.
Keyboard. Tab and Shift-Tab move focus; Escape cancels. The traversal machinery is old and
could not be wired to Tab as it stood: its filter is opt-in,
preferredTabIndexdefaults to-1 and
TextAreais the only class in the framework that opts in, so Tab would have walkedbetween a form's text fields and skipped every button between them. The desktop order is its
own, filtered on focusability. Escape resolves through one
Dialog.cancel()that the windowclose control and the back gesture already meant separately.
desktopTitleBarModegained a reader. The three themes have carried the constant sincethey landed and nothing read it.
Three new components.
Separator,GroupBox,Stepper-- every desktop toolkit has allthree and Codename One had none.
The flip. Windows -> Fluent, Linux -> Adwaita, macOS -> Aqua. Java SE stays on legacy
deliberately: that default reaches every desktop application ever built rather than only ours.
Fidelity: 9 scored controls to 22. Everything that makes a window look like a desktop
window was unmeasured, including the interactive scrollbar. Three rows are not scored on every
platform, and that is the honest answer rather than a gap -- an
NSScrollerrenders nothingthrough
cacheDisplay(measured: correctusableParts, correct frame,isHidden=false, onecolour in the tile, tried detached and inside a real
NSScrollViewin both styles),NSMenubelongs to the window server, and the AppKit and GTK tooltips are separate windows.
What is still open
screenshot baselines. Both are captured from the runners that score them, never locally, so
those legs are red until the capture workflows have run and their output has been reviewed.
toolchain exists on the machine this was written on. CI is the first verifier.
setNativeCommands, sotheir commands stay in the now-properly-themed Toolbar, and
isDesktopHideToolbar()will nothide the only place they are drawn. Tracked in
COVERAGE.md.7294 core unit tests green. SpotBugs zero findings on
core-unittests; cast-semantics,control-characters, copyright, build-hint-catalog and the four fidelity gates clean. Vale and
LanguageTool clean on both guide chapters.
🤖 Generated with Claude Code