On Tue, 3 Sep 2024 10:37:20 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:

> > I am not sure why secondary loop was added in the testcase for JDK-8078269..
> 
> That is unclear to me as well.
> 
> > JDK-8078269 issue was `TabbedPane.tabAreaBackground` should have precedence 
> > over `setBackground` but it was not working after JDK-8007563.. The 8007563 
> > tests for non-opaque case whereas 8078269 issue was for opaque. It seems if 
> > we put `UIManager.put("TabbedPane.tabAreaBackground", Color.GREEN); ` then 
> > the tabbedpane areas where there are no tabs are not GREEN but RED... Seems 
> > like we need a new testcase for JDK-8078269 and do away with secondary loop 
> > in 8007563 testcase..
> 
> This issue still exists with the existing implementation in 
> MetalTabbedPaneUI. If we add `UIManager.put("TabbedPane.tabAreaBackground", 
> Color.GREEN); ` to the existing test then the expected output is that `the 
> tabbedpane areas where there are no tabs should be GREEN` but it is `RED` and 
> the reason is the **background color instance check of UIResource** at 
> [L905](https://github.com/kumarabhi006/jdk/blob/73f7a5f15dbba54a98f3916ff1190520ac07874d/src/java.desktop/share/classes/javax/swing/plaf/metal/MetalTabbedPaneUI.java#L905).
>  Since `background is an instance of Color object` the condition evaluates to 
> **false** and the graphic's object color is set to the background of 
> component which is RED.
> 
> I think removal of SecondaryLoop should be ok for this PR. And for 
> JDK-8078269 as you suggested we may need a new test case which checks for the 
> `UIManager.put("TabbedPane.tabAreaBackground", Color.GREEN);` as the property 
> is not tested here.

As per
https://mail.openjdk.org/pipermail/swing-dev/2015-May/004521.html
secondary loop was added for
> Also inappropriate design chosen for the 8007563 reg test code affected test 
> results stability. It also has been fixed.

I guess because pre-2018 our tests was run in samevm mode so there might be 
some stability issue and since it has been changed to othervm mode by default 
since '18, I guess this loop is not needed.
Also, as per the discussion for the webrev, it seems the "background" color, if 
set, was given precedence over property or in other words if `background color 
instance check of UIResource` means if background color is not set by user and 
is set by L&F which in case of MetalL&F is `"TabbedPane.background", 
controlShadow,` which is UIResource and in this case if 
`TabbedPane.tabAreaBackground` is set, TabbedPane.tabAreaBackground will take 
precedence

but if background Color set by user as is in this case in the testcase, it will 
not be instance of UIResource but Color so background color will take precedence

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

PR Comment: https://git.openjdk.org/jdk/pull/20791#issuecomment-2326565674

Reply via email to