Re: RFR 6542439:Significant memory leak in BasicComboBoxUI and MetalComboBoxButton

[Sergey Bylokhov]

+1

On 27.08.2020 01:33, Prasanta Sadhukhan wrote:

Thanks Sergey for your review. I have added AquaComboBoxButton also in the fix as 
the regression test fails for Aqua L

http://cr.openjdk.java.net/~psadhukhan/6542439/webrev.1/

mach5 job link is in JBS.

Regards
Prasanta
On 22-Aug-20 8:27 AM, Sergey Bylokhov wrote:

Looks fine.

On 20.08.2020 06:54, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix for an memory leak issue whereby the BasicComboBoxUI uses a 
CellRenderer pane to paint the renderer component
retrieved via getListCellRenderer from the JList instance. CellRendererPane 
paint the component but it never removes it from its component hierarchy.
JCombobox can produce dynamic renderer component instances in 
getListCellRenderer method. Each one of these instances
will linger in memory because it remains in the component hierarchy of the 
CellRendererPane resulting in memory leak.

This is solved for JList in JDK-5044798 
.

The fix is now extended for BasicComboBoxUI where removeAll() of components is 
done after all painting is done.

The JDK-5044798  regression 
testcase is used for JComboBox with all L and other *ComboBoxUIs are fixed 
similarly.

Bug: https://bugs.openjdk.java.net/browse/JDK-6542439

webrev: http://cr.openjdk.java.net/~psadhukhan/6542439/webrev.0/

Regards
Prasanta






--
Best regards, Sergey.

Re: RFR: 8252469: Backout JDK-8250935 fix

[Sergey Bylokhov]

+1

On 27.08.2020 11:51, Philip Race wrote:

+1

-phil

On 8/27/20, 11:42 AM, Prasanta Sadhukhan wrote:

Hi All,

This is a request to review backout of JFileChooser fix. The change affects mac 
and the fix should be restricted to windows. I tested the fix on windows/linux 
but mac was omitted.

It seems Desktop/ is not IShellFolder/IShellDetails interface so 
returns null, which is why the data is not matching windows folder details. We need 
to look at Win32ShellFolder implementation.

Since we need to integrate our repo, it's seem prudent to backout this fix and 
work on it later.

Bug: https://bugs.openjdk.java.net/browse/JDK-8252469

webrev: cr.openjdk.java.net/~psadhukhan/8252469/webrev.0/

Regards
Prasanta



--
Best regards, Sergey.

Re: RFR: 8252469: Backout JDK-8250935 fix

[Philip Race]

+1

-phil

On 8/27/20, 11:42 AM, Prasanta Sadhukhan wrote:

Hi All,

This is a request to review backout of JFileChooser fix. The change 
affects mac and the fix should be restricted to windows. I tested the 
fix on windows/linux but mac was omitted.


It seems Desktop/ is not IShellFolder/IShellDetails 
interface so returns null, which is why the data is not matching 
windows folder details. We need to look at Win32ShellFolder 
implementation.


Since we need to integrate our repo, it's seem prudent to backout this 
fix and work on it later.


Bug: https://bugs.openjdk.java.net/browse/JDK-8252469

webrev: cr.openjdk.java.net/~psadhukhan/8252469/webrev.0/

Regards
Prasanta

RFR: 8252469: Backout JDK-8250935 fix

[Prasanta Sadhukhan]

Hi All,

This is a request to review backout of JFileChooser fix. The change 
affects mac and the fix should be restricted to windows. I tested the 
fix on windows/linux but mac was omitted.


It seems Desktop/ is not IShellFolder/IShellDetails 
interface so returns null, which is why the data is not matching windows 
folder details. We need to look at Win32ShellFolder implementation.


Since we need to integrate our repo, it's seem prudent to backout this 
fix and work on it later.


Bug: https://bugs.openjdk.java.net/browse/JDK-8252469

webrev: cr.openjdk.java.net/~psadhukhan/8252469/webrev.0/

Regards
Prasanta

Re: RFR 8250935:JFileChooser incorrectly placed "Date" value in "Type" field

[Sergey Bylokhov]

Hi, Prasanta.

On 27.08.2020 02:59, Prasanta Sadhukhan wrote:

I have added the evaluation to JBS and I can see the issue even in 6u211, 7u281 
too in my windows10.


 - I checked behavior of the test on macOS and it looks like the JFileChooser has only 3 columns: 
"name", "Size", "date".
   How it is intended to work after your fix? I guess you need to change the 
Win32ShellFolder2 not the ShellFolder.

 - You do not need to print stack trace here, just exit the test:
try {

UIManager.setLookAndFeel("javax.swing.plaf.metal.MetalLookAndFeel");
} catch (ClassNotFoundException | InstantiationException | 
IllegalAccessException
| UnsupportedLookAndFeelException e) {
e.printStackTrace();
}



Regards
Prasanta

-phil.

On 8/24/20, 7:27 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix for a JFileChooser issue where it is seen that if we open the dialog and click 
on "Details" view and then click on "Home" button,
the "Modified" time is shown in "Type" field. This issue is present from at 
least jdk8 onwards.

Issue is because for ShellFolder#getFolderColumnValue handles "name", "Size", "date" 
cases ignoring "Type" case so date value is shown in Type field.

Proposed fix is to handle folder "Type" case too and also "Date" value is 
formatted correctly
to show in same format as other non-shellfolder options and
also to be same as "native" time format for same folder.

Bug: https://bugs.openjdk.java.net/browse/JDK-8250935

webrev: http://cr.openjdk.java.net/~psadhukhan/8250935/webrev.00

Regards

Prasanta

P.S: There is still some issue regarding the header values recursively inside 
Shellfolders which will handled subsequently in separate issue




--
Best regards, Sergey.

Re: RFR 8250935:JFileChooser incorrectly placed "Date" value in "Type" field

[Prasanta Sadhukhan]

On 27-Aug-20 12:58 AM, Philip Race wrote:

Approved but ...

Can you add an evaluation to the bug report ? It has none.

Was this a regression due to some other fix in JDK 8? Or been there 
forever which would be suprising

Either way please add that info.

I have added the evaluation to JBS and I can see the issue even in 
6u211, 7u281 too in my windows10.


Regards
Prasanta

-phil.

On 8/24/20, 7:27 AM, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix for a JFileChooser issue where it is seen that if 
we open the dialog and click on "Details" view and then click on 
"Home" button,
the "Modified" time is shown in "Type" field. This issue is present 
from at least jdk8 onwards.


Issue is because for ShellFolder#getFolderColumnValue handles "name", 
"Size", "date" cases ignoring "Type" case so date value is shown in 
Type field.


Proposed fix is to handle folder "Type" case too and also "Date" 
value is formatted correctly

to show in same format as other non-shellfolder options and
also to be same as "native" time format for same folder.

Bug: https://bugs.openjdk.java.net/browse/JDK-8250935

webrev: http://cr.openjdk.java.net/~psadhukhan/8250935/webrev.00

Regards

Prasanta

P.S: There is still some issue regarding the header values 
recursively inside Shellfolders which will handled subsequently in 
separate issue

Re: RFR: 8251122: doclint html5 errors in java.desktop/share/classes/javax/swing/plaf/nimbus/doc-files/properties.html

[Tejpal Rebari]

Hi Alexey,
Thanks for the review

> On 21-Aug-2020, at 1:04 AM, Alexey Ivanov  wrote:
> 
> Hi Tejpal,
> 
> Looks good overall, however, I have a couple of comments.
> 
> I suggest using the  elements for setting width of table columns. Add 
> the following three lines before  in *properties.html*:
> 
> 
> 
> 
> 
> Dropping "padding:2px;" style makes the table look consistent with other 
> tables above: there's no additional padding between table border and its 
> cells.
> 
> Removing all other style attributes ("border-collapse: separate; 
> border-spacing: 2px;") does not change rendering. The table looks exactly as 
> the table above which does not have any additional style specifiers.
> 
> I guess we should strive for consistent look of all the tables on the page. 
> Shall we drop the additional attributes then?

Yeah, I have dropped the style attributes to make the table  consistent.
Updated webrev : http://cr.openjdk.java.net/~trebari/swing/8251122/webrev01/ 


Also there was one new error in java/awt/doc-files/DesktopProperties.html due 
to the fix of 8251124.
So I have removed an empty  tag.
Also verified that no accessibility doclint errors were thrown.

> The same comments apply to the following JInternalFrame, 
> JInternalFrameTitlePane, JProgressBar…
> 
> If required, I'd rather add these style declarations inside 

Re: RFR 6542439:Significant memory leak in BasicComboBoxUI and MetalComboBoxButton

[Prasanta Sadhukhan]

Thanks Sergey for your review. I have added AquaComboBoxButton also in 
the fix as the regression test fails for Aqua L


http://cr.openjdk.java.net/~psadhukhan/6542439/webrev.1/

mach5 job link is in JBS.

Regards
Prasanta
On 22-Aug-20 8:27 AM, Sergey Bylokhov wrote:

Looks fine.

On 20.08.2020 06:54, Prasanta Sadhukhan wrote:

Hi All,

Please review a fix for an memory leak issue whereby the 
BasicComboBoxUI uses a CellRenderer pane to paint the renderer component
retrieved via getListCellRenderer from the JList instance. 
CellRendererPane paint the component but it never removes it from its 
component hierarchy.
JCombobox can produce dynamic renderer component instances in 
getListCellRenderer method. Each one of these instances
will linger in memory because it remains in the component hierarchy 
of the CellRendererPane resulting in memory leak.


This is solved for JList in JDK-5044798 
.


The fix is now extended for BasicComboBoxUI where removeAll() of 
components is done after all painting is done.


The JDK-5044798  
regression testcase is used for JComboBox with all L and other 
*ComboBoxUIs are fixed similarly.


Bug: https://bugs.openjdk.java.net/browse/JDK-6542439

webrev: http://cr.openjdk.java.net/~psadhukhan/6542439/webrev.0/

Regards
Prasanta

RFR 8040914:Test javax/swing/JLabel/6596966/bug6596966.java fails : comboBox isn't focus owner

[Prasanta Sadhukhan]

Hi All,

Please review a fix for a test issue problemlisted for mac.

Proposed fix is to add autodelay to add some delay between key events, 
without which the test fail in some slow mach5 system.


mach5 job for several iterations is successful in all platforms and link 
is in JBS.


Bug: https://bugs.openjdk.java.net/browse/JDK-8040914

webrev: http://cr.openjdk.java.net/~psadhukhan/8040914/webrev.0/

Regards
Prasanta