On Thu, 26 Oct 2023 00:31:52 GMT, Michael Strauß <[email protected]> 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
