RFR: 6664309: Docking point of a floating toolbar changes after closing [v2]

[Phil Race]

On Wed, 14 Jun 2023 09:45:17 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> Issue is if we create a floating JToolBar at the EAST side and drag it and then if we close the floating toolbar, then it gets docket at NORTH side instead of the EAST side where it was docked before being floated.
>> This is because of some contentious code in BasicToolBarUI where we change the constaint to NORTH if it is EAST/WEST.
>> I could not find any JBS which did that code change.
>> 
>> Albeit, removing that piece of code does not cause regression in "clientlibs" jtreg/jck tests in addition to solve this anomaly, so thought, removing the code seems prudent..
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Test fix

Marked as reviewed by prr (Reviewer).

So there's two things at play here
(1) the orientation property set on the toolbar, which governs how the components on the toolbar are laid out.
(2) the special interpretation by JToolBar of being laid out on one side of a BorderLayout.
I don't know that I really understand why
>For drag-out to work correctly, it is recommended that you add JToolBar instances to one of the four "sides"
> of a container whose layout manager is a BorderLayout, and do not add children to any of the other four "sides".

my guess is that it just has the right characteristics - the CENTER component would be free to use for the rest
of the window, and empty sides are not an issue, and you can easily make it undock from any edge, and redock
on any edge without affecting layout of the rest of the window.

I've played around with it and it seems to me that the intent of the code you are removing is that
a HORIZONTAL layout implies it was docked on a side that would be HORIZONTAL - but in that
case why NORTH and not SOUTH ?

And I've verified that its perfectly possible, if a bit weird, to have HORIZONTAL toolbar on the EAST
i.e toolbar buttons laid out left to right.

So conflating orientation of the toolbar with the edge on which it is docked is probably wrong.

BTW you should add to the bug evaluation that this code was introduced by the fix for

https://bugs.openjdk.org/browse/JDK-4700351

The reasoning there seems to be a bit back and forth and confusing as to what views won out,
but I do not agree with all the comment.
For example I don't agree that "Also, when placed in a BorderLayout, the orientation should change
to the appropriate value, even if the orientation has been set explicitly"

But PROBABLY most people putting a toolbar on the EAST will make it vertical so I expect few apps
to see an unwelcome change in behaviour here .. but there's always a risk of that.

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

PR Review: https://git.openjdk.org/jdk/pull/14444#pullrequestreview-1488953439
PR Comment: https://git.openjdk.org/jdk/pull/14444#issuecomment-1599440139