From 9708464110f38ac905dad8d169bd6467c4b72c7b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:31:31 +0300 Subject: [PATCH 01/10] Stop padding a Sheet by a CSS corner radius (issue #5488) A stylesheet rule such as cntSheet { border-radius: 4mm 4mm 0mm 0mm; padding: 0mm; margin: 0mm; } renders a band of empty space under the sheet title in 7.0.262 that was not there in 7.0.233. Cause: the same border class switch behind #5454. PR #5054 made a simple border-radius compile to RoundRectBorder rather than CSSBorder, and Sheet.show has always inset the content pane by the corner radius for every RoundRectBorder it sees. That inset exists because a hand written RoundRectBorder reserves twice its radius, so content would otherwise be drawn under the rounded corners. A border out of a stylesheet reserves nothing and the sheet is padded by whatever the CSS asked for, which here is nothing, so the inset is 4mm of padding on all four sides that the author never wrote. The reported app lays the sheet out in a Y box and adds to it directly, so the empty content pane sits between the title bar and the labels and those 8mm are the reported gap. #5469 already stopped the radius from inflating the box, but the cssBoxModel flag it added never reached this padding line. Skip the inset for a CSS sized border and keep it for a hand written one. The default themes are unaffected: neither native theme defines a Sheet UIID, so the branch only runs for a sheet a developer styled. Tests: SheetCssBorderRadiusTest covers the content pane picking up no padding, the reported layout leaving no gap under the title, and a hand written border still being inset. The first two fail on master with 4 and 8 pixels of padding respectively. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 9 +- .../ui/SheetCssBorderRadiusTest.java | 138 ++++++++++++++++++ 2 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index 12500b09708..57f1c9c1dd1 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -795,7 +795,14 @@ public void show(final int duration) { if (border instanceof RoundRectBorder) { RoundRectBorder b = (RoundRectBorder) border; - $(contentPane).setPaddingMillimeters(b.getCornerRadius()); + // A hand written RoundRectBorder reserves twice the radius of its own, so insetting + // the content pane by the radius keeps the content clear of the rounded corners. A + // border that came out of a stylesheet reserves nothing and the padding of the sheet + // is whatever the CSS asked for, so an inset here is padding the author never wrote, + // and on an empty content pane it becomes a gap under the title, see issue 5488. + if (!b.isCssBoxModel()) { + $(contentPane).setPaddingMillimeters(b.getCornerRadius()); + } } // Deal with iPhoneX notch. diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java new file mode 100644 index 00000000000..fae83d896ba --- /dev/null +++ b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java @@ -0,0 +1,138 @@ +/* + * Copyright (c) 2026, Codename One and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Codename One designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Codename One through http://www.codenameone.com/ if you + * need additional information or have any questions. + */ +package com.codename1.ui; + +import com.codename1.junit.FormTest; +import com.codename1.junit.UITestBase; +import com.codename1.ui.layouts.BorderLayout; +import com.codename1.ui.layouts.BoxLayout; +import com.codename1.ui.plaf.RoundRectBorder; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/// How a `border-radius` coming out of a stylesheet may pad a `Sheet`. +/// +/// A rule such as +/// +/// ```css +/// cntSheet { border-radius: 4mm 4mm 0mm 0mm; padding: 0mm; margin: 0mm; border: none; } +/// ``` +/// +/// compiles to a `RoundRectBorder` (a `CSSBorder` before the JS port native themes work), +/// and `Sheet.show` used to inset the content pane by the corner radius for every +/// `RoundRectBorder` it saw. With `padding: 0mm` in the stylesheet that inset is padding +/// the author never wrote and it renders as a band of empty space under the title, which is +/// [issue 5488](https://github.com/codenameone/CodenameOne/issues/5488). The inset stays for +/// hand written borders, which reserve twice the radius of their own. +class SheetCssBorderRadiusTest extends UITestBase { + + @FormTest + void cssSizedBorderDoesNotPadTheContentPane() { + Sheet sheet = showSheet(cssBorder()); + + Container contentPane = sheet.getContentPane(); + assertEquals(0, contentPane.getStyle().getPaddingTop(), + "a stylesheet radius may not add padding above the content"); + assertEquals(0, contentPane.getStyle().getPaddingBottom(), + "a stylesheet radius may not add padding below the content"); + assertEquals(0, contentPane.getStyle().getPaddingLeftNoRTL(), + "a stylesheet radius may not indent the content"); + assertEquals(0, contentPane.getStyle().getPaddingRightNoRTL(), + "a stylesheet radius may not indent the content"); + } + + @FormTest + void cssSizedBorderLeavesNoGapUnderTheTitle() { + // The reported app lays the sheet itself out in a Y box and adds to it directly, so the + // empty content pane sits between the title bar and the content: any padding it picks up + // is visible as a gap. + Sheet sheet = new Sheet(null, "Title"); + sheet.getAllStyles().setBorder(cssBorder()); + zeroBox(sheet); + sheet.setLayout(BoxLayout.y()); + Label first = new Label("Test label 1"); + sheet.add(first); + sheet.add(new Label("Test label 2")); + show(sheet); + + Container contentPane = sheet.getContentPane(); + assertEquals(0, contentPane.getPreferredH(), + "the empty content pane may not reserve height for the corner radius"); + assertEquals(0, contentPane.getHeight(), + "the empty content pane may not take up height under the title"); + int gap = first.getY() - (contentPane.getY() + contentPane.getHeight()); + assertEquals(first.getStyle().getMarginTop(), gap, + "the first label must follow the title bar with nothing but its own margin above it"); + } + + @FormTest + void handWrittenBorderStillInsetsTheContentPane() { + // Legacy sizing: the border reserves twice the radius, so the content pane is inset by the + // radius to keep content clear of the rounded corners. That behavior is unchanged. + Sheet sheet = showSheet(RoundRectBorder.create().cornerRadius(4f)); + + int radius = Display.getInstance().convertToPixels(4f); + assertEquals(radius, sheet.getContentPane().getStyle().getPaddingTop(), + "a hand written radius keeps insetting the content pane"); + assertEquals(radius, sheet.getContentPane().getStyle().getPaddingLeftNoRTL(), + "a hand written radius keeps insetting the content pane"); + } + + private RoundRectBorder cssBorder() { + // What the CSS compiler emits for border-radius: 4mm 4mm 0mm 0mm + return RoundRectBorder.create() + .cornerRadius(4f) + .topLeftMode(true) + .topRightMode(true) + .bottomLeftMode(false) + .bottomRightMode(false) + .cssBoxModel(true); + } + + private Sheet showSheet(RoundRectBorder border) { + Sheet sheet = new Sheet(null, "Title"); + sheet.getAllStyles().setBorder(border); + zeroBox(sheet); + sheet.getContentPane().add(new Label("Test label 1")); + sheet.getContentPane().add(new Label("Test label 2")); + show(sheet); + return sheet; + } + + private void zeroBox(Sheet sheet) { + // padding: 0mm; margin: 0mm from the reported stylesheet + sheet.getAllStyles().setPadding(0, 0, 0, 0); + sheet.getAllStyles().setMargin(0, 0, 0, 0); + } + + private void show(Sheet sheet) { + implementation.setBuiltinSoundsEnabled(false); + Form form = Display.getInstance().getCurrent(); + form.setLayout(new BorderLayout()); + sheet.show(0); + form.getAnimationManager().flush(); + flushSerialCalls(); + form.revalidate(); + } +} From d9ba37b92aced027d633b1f64f7c1f17bf23f1f8 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 17:47:58 +0300 Subject: [PATCH 02/10] Take the legacy inset off the content pane on a restyle Review of the previous commit: skipping the inset for a CSS sized border is not enough when the same sheet was already shown with a hand written one. The inset is written into the style of the content pane, so it outlives the restyle and the gap comes back. Remember the padding of the content pane before the first inset, units included, and put it back when the sheet is next shown with a border that asks for no inset, a CSS sized RoundRectBorder or no RoundRectBorder at all. Restoring rather than zeroing matters because the content pane is public API: a developer who padded it keeps that padding instead of having it silently cleared. Nothing is touched when no inset was ever applied. Also drop the unused assertTrue import from the test. Tests: two more cases in SheetCssBorderRadiusTest, one restyling from a hand written border to a CSS sized one and one checking the padding a developer set in millimetres comes back in millimetres. Both fail on the previous commit with the stale 4px inset. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 61 +++++++++++++++---- .../ui/SheetCssBorderRadiusTest.java | 48 ++++++++++++++- 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index 57f1c9c1dd1..f76914ca5ae 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -351,6 +351,13 @@ public void actionPerformed(ActionEvent evt) { /// Original padding values to prevent accumulation when showing the sheet multiple times. /// These are set the first time the sheet is shown and used as the base for safe area calculations. private int[] originalPadding = null; + /// Padding of the content pane as it was before a hand written `RoundRectBorder` inset it by its + /// corner radius, in top, bottom, left, right order and in the units of `#contentPaneInsetUnits`. + /// Null while no inset is applied, so restyling the sheet with a border that asks for no inset + /// puts the padding back instead of leaving the inset of the previous border behind. + private float[] contentPaneInset = null; + /// Padding units of the content pane as they were before the inset, null for pixels. + private byte[] contentPaneInsetUnits = null; private Form form; private final Rectangle sheetBounds = new Rectangle(); private boolean trackSheetBounds; @@ -792,17 +799,18 @@ public void show(final int duration) { titleParentStyle.setMarginLeft(titleMargin); titleParentStyle.setMarginRight(titleMargin); Border border = s.getBorder(); - if (border instanceof RoundRectBorder) { - RoundRectBorder b = (RoundRectBorder) border; - - // A hand written RoundRectBorder reserves twice the radius of its own, so insetting - // the content pane by the radius keeps the content clear of the rounded corners. A - // border that came out of a stylesheet reserves nothing and the padding of the sheet - // is whatever the CSS asked for, so an inset here is padding the author never wrote, - // and on an empty content pane it becomes a gap under the title, see issue 5488. - if (!b.isCssBoxModel()) { - $(contentPane).setPaddingMillimeters(b.getCornerRadius()); - } + // A hand written RoundRectBorder reserves twice the radius of its own, so insetting the + // content pane by the radius keeps the content clear of the rounded corners. A border that + // came out of a stylesheet reserves nothing and the padding of the sheet is whatever the + // CSS asked for, so an inset here is padding the author never wrote, and on an empty + // content pane it becomes a gap under the title, see issue 5488. + if (border instanceof RoundRectBorder && !((RoundRectBorder) border).isCssBoxModel()) { + rememberContentPanePadding(); + $(contentPane).setPaddingMillimeters(((RoundRectBorder) border).getCornerRadius()); + } else { + // Restyling the sheet with a border that wants no inset has to take the inset of the + // previous border back off, otherwise the gap survives the restyle + restoreContentPanePadding(); } // Deal with iPhoneX notch. @@ -931,6 +939,37 @@ public void call(Component c) { } } + /// Stores the padding of the content pane as it is before the corner radius of a hand written + /// border insets it. Only the first inset is recorded, so showing the sheet again does not + /// record the inset as if it were the padding of the developer. + private void rememberContentPanePadding() { + if (contentPaneInset != null) { + return; + } + Style cps = contentPane.getStyle(); + contentPaneInset = new float[]{ + cps.getPaddingFloatValue(false, Component.TOP), + cps.getPaddingFloatValue(false, Component.BOTTOM), + cps.getPaddingFloatValue(false, Component.LEFT), + cps.getPaddingFloatValue(false, Component.RIGHT) + }; + byte[] units = cps.getPaddingUnit(); + contentPaneInsetUnits = units == null ? null : new byte[]{units[0], units[1], units[2], units[3]}; + } + + /// Puts back the padding `#rememberContentPanePadding` stored, doing nothing when no inset was + /// ever applied so a content pane the developer padded is left alone. + private void restoreContentPanePadding() { + if (contentPaneInset == null) { + return; + } + Style cps = contentPane.getAllStyles(); + cps.setPaddingUnit(contentPaneInsetUnits); + cps.setPadding(contentPaneInset[0], contentPaneInset[1], contentPaneInset[2], contentPaneInset[3]); + contentPaneInset = null; + contentPaneInsetUnits = null; + } + /// Gets the position where the Sheet is to be displayed. /// One of `BorderLayout#CENTER`, `BorderLayout#NORTH`, `BorderLayout#SOUTH`, /// `BorderLayout#WEST`, or `BorderLayout#EAST`. Default is `BorderLayout#SOUTH`. diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java index fae83d896ba..2eb016d1a25 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java @@ -27,9 +27,9 @@ import com.codename1.ui.layouts.BorderLayout; import com.codename1.ui.layouts.BoxLayout; import com.codename1.ui.plaf.RoundRectBorder; +import com.codename1.ui.plaf.Style; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; /// How a `border-radius` coming out of a stylesheet may pad a `Sheet`. /// @@ -99,6 +99,52 @@ void handWrittenBorderStillInsetsTheContentPane() { "a hand written radius keeps insetting the content pane"); } + @FormTest + void restylingToACssBorderTakesTheLegacyInsetBackOff() { + // The inset of a hand written border is written into the style of the content pane, so a + // sheet restyled with a CSS sized border has to have it removed rather than merely not + // reapplied, otherwise the gap outlives the restyle. + Sheet sheet = showSheet(RoundRectBorder.create().cornerRadius(4f)); + assertEquals(Display.getInstance().convertToPixels(4f), + sheet.getContentPane().getStyle().getPaddingTop(), + "the hand written border insets the content pane on the first show"); + + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + assertEquals(0, sheet.getContentPane().getStyle().getPaddingTop(), + "the inset of the previous border may not survive the restyle"); + assertEquals(0, sheet.getContentPane().getStyle().getPaddingLeftNoRTL(), + "the inset of the previous border may not survive the restyle"); + } + + @FormTest + void restylingRestoresThePaddingTheDeveloperSet() { + // Restoring must not zero the content pane, it puts back whatever was there before the + // border inset it, in the units it was written in. + Sheet sheet = new Sheet(null, "Title"); + sheet.getContentPane().getAllStyles().setPaddingUnit(Style.UNIT_TYPE_DIPS); + sheet.getContentPane().getAllStyles().setPadding(1f, 1f, 2f, 2f); + sheet.getAllStyles().setBorder(RoundRectBorder.create().cornerRadius(4f)); + zeroBox(sheet); + sheet.getContentPane().add(new Label("Test label 1")); + show(sheet); + assertEquals(Display.getInstance().convertToPixels(4f), + sheet.getContentPane().getStyle().getPaddingTop(), + "the hand written border insets the content pane on the first show"); + + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + Style contentStyle = sheet.getContentPane().getStyle(); + assertEquals(Display.getInstance().convertToPixels(1f), contentStyle.getPaddingTop(), + "the padding of the developer comes back, in millimetres"); + assertEquals(Display.getInstance().convertToPixels(2f), contentStyle.getPaddingLeftNoRTL(), + "the padding of the developer comes back, in millimetres"); + assertEquals(Style.UNIT_TYPE_DIPS, contentStyle.getPaddingUnit()[Component.TOP], + "the padding unit of the developer comes back too"); + } + private RoundRectBorder cssBorder() { // What the CSS compiler emits for border-radius: 4mm 4mm 0mm 0mm return RoundRectBorder.create() From f12aba48568d03802b87dbf89f77feca166dad1a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:08:58 +0300 Subject: [PATCH 03/10] Restore the content pane padding to the style the inset wrote to Review of the previous commit: the restore went through getAllStyles while the inset goes through the component selector, which pads the current style of the content pane and leaves the selected, pressed and disabled styles alone. So restoring wrote the padding of the current style over three styles that were never insetted. Read and write the same style instead. The other styles are never part of the inset, so they have nothing to restore and are now left untouched. Tests: restoringLeavesTheOtherStylesOfTheContentPaneAlone pads the selected and pressed styles of the content pane before the first show and checks they survive the restyle. It fails on the previous commit with the selected padding replaced by 0. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 7 +++++- .../ui/SheetCssBorderRadiusTest.java | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index f76914ca5ae..d2de751e2d8 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -942,6 +942,11 @@ public void call(Component c) { /// Stores the padding of the content pane as it is before the corner radius of a hand written /// border insets it. Only the first inset is recorded, so showing the sheet again does not /// record the inset as if it were the padding of the developer. + /// + /// The inset is written by the component selector, which pads the current style of the content + /// pane rather than all of its styles, so this reads that same style and + /// `#restoreContentPanePadding` writes back to it. The selected, pressed and disabled styles + /// are never insetted and so have nothing to restore. private void rememberContentPanePadding() { if (contentPaneInset != null) { return; @@ -963,7 +968,7 @@ private void restoreContentPanePadding() { if (contentPaneInset == null) { return; } - Style cps = contentPane.getAllStyles(); + Style cps = contentPane.getStyle(); cps.setPaddingUnit(contentPaneInsetUnits); cps.setPadding(contentPaneInset[0], contentPaneInset[1], contentPaneInset[2], contentPaneInset[3]); contentPaneInset = null; diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java index 2eb016d1a25..6039ec1721e 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java @@ -145,6 +145,28 @@ void restylingRestoresThePaddingTheDeveloperSet() { "the padding unit of the developer comes back too"); } + @FormTest + void restoringLeavesTheOtherStylesOfTheContentPaneAlone() { + // The inset pads the current style of the content pane, not all of its styles, so restoring + // may not write the padding of the current style over the selected, pressed and disabled + // styles the inset never touched. + Sheet sheet = new Sheet(null, "Title"); + sheet.getContentPane().getSelectedStyle().setPadding(7, 7, 7, 7); + sheet.getContentPane().getPressedStyle().setPadding(9, 9, 9, 9); + sheet.getAllStyles().setBorder(RoundRectBorder.create().cornerRadius(4f)); + zeroBox(sheet); + sheet.getContentPane().add(new Label("Test label 1")); + show(sheet); + + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + assertEquals(7, sheet.getContentPane().getSelectedStyle().getPaddingTop(), + "the selected style of the content pane is not part of the inset"); + assertEquals(9, sheet.getContentPane().getPressedStyle().getPaddingTop(), + "the pressed style of the content pane is not part of the inset"); + } + private RoundRectBorder cssBorder() { // What the CSS compiler emits for border-radius: 4mm 4mm 0mm 0mm return RoundRectBorder.create() From c315719f3709b99280f06cadaf405619386d7e4a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:22:02 +0300 Subject: [PATCH 04/10] Only restore the content pane padding while the inset is still there Review of the previous commit: the snapshot was put back unconditionally on the next show, but the style it describes can be gone by then. A theme refresh replaces the style of the content pane, and application code is free to pad it between two shows. In both cases restoring wrote a stale snapshot over padding that was deliberately set. Keep the style that was padded and the padding the inset wrote alongside the padding it replaced, and restore only into a style that is still that same object and still holds exactly what the inset left. Anything else drops the snapshot and leaves the style alone, which is the safe reading: the inset is gone in that case anyway, so there is nothing to take off. The three fields become one ContentPaneInset holding them together. Also two review nits: "insetted" is not a word, and a test message said millimetres of a padding the test writes in DIPs. Tests: aThemeRefreshBetweenShowsDropsTheSnapshot swaps the style of the content pane between the two shows, paddingChangedBetweenShowsIsNot Overwritten pads it between them. Both fail on the previous commit, which replaces the new padding with the snapshot. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 138 +++++++++++++----- .../ui/SheetCssBorderRadiusTest.java | 39 ++++- 2 files changed, 138 insertions(+), 39 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index d2de751e2d8..178eb75cbb9 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -351,13 +351,10 @@ public void actionPerformed(ActionEvent evt) { /// Original padding values to prevent accumulation when showing the sheet multiple times. /// These are set the first time the sheet is shown and used as the base for safe area calculations. private int[] originalPadding = null; - /// Padding of the content pane as it was before a hand written `RoundRectBorder` inset it by its - /// corner radius, in top, bottom, left, right order and in the units of `#contentPaneInsetUnits`. - /// Null while no inset is applied, so restyling the sheet with a border that asks for no inset - /// puts the padding back instead of leaving the inset of the previous border behind. - private float[] contentPaneInset = null; - /// Padding units of the content pane as they were before the inset, null for pixels. - private byte[] contentPaneInsetUnits = null; + /// The padding a hand written `RoundRectBorder` replaced on the content pane, null while no + /// inset is applied. Restyling the sheet with a border that asks for no inset puts this back + /// rather than leaving the inset of the previous border behind. + private ContentPaneInset contentPaneInset; private Form form; private final Rectangle sheetBounds = new Rectangle(); private boolean trackSheetBounds; @@ -805,8 +802,17 @@ public void show(final int duration) { // CSS asked for, so an inset here is padding the author never wrote, and on an empty // content pane it becomes a gap under the title, see issue 5488. if (border instanceof RoundRectBorder && !((RoundRectBorder) border).isCssBoxModel()) { - rememberContentPanePadding(); + // The inset pads the current style of the content pane, so that is the style the + // padding is taken from and the one it is later put back into + Style contentStyle = contentPane.getStyle(); + if (contentPaneInset == null || !contentPaneInset.isIntact(contentStyle)) { + // Nothing was inset yet, or what was inset is gone: a theme refresh replaced + // the style, or the padding was changed since. Either way the padding in front of + // us now is the one to preserve + contentPaneInset = new ContentPaneInset(contentStyle); + } $(contentPane).setPaddingMillimeters(((RoundRectBorder) border).getCornerRadius()); + contentPaneInset.recordApplied(contentStyle); } else { // Restyling the sheet with a border that wants no inset has to take the inset of the // previous border back off, otherwise the gap survives the restyle @@ -939,40 +945,16 @@ public void call(Component c) { } } - /// Stores the padding of the content pane as it is before the corner radius of a hand written - /// border insets it. Only the first inset is recorded, so showing the sheet again does not - /// record the inset as if it were the padding of the developer. - /// - /// The inset is written by the component selector, which pads the current style of the content - /// pane rather than all of its styles, so this reads that same style and - /// `#restoreContentPanePadding` writes back to it. The selected, pressed and disabled styles - /// are never insetted and so have nothing to restore. - private void rememberContentPanePadding() { - if (contentPaneInset != null) { - return; - } - Style cps = contentPane.getStyle(); - contentPaneInset = new float[]{ - cps.getPaddingFloatValue(false, Component.TOP), - cps.getPaddingFloatValue(false, Component.BOTTOM), - cps.getPaddingFloatValue(false, Component.LEFT), - cps.getPaddingFloatValue(false, Component.RIGHT) - }; - byte[] units = cps.getPaddingUnit(); - contentPaneInsetUnits = units == null ? null : new byte[]{units[0], units[1], units[2], units[3]}; - } - - /// Puts back the padding `#rememberContentPanePadding` stored, doing nothing when no inset was - /// ever applied so a content pane the developer padded is left alone. + /// Puts back the padding the corner radius of a hand written border replaced, when that inset + /// is still there to take off. Nothing happens when no inset was applied, when the style it was + /// applied to has since been replaced, by a theme refresh for instance, or when the padding has + /// been changed since, so a content pane padded by the developer is left as they left it. private void restoreContentPanePadding() { if (contentPaneInset == null) { return; } - Style cps = contentPane.getStyle(); - cps.setPaddingUnit(contentPaneInsetUnits); - cps.setPadding(contentPaneInset[0], contentPaneInset[1], contentPaneInset[2], contentPaneInset[3]); + contentPaneInset.restore(contentPane.getStyle()); contentPaneInset = null; - contentPaneInsetUnits = null; } /// Gets the position where the Sheet is to be displayed. @@ -1488,4 +1470,86 @@ public void paint(Graphics g, Rectangle rect) { } } + + /// The padding of the content pane as it was before the corner radius of a hand written + /// `RoundRectBorder` replaced it, kept alongside the style that was padded and the padding the + /// inset wrote there. Restoring writes the old padding back only into a style that still holds + /// exactly what the inset left, so a theme refresh that swaps the style, or a developer padding + /// the content pane between two shows, drops the snapshot rather than overwriting their work. + /// + /// The inset is applied through the component selector, which pads the current style of the + /// content pane rather than all of its styles, so only that one style is ever recorded. The + /// selected, pressed and disabled styles are not part of the inset and have nothing to restore. + private static class ContentPaneInset { + private final Style style; + private final float[] padding; + private final byte[] units; + private float[] appliedPadding; + private byte[] appliedUnits; + + ContentPaneInset(Style style) { + this.style = style; + padding = paddingOf(style); + units = unitsOf(style); + } + + /// Records what the inset left in the style, which is what `#isIntact` looks for later. + void recordApplied(Style inset) { + appliedPadding = paddingOf(inset); + appliedUnits = unitsOf(inset); + } + + /// True when the given style is the one that was inset and still holds that inset. + boolean isIntact(Style current) { + return current == style //NOPMD CompareObjectsWithEquals + && appliedPadding != null + && same(paddingOf(current), appliedPadding) + && same(unitsOf(current), appliedUnits); + } + + /// Puts the padding back if the inset it replaced is still in place, otherwise leaves the + /// style alone. + void restore(Style current) { + if (!isIntact(current)) { + return; + } + current.setPaddingUnit(units); + current.setPadding(padding[0], padding[1], padding[2], padding[3]); + } + + private static float[] paddingOf(Style s) { + return new float[]{ + s.getPaddingFloatValue(false, Component.TOP), + s.getPaddingFloatValue(false, Component.BOTTOM), + s.getPaddingFloatValue(false, Component.LEFT), + s.getPaddingFloatValue(false, Component.RIGHT) + }; + } + + private static byte[] unitsOf(Style s) { + byte[] u = s.getPaddingUnit(); + return u == null ? null : new byte[]{u[0], u[1], u[2], u[3]}; + } + + private static boolean same(float[] a, float[] b) { + for (int i = 0; i < a.length; i++) { + if (a[i] != b[i]) { + return false; + } + } + return true; + } + + private static boolean same(byte[] a, byte[] b) { + if (a == null || b == null) { + return a == b; //NOPMD CompareObjectsWithEquals + } + for (int i = 0; i < a.length; i++) { + if (a[i] != b[i]) { + return false; + } + } + return true; + } + } } diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java index 6039ec1721e..5189675b80b 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java @@ -138,9 +138,9 @@ void restylingRestoresThePaddingTheDeveloperSet() { Style contentStyle = sheet.getContentPane().getStyle(); assertEquals(Display.getInstance().convertToPixels(1f), contentStyle.getPaddingTop(), - "the padding of the developer comes back, in millimetres"); + "the padding of the developer comes back, in the unit it was written in"); assertEquals(Display.getInstance().convertToPixels(2f), contentStyle.getPaddingLeftNoRTL(), - "the padding of the developer comes back, in millimetres"); + "the padding of the developer comes back, in the unit it was written in"); assertEquals(Style.UNIT_TYPE_DIPS, contentStyle.getPaddingUnit()[Component.TOP], "the padding unit of the developer comes back too"); } @@ -167,6 +167,41 @@ void restoringLeavesTheOtherStylesOfTheContentPaneAlone() { "the pressed style of the content pane is not part of the inset"); } + @FormTest + void aThemeRefreshBetweenShowsDropsTheSnapshot() { + // Replacing the style of the content pane, which is what a theme refresh does, takes the + // inset with it. The snapshot then describes a style nobody is using any more and writing + // it into the fresh style would undo the theme. + Sheet sheet = showSheet(RoundRectBorder.create().cornerRadius(4f)); + + Style fresh = new Style(sheet.getContentPane().getStyle()); + fresh.setPadding(3, 3, 3, 3); + fresh.setPaddingUnit(Style.UNIT_TYPE_PIXELS); + sheet.getContentPane().setUnselectedStyle(fresh); + + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + assertEquals(3, sheet.getContentPane().getStyle().getPaddingTop(), + "the padding of the new style survives, the stale snapshot is dropped"); + } + + @FormTest + void paddingChangedBetweenShowsIsNotOverwritten() { + // The developer padding the content pane after the inset went on is saying what they want + // it to be. Restoring may not put the pre-inset padding back over that. + Sheet sheet = showSheet(RoundRectBorder.create().cornerRadius(4f)); + + sheet.getContentPane().getStyle().setPaddingUnit(Style.UNIT_TYPE_PIXELS); + sheet.getContentPane().getStyle().setPadding(5, 5, 5, 5); + + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + assertEquals(5, sheet.getContentPane().getStyle().getPaddingTop(), + "padding set after the inset wins over the snapshot"); + } + private RoundRectBorder cssBorder() { // What the CSS compiler emits for border-radius: 4mm 4mm 0mm 0mm return RoundRectBorder.create() From 3644c00698969128a98a286f0e2ccd85969b92c5 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:32:52 +0300 Subject: [PATCH 05/10] Take the inset off the style it went into, not the one presented now Review of the previous commit: the style a component presents follows its state, so getStyle returns the disabled style once the content pane is disabled. A sheet shown with a hand written border while the pane was enabled therefore insetted the unselected style, and restyling it after the pane was disabled compared that inset against the disabled style, rejected it and dropped the snapshot anyway. The inset stayed in the unselected style and the gap came back with the pane. The snapshot already holds the style it was taken from, so restore into that style rather than looking up the current one, and keep one snapshot per style that was insetted rather than a single one. Which style the inset lands in is decided by the state of the pane at the time and can differ between two shows, so more than one may be outstanding. The list is capped at the four styles a component presents: an entry older than that belongs to a style that has since been replaced and is no longer attached to the content pane. Tests: disablingTheContentPaneBetweenShowsStillTakesTheInsetOff disables the pane between the two shows and checks the unselected style is clean afterwards. It fails on the previous commit with the stranded 4px inset. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 127 ++++++++++++------ .../ui/SheetCssBorderRadiusTest.java | 17 +++ 2 files changed, 104 insertions(+), 40 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index 178eb75cbb9..69c8f40602e 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -39,6 +39,8 @@ import com.codename1.ui.util.EventDispatcher; import com.codename1.util.AsyncResource; +import java.util.ArrayList; + import static com.codename1.ui.ComponentSelector.$; /// A light-weight dialog that slides up from the bottom of the screen on mobile devices. @@ -120,6 +122,9 @@ public class Sheet extends Container { private static final int W = 3; private static final int C = 4; private static final int DEFAULT_TRANSITION_DURATION = 300; + /// The styles a component presents: unselected, selected, pressed and disabled. Caps how many + /// content pane insets are kept, see `#recordContentPaneInset`. + private static final int CONTENT_PANE_STYLES = 4; private final Sheet parentSheet; private final Label title = new Label(); private Component titleComponent = title; @@ -351,10 +356,11 @@ public void actionPerformed(ActionEvent evt) { /// Original padding values to prevent accumulation when showing the sheet multiple times. /// These are set the first time the sheet is shown and used as the base for safe area calculations. private int[] originalPadding = null; - /// The padding a hand written `RoundRectBorder` replaced on the content pane, null while no - /// inset is applied. Restyling the sheet with a border that asks for no inset puts this back - /// rather than leaving the inset of the previous border behind. - private ContentPaneInset contentPaneInset; + /// The padding a hand written `RoundRectBorder` replaced on the content pane, one entry per + /// style it was applied to, null while no inset is applied. Restyling the sheet with a border + /// that asks for no inset puts these back rather than leaving the inset of the previous border + /// behind. + private ArrayList contentPaneInsets; private Form form; private final Rectangle sheetBounds = new Rectangle(); private boolean trackSheetBounds; @@ -802,17 +808,19 @@ public void show(final int duration) { // CSS asked for, so an inset here is padding the author never wrote, and on an empty // content pane it becomes a gap under the title, see issue 5488. if (border instanceof RoundRectBorder && !((RoundRectBorder) border).isCssBoxModel()) { - // The inset pads the current style of the content pane, so that is the style the - // padding is taken from and the one it is later put back into + // The inset pads the current style of the content pane, which is not always the same + // style: it follows the state of the pane, so a sheet shown while the pane is disabled + // pads the disabled style. Each style that gets insetted is recorded separately Style contentStyle = contentPane.getStyle(); - if (contentPaneInset == null || !contentPaneInset.isIntact(contentStyle)) { - // Nothing was inset yet, or what was inset is gone: a theme refresh replaced - // the style, or the padding was changed since. Either way the padding in front of - // us now is the one to preserve - contentPaneInset = new ContentPaneInset(contentStyle); + ContentPaneInset inset = contentPaneInsetFor(contentStyle); + if (inset == null || !inset.isIntact()) { + // Nothing was inset in this style yet, or what was inset is gone because the + // padding was changed since. Either way the padding in front of us now is the one + // to preserve + inset = recordContentPaneInset(contentStyle); } $(contentPane).setPaddingMillimeters(((RoundRectBorder) border).getCornerRadius()); - contentPaneInset.recordApplied(contentStyle); + inset.recordApplied(); } else { // Restyling the sheet with a border that wants no inset has to take the inset of the // previous border back off, otherwise the gap survives the restyle @@ -945,16 +953,55 @@ public void call(Component c) { } } - /// Puts back the padding the corner radius of a hand written border replaced, when that inset - /// is still there to take off. Nothing happens when no inset was applied, when the style it was - /// applied to has since been replaced, by a theme refresh for instance, or when the padding has - /// been changed since, so a content pane padded by the developer is left as they left it. + /// Puts back the padding the corner radius of a hand written border replaced, in every style it + /// was applied to rather than only the style the content pane presents right now, which depends + /// on the state of the pane and may well be a different one by the time the sheet is restyled. + /// A style whose padding has been changed since is left alone, so a content pane padded by the + /// developer stays as they left it. private void restoreContentPanePadding() { - if (contentPaneInset == null) { + if (contentPaneInsets == null) { return; } - contentPaneInset.restore(contentPane.getStyle()); - contentPaneInset = null; + for (int iter = 0; iter < contentPaneInsets.size(); iter++) { + contentPaneInsets.get(iter).restore(); + } + contentPaneInsets = null; + } + + /// The inset recorded for the given style of the content pane, null when that style was never + /// insetted. + private ContentPaneInset contentPaneInsetFor(Style style) { + if (contentPaneInsets == null) { + return null; + } + for (int iter = 0; iter < contentPaneInsets.size(); iter++) { + ContentPaneInset inset = contentPaneInsets.get(iter); + if (inset.style == style) { //NOPMD CompareObjectsWithEquals + return inset; + } + } + return null; + } + + /// Starts recording an inset for the given style of the content pane, replacing whatever was + /// recorded for it before. + private ContentPaneInset recordContentPaneInset(Style style) { + if (contentPaneInsets == null) { + contentPaneInsets = new ArrayList(); + } + for (int iter = contentPaneInsets.size() - 1; iter >= 0; iter--) { + if (contentPaneInsets.get(iter).style == style) { //NOPMD CompareObjectsWithEquals + contentPaneInsets.remove(iter); + } + } + while (contentPaneInsets.size() >= CONTENT_PANE_STYLES) { + // A component presents four styles at most, so an older entry than that belongs to a + // style that has since been replaced and is no longer attached to the content pane + contentPaneInsets.remove(0); + } + ContentPaneInset inset = new ContentPaneInset(style); + contentPaneInsets.add(inset); + return inset; } /// Gets the position where the Sheet is to be displayed. @@ -1471,15 +1518,16 @@ public void paint(Graphics g, Rectangle rect) { } - /// The padding of the content pane as it was before the corner radius of a hand written - /// `RoundRectBorder` replaced it, kept alongside the style that was padded and the padding the - /// inset wrote there. Restoring writes the old padding back only into a style that still holds - /// exactly what the inset left, so a theme refresh that swaps the style, or a developer padding - /// the content pane between two shows, drops the snapshot rather than overwriting their work. + /// The padding of one style of the content pane as it was before the corner radius of a hand + /// written `RoundRectBorder` replaced it, kept alongside that style and the padding the inset + /// wrote into it. Restoring writes the old padding back only while the style still holds + /// exactly what the inset left, so padding changed since is not overwritten. /// - /// The inset is applied through the component selector, which pads the current style of the - /// content pane rather than all of its styles, so only that one style is ever recorded. The - /// selected, pressed and disabled styles are not part of the inset and have nothing to restore. + /// The inset is applied through the component selector, which pads the style the content pane + /// presents at the time rather than all of its styles. Which style that is follows the state of + /// the pane, so an inset applied while it was enabled has to be taken off the unselected style + /// even if the pane is disabled by the time the sheet is restyled. Hence the style is held + /// here, and the sheet keeps one of these per style it insetted. private static class ContentPaneInset { private final Style style; private final float[] padding; @@ -1494,27 +1542,26 @@ private static class ContentPaneInset { } /// Records what the inset left in the style, which is what `#isIntact` looks for later. - void recordApplied(Style inset) { - appliedPadding = paddingOf(inset); - appliedUnits = unitsOf(inset); + void recordApplied() { + appliedPadding = paddingOf(style); + appliedUnits = unitsOf(style); } - /// True when the given style is the one that was inset and still holds that inset. - boolean isIntact(Style current) { - return current == style //NOPMD CompareObjectsWithEquals - && appliedPadding != null - && same(paddingOf(current), appliedPadding) - && same(unitsOf(current), appliedUnits); + /// True when the style still holds the inset that was applied to it. + boolean isIntact() { + return appliedPadding != null + && same(paddingOf(style), appliedPadding) + && same(unitsOf(style), appliedUnits); } /// Puts the padding back if the inset it replaced is still in place, otherwise leaves the /// style alone. - void restore(Style current) { - if (!isIntact(current)) { + void restore() { + if (!isIntact()) { return; } - current.setPaddingUnit(units); - current.setPadding(padding[0], padding[1], padding[2], padding[3]); + style.setPaddingUnit(units); + style.setPadding(padding[0], padding[1], padding[2], padding[3]); } private static float[] paddingOf(Style s) { diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java index 5189675b80b..a7c2b614860 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java @@ -202,6 +202,23 @@ void paddingChangedBetweenShowsIsNotOverwritten() { "padding set after the inset wins over the snapshot"); } + @FormTest + void disablingTheContentPaneBetweenShowsStillTakesTheInsetOff() { + // The style a component presents follows its state, so the pane being disabled by the time + // the sheet is restyled must not strand the inset in the unselected style it went into. + Sheet sheet = showSheet(RoundRectBorder.create().cornerRadius(4f)); + assertEquals(Display.getInstance().convertToPixels(4f), + sheet.getContentPane().getUnselectedStyle().getPaddingTop(), + "the inset goes into the unselected style while the pane is enabled"); + + sheet.getContentPane().setEnabled(false); + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + assertEquals(0, sheet.getContentPane().getUnselectedStyle().getPaddingTop(), + "the inset comes off the style it went into, not the style presented now"); + } + private RoundRectBorder cssBorder() { // What the CSS compiler emits for border-radius: 4mm 4mm 0mm 0mm return RoundRectBorder.create() From 879b0f7161f30c6a47c52a6a5562c00dc4b40ce0 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:44:38 +0300 Subject: [PATCH 06/10] Never evict a content pane inset to make room Review of the previous commit: capping the recorded insets at four and dropping the oldest assumed age says whether a style is still attached to the content pane, and it does not. A style insetted first and still in use is evicted by four later entries, stranding its inset. Drop the cap. The other way to bound the list, asking the content pane for its four styles and pruning anything not among them, would create the selected, pressed and disabled styles on a pane that never had them, and creating those registers elevation and surface state, so it is not free. Instead an entry for a style that has been replaced is carried until the next restore, where putting padding back into a detached style costs nothing. Entries there is nothing left to restore for are dropped when the next inset is recorded, which is what keeps the list short in practice. Tests: everyStyleThatWasInsetIsRestoredHoweverManyThereAre insets one style, pushes four more through the content pane, and checks the first one is still cleaned up. It fails on the previous commit with the first inset stranded. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 22 ++++++++--------- .../ui/SheetCssBorderRadiusTest.java | 24 +++++++++++++++++++ 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index 69c8f40602e..d961cdd512d 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -122,9 +122,6 @@ public class Sheet extends Container { private static final int W = 3; private static final int C = 4; private static final int DEFAULT_TRANSITION_DURATION = 300; - /// The styles a component presents: unselected, selected, pressed and disabled. Caps how many - /// content pane insets are kept, see `#recordContentPaneInset`. - private static final int CONTENT_PANE_STYLES = 4; private final Sheet parentSheet; private final Label title = new Label(); private Component titleComponent = title; @@ -983,22 +980,25 @@ private ContentPaneInset contentPaneInsetFor(Style style) { return null; } - /// Starts recording an inset for the given style of the content pane, replacing whatever was - /// recorded for it before. + /// Starts recording an inset for the given style of the content pane, dropping the entry + /// recorded for that style before along with any entry there is nothing left to restore for. + /// + /// Entries are not otherwise evicted. Dropping the oldest once a few have accumulated would be + /// wrong, because age does not say whether a style is still attached to the content pane, and + /// the alternative of asking the pane for its four styles would create the selected, pressed + /// and disabled ones on a pane that never had them, which registers elevation and surface + /// state. So an entry for a style that has been replaced is simply carried until the next + /// restore, where putting padding back into a detached style costs nothing. private ContentPaneInset recordContentPaneInset(Style style) { if (contentPaneInsets == null) { contentPaneInsets = new ArrayList(); } for (int iter = contentPaneInsets.size() - 1; iter >= 0; iter--) { - if (contentPaneInsets.get(iter).style == style) { //NOPMD CompareObjectsWithEquals + ContentPaneInset recorded = contentPaneInsets.get(iter); + if (recorded.style == style || !recorded.isIntact()) { //NOPMD CompareObjectsWithEquals contentPaneInsets.remove(iter); } } - while (contentPaneInsets.size() >= CONTENT_PANE_STYLES) { - // A component presents four styles at most, so an older entry than that belongs to a - // style that has since been replaced and is no longer attached to the content pane - contentPaneInsets.remove(0); - } ContentPaneInset inset = new ContentPaneInset(style); contentPaneInsets.add(inset); return inset; diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java index a7c2b614860..0c6ae0fc674 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java @@ -219,6 +219,30 @@ void disablingTheContentPaneBetweenShowsStillTakesTheInsetOff() { "the inset comes off the style it went into, not the style presented now"); } + @FormTest + void everyStyleThatWasInsetIsRestoredHoweverManyThereAre() { + // Insets pile up one per style, and none of them may be evicted to make room: a style being + // the oldest recorded does not say it is gone, so evicting it would strand its inset. + Sheet sheet = showSheet(RoundRectBorder.create().cornerRadius(4f)); + Style first = sheet.getContentPane().getStyle(); + assertEquals(Display.getInstance().convertToPixels(4f), first.getPaddingTop(), + "the first style is inset by the hand written border"); + + for (int iter = 0; iter < 4; iter++) { + Style fresh = new Style(first); + fresh.setPaddingUnit(Style.UNIT_TYPE_PIXELS); + fresh.setPadding(0, 0, 0, 0); + sheet.getContentPane().setUnselectedStyle(fresh); + show(sheet); + } + + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + assertEquals(0, first.getPaddingTop(), + "the style inset first is restored however many were recorded after it"); + } + private RoundRectBorder cssBorder() { // What the CSS compiler emits for border-radius: 4mm 4mm 0mm 0mm return RoundRectBorder.create() From a863424e3edef6c5e533940b46bf89b77bc83fce Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:46:03 +0300 Subject: [PATCH 07/10] Spell inset as inset in the content pane inset docs Review nit: the past participle of inset is inset, not insetted. Three comments in Sheet used the nonstandard form. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index d961cdd512d..cadc9b96faf 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -807,7 +807,7 @@ public void show(final int duration) { if (border instanceof RoundRectBorder && !((RoundRectBorder) border).isCssBoxModel()) { // The inset pads the current style of the content pane, which is not always the same // style: it follows the state of the pane, so a sheet shown while the pane is disabled - // pads the disabled style. Each style that gets insetted is recorded separately + // pads the disabled style. Each style that gets inset is recorded separately Style contentStyle = contentPane.getStyle(); ContentPaneInset inset = contentPaneInsetFor(contentStyle); if (inset == null || !inset.isIntact()) { @@ -966,7 +966,7 @@ private void restoreContentPanePadding() { } /// The inset recorded for the given style of the content pane, null when that style was never - /// insetted. + /// inset. private ContentPaneInset contentPaneInsetFor(Style style) { if (contentPaneInsets == null) { return null; @@ -1527,7 +1527,7 @@ public void paint(Graphics g, Rectangle rect) { /// presents at the time rather than all of its styles. Which style that is follows the state of /// the pane, so an inset applied while it was enabled has to be taken off the unselected style /// even if the pane is disabled by the time the sheet is restyled. Hence the style is held - /// here, and the sheet keeps one of these per style it insetted. + /// here, and the sheet keeps one of these per style it inset. private static class ContentPaneInset { private final Style style; private final float[] padding; From a18f10ae65d718454a5fb7cebc6052599d7537ff Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 18:58:35 +0300 Subject: [PATCH 08/10] Track each side of a content pane inset on its own Review of the previous commit: padding one side of the content pane after the inset went on made the whole snapshot count as changed, so restyling the sheet left the inset stranded on the other three sides. Each side is now compared and restored on its own. A side that still holds what the inset wrote is put back, a side padded since keeps what it was given. Insetting again refreshes the remembered padding of the sides that were changed, so the value preserved for a side is always the last one asked for rather than the one from before the first inset. Tests: changingOneSideAfterTheInsetLeavesTheOtherThreeRestorable pads only the top after the inset and checks the top survives while the other three are cleaned up. It fails on the previous commit with the three sides left holding the inset. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 132 +++++++++++------- .../ui/SheetCssBorderRadiusTest.java | 23 +++ 2 files changed, 104 insertions(+), 51 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index cadc9b96faf..ac056c235fe 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -810,11 +810,12 @@ public void show(final int duration) { // pads the disabled style. Each style that gets inset is recorded separately Style contentStyle = contentPane.getStyle(); ContentPaneInset inset = contentPaneInsetFor(contentStyle); - if (inset == null || !inset.isIntact()) { - // Nothing was inset in this style yet, or what was inset is gone because the - // padding was changed since. Either way the padding in front of us now is the one - // to preserve + if (inset == null) { inset = recordContentPaneInset(contentStyle); + } else { + // A side that no longer holds the inset was padded since, so the padding in front + // of us now is the one to preserve for that side + inset.rememberChangedSides(); } $(contentPane).setPaddingMillimeters(((RoundRectBorder) border).getCornerRadius()); inset.recordApplied(); @@ -995,7 +996,7 @@ private ContentPaneInset recordContentPaneInset(Style style) { } for (int iter = contentPaneInsets.size() - 1; iter >= 0; iter--) { ContentPaneInset recorded = contentPaneInsets.get(iter); - if (recorded.style == style || !recorded.isIntact()) { //NOPMD CompareObjectsWithEquals + if (recorded.style == style || !recorded.hasIntactSide()) { //NOPMD CompareObjectsWithEquals contentPaneInsets.remove(iter); } } @@ -1520,8 +1521,9 @@ public void paint(Graphics g, Rectangle rect) { /// The padding of one style of the content pane as it was before the corner radius of a hand /// written `RoundRectBorder` replaced it, kept alongside that style and the padding the inset - /// wrote into it. Restoring writes the old padding back only while the style still holds - /// exactly what the inset left, so padding changed since is not overwritten. + /// wrote into it. Each side is tracked on its own: a side is put back only while it still holds + /// what the inset wrote there, so padding changed since is not overwritten, and changing one + /// side does not strand the inset on the other three. /// /// The inset is applied through the component selector, which pads the style the content pane /// presents at the time rather than all of its styles. Which style that is follows the state of @@ -1529,74 +1531,102 @@ public void paint(Graphics g, Rectangle rect) { /// even if the pane is disabled by the time the sheet is restyled. Hence the style is held /// here, and the sheet keeps one of these per style it inset. private static class ContentPaneInset { + /// The sides of a style, in the order `Style#getPaddingUnit` indexes them. + private static final int[] SIDES = {Component.TOP, Component.LEFT, Component.BOTTOM, Component.RIGHT}; private final Style style; - private final float[] padding; - private final byte[] units; + private final float[] padding = new float[SIDES.length]; + private final byte[] units = new byte[SIDES.length]; private float[] appliedPadding; private byte[] appliedUnits; ContentPaneInset(Style style) { this.style = style; - padding = paddingOf(style); - units = unitsOf(style); - } - - /// Records what the inset left in the style, which is what `#isIntact` looks for later. - void recordApplied() { - appliedPadding = paddingOf(style); - appliedUnits = unitsOf(style); + for (int iter = 0; iter < SIDES.length; iter++) { + remember(SIDES[iter]); + } } - /// True when the style still holds the inset that was applied to it. - boolean isIntact() { - return appliedPadding != null - && same(paddingOf(style), appliedPadding) - && same(unitsOf(style), appliedUnits); + /// Takes the padding of the given side as the one to put back. + private void remember(int side) { + padding[side] = style.getPaddingFloatValue(false, side); + units[side] = unitOf(style, side); } - /// Puts the padding back if the inset it replaced is still in place, otherwise leaves the - /// style alone. - void restore() { - if (!isIntact()) { + /// Takes the padding of every side the inset no longer holds as the one to put back. Called + /// before insetting again, so a side padded since the last inset keeps the padding it was + /// given rather than the one from before that inset. + void rememberChangedSides() { + if (appliedPadding == null) { return; } - style.setPaddingUnit(units); - style.setPadding(padding[0], padding[1], padding[2], padding[3]); + for (int iter = 0; iter < SIDES.length; iter++) { + if (!isIntact(SIDES[iter])) { + remember(SIDES[iter]); + } + } } - private static float[] paddingOf(Style s) { - return new float[]{ - s.getPaddingFloatValue(false, Component.TOP), - s.getPaddingFloatValue(false, Component.BOTTOM), - s.getPaddingFloatValue(false, Component.LEFT), - s.getPaddingFloatValue(false, Component.RIGHT) - }; + /// Records what the inset left in the style, which is what `#isIntact` looks for later. + void recordApplied() { + appliedPadding = new float[SIDES.length]; + appliedUnits = new byte[SIDES.length]; + for (int iter = 0; iter < SIDES.length; iter++) { + int side = SIDES[iter]; + appliedPadding[side] = style.getPaddingFloatValue(false, side); + appliedUnits[side] = unitOf(style, side); + } } - private static byte[] unitsOf(Style s) { - byte[] u = s.getPaddingUnit(); - return u == null ? null : new byte[]{u[0], u[1], u[2], u[3]}; + /// True when the given side of the style still holds the inset that was applied to it. + boolean isIntact(int side) { + return appliedPadding != null + && style.getPaddingFloatValue(false, side) == appliedPadding[side] + && unitOf(style, side) == appliedUnits[side]; } - private static boolean same(float[] a, float[] b) { - for (int i = 0; i < a.length; i++) { - if (a[i] != b[i]) { - return false; + /// True when any side still holds its inset, meaning there is something left to restore. + boolean hasIntactSide() { + for (int iter = 0; iter < SIDES.length; iter++) { + if (isIntact(SIDES[iter])) { + return true; } } - return true; + return false; } - private static boolean same(byte[] a, byte[] b) { - if (a == null || b == null) { - return a == b; //NOPMD CompareObjectsWithEquals - } - for (int i = 0; i < a.length; i++) { - if (a[i] != b[i]) { - return false; + /// Puts back the padding of every side that still holds its inset, leaving the sides that + /// were changed since as they are. + void restore() { + for (int iter = 0; iter < SIDES.length; iter++) { + int side = SIDES[iter]; + if (isIntact(side)) { + setPaddingUnit(style, side, units[side]); + style.setPadding(side, padding[side]); } } - return true; + } + + private static byte unitOf(Style s, int side) { + byte[] u = s.getPaddingUnit(); + // A style with no units of its own measures in pixels + return u == null ? Style.UNIT_TYPE_PIXELS : u[side]; + } + + private static void setPaddingUnit(Style s, int side, byte unit) { + switch (side) { + case Component.TOP: + s.setPaddingUnitTop(unit); + break; + case Component.BOTTOM: + s.setPaddingUnitBottom(unit); + break; + case Component.LEFT: + s.setPaddingUnitLeft(unit); + break; + default: + s.setPaddingUnitRight(unit); + break; + } } } } diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java index 0c6ae0fc674..a247c2076ba 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/SheetCssBorderRadiusTest.java @@ -243,6 +243,29 @@ void everyStyleThatWasInsetIsRestoredHoweverManyThereAre() { "the style inset first is restored however many were recorded after it"); } + @FormTest + void changingOneSideAfterTheInsetLeavesTheOtherThreeRestorable() { + // Each side stands on its own: padding one side after the inset says what that side should + // be, and says nothing about the three the inset is still sitting on. + Sheet sheet = showSheet(RoundRectBorder.create().cornerRadius(4f)); + Style contentStyle = sheet.getContentPane().getStyle(); + + contentStyle.setPaddingUnitTop(Style.UNIT_TYPE_PIXELS); + contentStyle.setPadding(Component.TOP, 5f); + + sheet.getAllStyles().setBorder(cssBorder()); + show(sheet); + + assertEquals(5, contentStyle.getPaddingTop(), + "the side padded after the inset keeps what it was given"); + assertEquals(0, contentStyle.getPaddingBottom(), + "the sides still holding the inset are restored"); + assertEquals(0, contentStyle.getPaddingLeftNoRTL(), + "the sides still holding the inset are restored"); + assertEquals(0, contentStyle.getPaddingRightNoRTL(), + "the sides still holding the inset are restored"); + } + private RoundRectBorder cssBorder() { // What the CSS compiler emits for border-radius: 4mm 4mm 0mm 0mm return RoundRectBorder.create() From 41fb3f5462ae045d26f2cb56370315255075c1f5 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:01:11 +0300 Subject: [PATCH 09/10] Write down what bounds the recorded content pane insets Review asked whether the list can grow without bound. Record the analysis where the decision is: one entry per style the content pane presents while a hand written border is in effect, so the count follows how often those styles are replaced, a theme refresh in practice, between one show and the show that takes the inset off. Weak references would let it shrink on its own, but the portable weak reference of the platform is allowed to report that it holds nothing, and reading that as a style that went away would silently skip a restore that is still owed. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index ac056c235fe..6c78b245c81 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -990,6 +990,13 @@ private ContentPaneInset contentPaneInsetFor(Style style) { /// and disabled ones on a pane that never had them, which registers elevation and surface /// state. So an entry for a style that has been replaced is simply carried until the next /// restore, where putting padding back into a detached style costs nothing. + /// + /// One entry is added per style the content pane presents while a hand written border is in + /// effect, so the count is bounded by how often something replaces those styles, a theme + /// refresh in practice, between one show of this sheet and the show that takes the inset off. + /// Holding the styles weakly instead would let the list shrink on its own, but the portable + /// weak reference of the platform is allowed to report that it holds nothing, and treating that + /// as a style that went away would silently skip a restore that is still owed. private ContentPaneInset recordContentPaneInset(Style style) { if (contentPaneInsets == null) { contentPaneInsets = new ArrayList(); From a2056b3e19066fbe65934bf95dbd9d9b6bbcd59a Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:24:40 +0300 Subject: [PATCH 10/10] Iterate the content pane insets with foreach The PMD gate forbids ForLoopCanBeForeach and the index loops added for the inset bookkeeping tripped it, failing build-test (8). None of them used the index for anything but element access, so they all convert. The one that stays indexed walks backwards while removing, which foreach cannot do, and PMD does not flag it. Verified by running the gate the way CI does, mvn verify on core-unittests followed by generate-quality-report.py, which now exits clean. Co-Authored-By: Claude Opus 5 (1M context) --- CodenameOne/src/com/codename1/ui/Sheet.java | 27 +++++++++------------ 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Sheet.java b/CodenameOne/src/com/codename1/ui/Sheet.java index 6c78b245c81..b5ba83a3c11 100644 --- a/CodenameOne/src/com/codename1/ui/Sheet.java +++ b/CodenameOne/src/com/codename1/ui/Sheet.java @@ -960,8 +960,8 @@ private void restoreContentPanePadding() { if (contentPaneInsets == null) { return; } - for (int iter = 0; iter < contentPaneInsets.size(); iter++) { - contentPaneInsets.get(iter).restore(); + for (ContentPaneInset inset : contentPaneInsets) { + inset.restore(); } contentPaneInsets = null; } @@ -972,8 +972,7 @@ private ContentPaneInset contentPaneInsetFor(Style style) { if (contentPaneInsets == null) { return null; } - for (int iter = 0; iter < contentPaneInsets.size(); iter++) { - ContentPaneInset inset = contentPaneInsets.get(iter); + for (ContentPaneInset inset : contentPaneInsets) { if (inset.style == style) { //NOPMD CompareObjectsWithEquals return inset; } @@ -1548,8 +1547,8 @@ private static class ContentPaneInset { ContentPaneInset(Style style) { this.style = style; - for (int iter = 0; iter < SIDES.length; iter++) { - remember(SIDES[iter]); + for (int side : SIDES) { + remember(side); } } @@ -1566,9 +1565,9 @@ void rememberChangedSides() { if (appliedPadding == null) { return; } - for (int iter = 0; iter < SIDES.length; iter++) { - if (!isIntact(SIDES[iter])) { - remember(SIDES[iter]); + for (int side : SIDES) { + if (!isIntact(side)) { + remember(side); } } } @@ -1577,8 +1576,7 @@ void rememberChangedSides() { void recordApplied() { appliedPadding = new float[SIDES.length]; appliedUnits = new byte[SIDES.length]; - for (int iter = 0; iter < SIDES.length; iter++) { - int side = SIDES[iter]; + for (int side : SIDES) { appliedPadding[side] = style.getPaddingFloatValue(false, side); appliedUnits[side] = unitOf(style, side); } @@ -1593,8 +1591,8 @@ boolean isIntact(int side) { /// True when any side still holds its inset, meaning there is something left to restore. boolean hasIntactSide() { - for (int iter = 0; iter < SIDES.length; iter++) { - if (isIntact(SIDES[iter])) { + for (int side : SIDES) { + if (isIntact(side)) { return true; } } @@ -1604,8 +1602,7 @@ boolean hasIntactSide() { /// Puts back the padding of every side that still holds its inset, leaving the sides that /// were changed since as they are. void restore() { - for (int iter = 0; iter < SIDES.length; iter++) { - int side = SIDES[iter]; + for (int side : SIDES) { if (isIntact(side)) { setPaddingUnit(style, side, units[side]); style.setPadding(side, padding[side]);