RFR: 8313709: Wrong layout of a FlowPane in a BorderPane in a ScrollPane, if padding is too big [v2]

John Hendrikx jhendrikx at openjdk.org
Thu Oct 26 07:30:48 UTC 2023


On Thu, 26 Oct 2023 00:31:52 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> Jose Pereda has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Migrate old tests to JUnit 5
>
> modules/javafx.graphics/src/main/java/javafx/scene/layout/BorderPane.java line 414:
> 
>> 412:         final Insets insets = getInsets();
>> 413:         if (width != -1) {
>> 414:             width -= (insets.getLeft() + insets.getRight());
> 
> Let's say we call `computeMinHeight(10)`, but the left and right insets are 20. This means that `width` is now -10, which probably means "ignore the value" (the spec isn't entirely clear about that, but -10 is not a valid width in any case).
> 
> I think the following code might be better:
> 
> if (width >= 0) {
>     width = Math.max(0, width - insets.getLeft() - insets.getRight());
> }

The `width` value passed to `computeMinHeight` (if not -1) should be the result of a call to `computeMinWidth(-1)` (depending on the result of `getContentBias`; this is specified somewhere); if that function always includes the insets, then subtracting them here should not result in a negative value.

Of course, one can never be too careful.

I don't like the changing of the meaning of the `width` parameter here though using parameter assignment.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1203#discussion_r1372711490


More information about the openjfx-dev mailing list