RFR: 6510914: JScrollBar.getMinimumSize() breaks the contract of JComponent.setMinimumSize() [v9]

[Alexey Ivanov]

On Fri, 19 Jan 2024 04:48:40 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> javadoc contract for JComponent.setMinimumSize(Dimension) states:
>> 
>> "Sets the minimum size of this component to a constant value. Subsequent calls to getMinimumSize will always return this value..."
>> 
>> However, JScrollBar overrides getMinimumSize() and breaks this contract - it always returns a minimum size derived from the preferred size even if you have previously called setMinimumSize()
>> 
>> Fix is made to check if mnimumSize is set and if so, honour it..
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   javadoc updated

I admit I'm lost…

Do we implement the hybrid approach? That is `getMinimumSize` and `getMaximumSize` return a value derived from `getPreferredSize` (modifying it so that the scroll bar is flexible along its scrolling axis rather than returning it directly as `ComponentUI` does). If minimum or maximum size is set, the set value is returned.

Is it correct?

If it is, do we even need to override `setMinimumSize` and `setMaximumSize`?

The contract specified in `JComponent` is satisfied. Yet, *not in full*: neither of these methods returns `ui.get*Size()`, either method calls `getPreferredSize` (which in its turn calls `ui.getPreferredSize`) and adjusts the value.

Hence, we still need to override them to update get min/max size to say the value is derived from preferred size rather than `ui` delegate's min/max size. (This can be asserted with a unit test.)

src/java.desktop/share/classes/javax/swing/JScrollBar.java line 762:

> 760:      * Thus, it overrides {@code JComponent.setMinimumSize} contract
> 761:      * that subsequent calls to getMinimumSize will return the
> 762:      * same value as set in {@code JComponent.setMinimumSize}

Doesn't the javadoc contradict the implementation?

This fix has restored the contract of `setMinimumSize` / `getMinimumSize`, so `JScrollBar` is now like other components.

What we can add though is how the size is calculated when it's not set, however, it's documented in the `get-` methods.

src/java.desktop/share/classes/javax/swing/JScrollBar.java line 785:

> 783: 
> 784:     /**
> 785:      * The scrollbar is flexible along it's scrolling axis and

Suggestion:

     * The scrollbar is flexible along its scrolling axis and

Let's fix it too.

src/java.desktop/share/classes/javax/swing/JScrollBar.java line 790:

> 788:      * @return the value of the {@code minimumSize} property if set by user
> 789:      *         or if not set, minimum size derived from
> 790:      *         preferred size in one axis and fixed size in another axis

I think you should refer the derived size first is the most common returned value, and then mention `minimumSize` if it's set.

src/java.desktop/share/classes/javax/swing/JScrollBar.java line 805:

> 803: 
> 804:     /**
> 805:      * The scrollbar is flexible along it's scrolling axis and

Suggestion:

     * The scrollbar is flexible along its scrolling axis and

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

PR Review: https://git.openjdk.org/jdk/pull/15325#pullrequestreview-1837127605
PR Review Comment: https://git.openjdk.org/jdk/pull/15325#discussion_r1462295939
PR Review Comment: https://git.openjdk.org/jdk/pull/15325#discussion_r1462310181
PR Review Comment: https://git.openjdk.org/jdk/pull/15325#discussion_r1462268856
PR Review Comment: https://git.openjdk.org/jdk/pull/15325#discussion_r1462310500