date:20160324

Re: [9] Review Request for several test bugs: 8150535, 8151033, 8151037 etc.

2016-03-24 Thread Yuri Nesterenko


Sergey, I'm sorry,
filing JDK-8152693, I found an error in that version.

A new one is
http://cr.openjdk.java.net/~yan/8150535/webrev.02
The spawned processes were provided with addExports.

Shame on me!
-yan


On 03/24/2016 05:34 PM, Sergey Bylokhov wrote:

cc swing-dev

On 24.03.16 17:17, Yuri Nesterenko wrote:

Hi,

please review this test-only fix, a leftover from Jigsaw M3 integration.
Some 7 tests fixed but note that not all of them always pass even fixed.
Webrev:
http://cr.openjdk.java.net/~yan/8150535/webrev.01

I need to file a bug about the fixed here manual test
java/awt/xembed/server/TestXEmbedServerJava

Thank you!

-yan

Re: [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

2016-03-24 Thread Sergey Bylokhov


On 24.03.16 16:52, Alexander Scherbatiy wrote:

On 24/03/16 10:36, Semyon Sadetsky wrote:

Hi Alexander,

Could you answer one question:
Why did you choose default interface methods to implement
TextUIDrawing and not implement them in DefaultTextUIDrawing having
declarations only in the interface?
AFAIK the common point of view is default methods should be used
rarely because they may lead to unreadable code.


   The only problem which I know about default methods is multiple
inheritance which has its own solution.


What kind of problems? The benefit is obvious: it will not be necessary 
to implement all methods if only one of them should be tweaked.




   Could you give links to discussion or provide use cases where default
methods leads to the unreadable code and show how does it relate to the
TextUIDrawing  implementation?

   Thanks,
   Alexandr.

--Semyon

On 3/18/2016 6:49 PM, Alexander Scherbatiy wrote:


Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8132119/webrev.08/

  - Public TextUIDrawing interface is added to the javax.swing.plaf
package
  - TextUIDrawing methods description does not mention component
properties to be more general
  - TextUIDrawing methods are made default
  - L sets an instance of the TextUIDrawing to look and feel
defaults using "uiDrawing.text" property
  - ComponentUI class is not changed
  - Each ComponentUI reads TextUIDrawing from UI defaults
  - There is an interesting issue described in
http://mail.openjdk.java.net/pipermail/swing-dev/2016-March/005509.html
which is related to the fact that MetalLabelUI returns a static
field from createUI() method.
TitleBorder creates a JLabel but does not put it to any component
hierarchy. In this case SwingUtilities.updateComponentTreeUI() method
calls MetalLabelUI.uninstallDefaults() on the static metalLabelUI
field and sets a new LabelUI for ordinary labels. The TitleBorder
label UI is not changed in this case and it still uses the
metalLabelUI field which is not initialized.
It seems that other applications can also use components just for
drawing and have the same issue.
For this case the textUIDrawing field is not cleared in the
uninstallDefaults but just set to a static default value which should
not lead to memory leaks.

  Thanks,
  Alexandr.

On 29/01/16 19:51, Alexander Scherbatiy wrote:

On 25/01/16 13:44, Andrej Golovnin wrote:

Hi Alexandr,


Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8132119/webrev.07/



- public TextUIDrawing interface is added to the javax.swing.plaf
package
- public "TextUIDrawing getTextUIDrawing()" method is added to the
ComponentUI class
- L sets an instance of the TextUIDrawing to look and feel
defaults using
"uiDrawing.text" property
- Look and Feel delegates use the instance of the TextUIDrawing
for text
drawing and measuring

Some thoughts on the current design/implementation:

By adding a field to the ComponentUI class the current
implementation increases
memory consumption for all Swing applications. And you get the
feeling that
there are different implementations of TextUIDrawing per
ComponentUI instances.
Personally I can't imagine to have different implementations of
TextUIDrawing for
a given LookAndFeel. If I would design/implement it, then I would
implement it as
a property of the LookAndFeel class (similar to LayoutStyle) and not
the ComponentUI.
Developers can use then the following code to obtain the instance of
TextUIDrawing:

UIManager.getLookAndFeel().getUIDrawing() // or
UIManager.getLookAndFeelUIDrawing() // use this static method as a
short cut for the line above.

  LayoutStyle keeps its instance per App context. The same is for
the LookAndFeel
  when it is got through UIManager.getLookAndFeel() call.
  It means that accessing an instance of a TextUIDrawing will leads
to a time consumption.

  There are 3 main ways of the SwingUtilities2.drawString(...) usage:
  1. ComponentUI classes
  2. Components created in UI (like BasicInternalFrameTitlePane)
  3. Public utilities methods (like WindowsGraphicsUtils.paintText())

  For the cases 1 and 2 it is possible to load and store the
UIDrawing instance during installUI()/updateUI() calls to decrease a
time access to it.

  For the case 3 it is necessary to get LookAndFeel instance each
time (which is taken from an App context)
  or use the passed JComponent object. It requires to have a public
method and the associated variable for each instance of
JComponent/ComponentUI/... class.

You can use this methods then in JDK too.

And maybe rename the TextUIDrawing class to just UIDrawing and add
more useful methods,
e.g. a method to create a composite font, a method to convert DLUs
to pixels.

  UIDrawing name may look like it should be used for any UI drawing,
not only for text ones. I am afraid that it can be misleading.

  Thanks,
  Alexandr.



Best regards,
Andrej Golovnin











--
Best regards, Sergey.

Re: [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

2016-03-24 Thread Alexander Scherbatiy


On 24/03/16 10:36, Semyon Sadetsky wrote:

Hi Alexander,

Could you answer one question:
Why did you choose default interface methods to implement 
TextUIDrawing and not implement them in DefaultTextUIDrawing having 
declarations only in the interface?
AFAIK the common point of view is default methods should be used 
rarely because they may lead to unreadable code.


  The only problem which I know about default methods is multiple 
inheritance which has its own solution.


  Could you give links to discussion or provide use cases where default 
methods leads to the unreadable code and show how does it relate to the 
TextUIDrawing  implementation?


  Thanks,
  Alexandr.

--Semyon

On 3/18/2016 6:49 PM, Alexander Scherbatiy wrote:


Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8132119/webrev.08/

  - Public TextUIDrawing interface is added to the javax.swing.plaf 
package
  - TextUIDrawing methods description does not mention component 
properties to be more general

  - TextUIDrawing methods are made default
  - L sets an instance of the TextUIDrawing to look and feel 
defaults using "uiDrawing.text" property

  - ComponentUI class is not changed
  - Each ComponentUI reads TextUIDrawing from UI defaults
  - There is an interesting issue described in
http://mail.openjdk.java.net/pipermail/swing-dev/2016-March/005509.html
which is related to the fact that MetalLabelUI returns a static 
field from createUI() method.
TitleBorder creates a JLabel but does not put it to any component 
hierarchy. In this case SwingUtilities.updateComponentTreeUI() method 
calls MetalLabelUI.uninstallDefaults() on the static metalLabelUI 
field and sets a new LabelUI for ordinary labels. The TitleBorder 
label UI is not changed in this case and it still uses the 
metalLabelUI field which is not initialized.
It seems that other applications can also use components just for 
drawing and have the same issue.
For this case the textUIDrawing field is not cleared in the 
uninstallDefaults but just set to a static default value which should 
not lead to memory leaks.


  Thanks,
  Alexandr.

On 29/01/16 19:51, Alexander Scherbatiy wrote:

On 25/01/16 13:44, Andrej Golovnin wrote:

Hi Alexandr,


Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8132119/webrev.07/


- public TextUIDrawing interface is added to the javax.swing.plaf 
package

- public "TextUIDrawing getTextUIDrawing()" method is added to the
ComponentUI class
- L sets an instance of the TextUIDrawing to look and feel 
defaults using

"uiDrawing.text" property
- Look and Feel delegates use the instance of the TextUIDrawing 
for text

drawing and measuring

Some thoughts on the current design/implementation:

By adding a field to the ComponentUI class the current 
implementation increases
memory consumption for all Swing applications. And you get the 
feeling that
there are different implementations of TextUIDrawing per 
ComponentUI instances.

Personally I can't imagine to have different implementations of
TextUIDrawing for
a given LookAndFeel. If I would design/implement it, then I would
implement it as
a property of the LookAndFeel class (similar to LayoutStyle) and not
the ComponentUI.
Developers can use then the following code to obtain the instance of
TextUIDrawing:

UIManager.getLookAndFeel().getUIDrawing() // or
UIManager.getLookAndFeelUIDrawing() // use this static method as a
short cut for the line above.
  LayoutStyle keeps its instance per App context. The same is for 
the LookAndFeel

  when it is got through UIManager.getLookAndFeel() call.
  It means that accessing an instance of a TextUIDrawing will leads 
to a time consumption.


  There are 3 main ways of the SwingUtilities2.drawString(...) usage:
  1. ComponentUI classes
  2. Components created in UI (like BasicInternalFrameTitlePane)
  3. Public utilities methods (like WindowsGraphicsUtils.paintText())

  For the cases 1 and 2 it is possible to load and store the 
UIDrawing instance during installUI()/updateUI() calls to decrease a 
time access to it.


  For the case 3 it is necessary to get LookAndFeel instance each 
time (which is taken from an App context)
  or use the passed JComponent object. It requires to have a public 
method and the associated variable for each instance of 
JComponent/ComponentUI/... class.

You can use this methods then in JDK too.

And maybe rename the TextUIDrawing class to just UIDrawing and add
more useful methods,
e.g. a method to create a composite font, a method to convert DLUs 
to pixels.
  UIDrawing name may look like it should be used for any UI drawing, 
not only for text ones. I am afraid that it can be misleading.


  Thanks,
  Alexandr.



Best regards,
Andrej Golovnin

Re: Review Request for 6439354 : Win L: TitledBorder colors are not from desktop

2016-03-24 Thread Sergey Bylokhov


+1 thanks.

On 22.03.16 11:56, Prem Balakrishnan wrote:

Hi Sergey,

Updated test as per the review comments.

Webrev: http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.01/

Regards,
Prem


-Original Message-
From: Sergey Bylokhov
Sent: Monday, March 21, 2016 7:31 PM
To: Prem Balakrishnan; Semyon Sadetsky; Rajeev Chamyal; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re: Review Request for 6439354 : Win L: TitledBorder colors are not 
from desktop

Hi, Prem.
In the test the Swing components accessed on non-EDT thread. Probably the test 
can be simplified further?

On 21.03.16 11:20, Prem Balakrishnan wrote:

Hi*,*

Please review fix for JDK 9,

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

*Webrev: *http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.00/

*Issue:*

Win L: TitledBorder colors are not from desktop

*Cause:*

"TitledBorder.border" is set to EtchedBorder with default
highlight/shadow colors.

*Fix:*

"TitledBorder.border" is set to  EtchedBorder with Desktop
   highlight/shadow colors.

**

*Test: *Manual Test (since windows theme to be set)**

**

Regards,
Prem




--
Best regards, Sergey.




--
Best regards, Sergey.

Re: CFV: New Swing Group Member: Semyon Sadetsky

2016-03-24 Thread Alexander Scherbatiy



Vote: yes.

Thanks,
Alexandr.

On 21/03/16 10:01, Alexander Scherbatiy wrote:


I hereby nominate Semyon Sadetsky (OpenJDK user name: ssadetsky) to 
Membership in the Swing Group.


Semyon is active member of Swing group and contributed a lot of fixes 
which include Swing TimerQueue race condition improvement, UndoManager 
deadlock resolving, proposing and implementation a public API for 
internal Swing functionality which is part of Swing modularization 
support for JDK 9, and others (see Semyon’s list OpenJDK changesets [1])


Semyon proposed a big change for the Swing part of the JEP 283 “Enable 
GTK 3 on Linux” [2] implementation [3].


Semyon not only provides fixes for Swing issues but also regularly 
reviews fixes suggested by other members.


Votes are due by Apr 5, 2016.

Only current Members of the Swing  Group [4] are eligible
to vote on this nomination.

For Lazy Consensus voting instructions, see [5].

Thanks,
Alexandr.

[1] 
http://hg.openjdk.java.net/jdk9/dev/jdk/log?revcount=1000=author%28%22ssadetsky%22%29

[2] http://openjdk.java.net/jeps/283
[3] 
http://mail.openjdk.java.net/pipermail/swing-dev/2016-March/005440.html

[4] http://openjdk.java.net/census#swing
[5] http://openjdk.java.net/groups/#member-vote

[9] JDK-8049069 : JButton incorrect behaviour on button release

2016-03-24 Thread Ajit Ghaisas

Hi,

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

Issue :  A JButton which is pressed using left mouse button gets released if 
right mouse button is pressed and released.

Root cause :
---
In file BasicButtonListener.java, mousePressed() and mouseReleased() methods 
check whether the event is from left mouse button.
This check is done using   if (SwingUtilities.isLeftMouseButton(e) ) 
This method returns true if left mouse button is down OR event is from left 
mouse button.
SwingUtilities.isLeftMouseButton() returns true if it is called for right mouse 
event while holding left button down. This causes mouseReleased() method to 
release pressed JButton.


Alternatives considered :
-
1. Modifying SwingUtilities.isLeftMouseButton() method to only test for whether 
the event is from left mouse button only
2. Modifying mousePressed() and mouseReleased() methods to check for whether 
the event is from left mouse button using argument passed to them.

Option 1 will break the code wherever SwingUtilities.isLeftMouseButton() is 
used to test for left mouse button down.
File history revealed that the mouse button down condition is added to fix 
another bug.

Hence, option 2 is chosen as it is safe and intuitive.
I have also added a test. It passes on Windows, Linux and Mac.
Test mentioned in comment on the bug also passed.

Please review the webrev :

http://cr.openjdk.java.net/~aghaisas/8049069/webrev.00/

Regards,
Ajit

Re: [9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

2016-03-24 Thread Avik Niyogi

Hi Rajeev,

The fix looks fine to me.

With Regards,
Avik Niyogi

-Original Message-
From: Rajeev Chamyal 
Sent: 23 March 2016 14:45
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net 

Subject: RE:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hello Sergey,

I had found below link about pre tag which states A P tag is strictly not 
permitted inside PRE, but if a browser encounters one, it should treat it as 
two newlines.
http://www.htmlhelp.com/reference/wilbur/block/pre.html 
 

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov
Sent: 22 March 2016 21:58
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net 

Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Looks fine to me. But I am not an expert here. And I wonder why the  tag is 
so specific, can we get the similar issue if some other tags will be used 
instead?

On 22.03.16 11:35, Rajeev Chamyal wrote:
> Hello All,
> 
> Gentle reminder.
> Please review the fix.
> 
> Bug : https://bugs.openjdk.java.net/browse/JDK-8150225 
> 
> Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/ 
> 
> 
> Regards,
> Rajeev Chamyal
> 
> -Original Message-
> From: Rajeev Chamyal
> Sent: 09 March 2016 15:58
> To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net 
> 
> Subject: Re:  [9] Review request for JDK-8150225 
> api/javax_swing/text/AbstractWriter/index_indent failed
> 
> Hello Sergey,
> 
> I have run JCK tests for HTMLWriter and AbstractWriter with this fix and all 
> passed.
> 
> Regards,
> Rajeev Chamyal
> 
> -Original Message-
> From: Sergey Bylokhov
> Sent: 09 March 2016 15:54
> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net 
> 
> Subject: Re:  [9] Review request for JDK-8150225 
> api/javax_swing/text/AbstractWriter/index_indent failed
> 
> Hi, Rajeev.
> Please confirm that there are no new issues in the jck after this fix.
> 
> On 09.03.16 12:18, Rajeev Chamyal wrote:
>> Hello All,
>> 
>> Please review the following fix for Jdk9:
>> 
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8150225 
>> 
>> 
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/ 
>> 
>> > >
>> 
>> Issue : JCK conformance test failed due to fix for bug JDK-7104635
>> 
>> Fix: Reverted the fix for JDK-7104635 and added a new method in 
>> HTMLWriter.java to check if P tag is within Pre tag.
>> 
>> Decrement indentation is skipped if P tag is with a Pre tag.
>> 
>> Regards,
>> 
>> Rajeev Chamyal
>> 
> 
> 
> --
> Best regards, Sergey.
> 


--
Best regards, Sergey.

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

2016-03-24 Thread Rajeev Chamyal

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 24 March 2016 12:54
To: Rajeev Chamyal; Alexander Scherbatiy; Sergey Bylokhov; 
swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8137169 : [macosx] Incorrect minimal 
heigh of JTabbedPane with more tabs

 

Hi All,

Please review code changes as per inputs received. 

 

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.03

 

As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

 

With Regards,

Avik Niyogi

 

On 24-Mar-2016, at 12:34 pm, Rajeev Chamyal mailto:rajeev.cham...@oracle.com"rajeev.cham...@oracle.com> wrote:

 

Hello Avik,

 

x variable on line 2195 is not used anywhere. Do we need for loop also?

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 24 March 2016 12:19
To: Alexander Scherbatiy
Cc: Sergey Bylokhov; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8137169 : [macosx] Incorrect minimal 
heigh of JTabbedPane with more tabs

 

Hi All,

 

Please review my code changes below as per the inputs received.

 

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.02/

 

As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

 

With Regards,

Avik Niyogi

 

 

On 23-Mar-2016, at 4:14 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 23/03/16 14:07, Avik Niyogi wrote:




 

On 23-Mar-2016, at 3:31 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 21/03/16 09:19, Avik Niyogi wrote:




Hi Alexander, 

I agree with what you said regarding the look and feel looking different. But 
this bug arrises due to setting of TabbedPaneScrollLayout only. If Scroll 
Layout is not meant for Aqua look and feel should not the setting of this 
parameter instead throw a helpful error saying this parameter is not accepted

  According to the JTabbedPane.setTabLayoutPolicy(int tabLayoutPolicy) javadoc: 
 
  "Some look and feels might only support a subset of the possible layout 
policies, in which case the value of this property may be ignored."
  
  Aqua L ignores WRAP_TAB_LAYOUT for JTabbedPane tabs layouting and always 
use SCROLL_TAB_LAYOUT. No exception should be thrown in this case.

 

Actually, it is doing the other way around for Aqua L It is defaulting 
WRAP_TAB_LAYOUT and setting SCROLL_TAB_LAYOUT is still setting a subclass of 
TabbedPaneLayout and not TabbedPaneScrollLayout.

   According to the JTabbedPane javadoc:
  SCROLL_TAB_LAYOUT: Tab layout policy for providing a subset of available tabs 
when all the tabs will not fit within a single run.
  WRAP_TAB_LAYOUT The tab layout policy for wrapping tabs in multiple runs when 
all tabs will not fit within a single run.
 
   The Aqua L uses only AquaTabbedPaneUI and AquaTabbedPaneContrastUI for 
tabbed pane UI and they both returns AquaTruncatingTabbedPaneLayout which has 
been designed for to place tabs according to the SCROLL_TAB_LAYOUT.

  Thanks,
  Alexandr.









  Thanks,
  Alexandr.





instead of absorbing this parameter and letting it render itself into a dummy 
node which does not proceed further with this parameter? Maybe we need to 
discuss what the expected behaviour may be. Also, thank you for the inputs 
regarding how to proceed with removing duplicate code.

 

With Regards,

Avik Niyogi

On 19-Mar-2016, at 1:52 am, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


I would think about something like:
-
public class TabbedPaneLayout implements LayoutManager {

protected int basePreferredTabAreaWidth(final int tabPlacement, final 
int height) {
// TabbedPaneLayout preferredTabAreaWidth implementation
}

protected int truncatingPreferredTabAreaWidth(final int tabPlacement, 
final int height) {
if (tabPlacement ==

Re: Review Request for 6439354 : Win L: TitledBorder colors are not from desktop

2016-03-24 Thread Rajeev Chamyal

Looks good to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Semyon Sadetsky 
Sent: 24 March 2016 11:45
To: Prem Balakrishnan; Sergey Bylokhov; Rajeev Chamyal; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re: Review Request for 6439354 : Win L: TitledBorder colors are not 
from desktop

Looks good.

--Semyon

On 3/22/2016 11:56 AM, Prem Balakrishnan wrote:
> Hi Sergey,
>
> Updated test as per the review comments.
>
> Webrev: http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.01/
>
> Regards,
> Prem
>
>
> -Original Message-
> From: Sergey Bylokhov
> Sent: Monday, March 21, 2016 7:31 PM
> To: Prem Balakrishnan; Semyon Sadetsky; Rajeev Chamyal; Alexander 
> Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re: Review Request for 6439354 : Win L: TitledBorder colors 
> are not from desktop
>
> Hi, Prem.
> In the test the Swing components accessed on non-EDT thread. Probably the 
> test can be simplified further?
>
> On 21.03.16 11:20, Prem Balakrishnan wrote:
>> Hi*,*
>>
>> Please review fix for JDK 9,
>>
>> *Bug: *https://bugs.openjdk.java.net/browse/JDK-6439354 **
>>
>> *Webrev: *http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.00/
>>
>> *Issue:*
>>
>> Win L: TitledBorder colors are not from desktop
>>
>> *Cause:*
>>
>> "TitledBorder.border" is set to EtchedBorder with default 
>> highlight/shadow colors.
>>
>> *Fix:*
>>
>> "TitledBorder.border" is set to  EtchedBorder with Desktop
>>highlight/shadow colors.
>>
>> **
>>
>> *Test: *Manual Test (since windows theme to be set)**
>>
>> **
>>
>> Regards,
>> Prem
>>
>
> --
> Best regards, Sergey.

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

2016-03-24 Thread Avik Niyogi

Hi All,
Please review code changes as per inputs received. 

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.03 


As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

With Regards,
Avik Niyogi

> On 24-Mar-2016, at 12:34 pm, Rajeev Chamyal  wrote:
> 
> Hello Avik,
>  
> x variable on line 2195 is not used anywhere. Do we need for loop also?
>  
> Regards,
> Rajeev Chamyal
>  
> From: Avik Niyogi 
> Sent: 24 March 2016 12:19
> To: Alexander Scherbatiy
> Cc: Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
> Subject: Re:  Review Request of 8137169 : [macosx] Incorrect 
> minimal heigh of JTabbedPane with more tabs
>  
> Hi All,
>  
> Please review my code changes below as per the inputs received.
>  
> http://cr.openjdk.java.net/~aniyogi/8137169/webrev.02/ 
> 
>  
> As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
> within the derived AquaTruncatedTabbedPaneLayout, extending of 
> TabbedPaneScrollLayout is not needed and management of row and column height 
> is done within it itself. Hence preferredTabAreaWidth and 
> preferredTabAreaHeight need not manage the number of columns and rows 
> respectively and will remain 1 as tabs can be brought forth to the UI by the 
> arrow buttons AQUA provides instead of placing them in another row or column. 
> This is also the expected behaviour for AquaLAF as per javadoc.
>  
> With Regards,
> Avik Niyogi
>  
>  
> On 23-Mar-2016, at 4:14 pm, Alexander Scherbatiy 
> > 
> wrote:
>  
> On 23/03/16 14:07, Avik Niyogi wrote:
> 
>  
> On 23-Mar-2016, at 3:31 pm, Alexander Scherbatiy 
> > 
> wrote:
>  
> On 21/03/16 09:19, Avik Niyogi wrote:
> 
> Hi Alexander, 
> I agree with what you said regarding the look and feel looking different. But 
> this bug arrises due to setting of TabbedPaneScrollLayout only. If Scroll 
> Layout is not meant for Aqua look and feel should not the setting of this 
> parameter instead throw a helpful error saying this parameter is not accepted
>   According to the JTabbedPane.setTabLayoutPolicy(int tabLayoutPolicy) 
> javadoc:  
>   "Some look and feels might only support a subset of the possible layout 
> policies, in which case the value of this property may be ignored."
>   
>   Aqua L ignores WRAP_TAB_LAYOUT for JTabbedPane tabs layouting and always 
> use SCROLL_TAB_LAYOUT. No exception should be thrown in this case.
>  
> Actually, it is doing the other way around for Aqua L It is defaulting 
> WRAP_TAB_LAYOUT and setting SCROLL_TAB_LAYOUT is still setting a subclass of 
> TabbedPaneLayout and not TabbedPaneScrollLayout.
>According to the JTabbedPane javadoc:
>   SCROLL_TAB_LAYOUT: Tab layout policy for providing a subset of available 
> tabs when all the tabs will not fit within a single run.
>   WRAP_TAB_LAYOUT The tab layout policy for wrapping tabs in multiple runs 
> when all tabs will not fit within a single run.
>  
>The Aqua L uses only AquaTabbedPaneUI and AquaTabbedPaneContrastUI for 
> tabbed pane UI and they both returns AquaTruncatingTabbedPaneLayout which has 
> been designed for to place tabs according to the SCROLL_TAB_LAYOUT.
> 
>   Thanks,
>   Alexandr.
> 
> 
> 
>   Thanks,
>   Alexandr.
> 
> 
> instead of absorbing this parameter and letting it render itself into a dummy 
> node which does not proceed further with this parameter? Maybe we need to 
> discuss what the expected behaviour may be. Also, thank you for the inputs 
> regarding how to proceed with removing duplicate code.
>  
> With Regards,
> Avik Niyogi
> On 19-Mar-2016, at 1:52 am, Alexander Scherbatiy 
> > 
> wrote:
>  
> 
> I would think about something like:
> -
> public class TabbedPaneLayout implements LayoutManager {
> 
> protected int basePreferredTabAreaWidth(final int tabPlacement, final 
> int height) {
> // TabbedPaneLayout preferredTabAreaWidth implementation
> }
> 
> protected int truncatingPreferredTabAreaWidth(final int tabPlacement, 
> final int height) {
> if (tabPlacement == SwingConstants.LEFT || tabPlacement == 
> SwingConstants.RIGHT) {
> return

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

2016-03-24 Thread Rajeev Chamyal

Hello Avik,

 

x variable on line 2195 is not used anywhere. Do we need for loop also?

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 24 March 2016 12:19
To: Alexander Scherbatiy
Cc: Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8137169 : [macosx] Incorrect minimal 
heigh of JTabbedPane with more tabs

 

Hi All,

 

Please review my code changes below as per the inputs received.

 

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.02/

 

As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

 

With Regards,

Avik Niyogi

 

 

On 23-Mar-2016, at 4:14 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 23/03/16 14:07, Avik Niyogi wrote:



 

On 23-Mar-2016, at 3:31 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 21/03/16 09:19, Avik Niyogi wrote:



Hi Alexander, 

I agree with what you said regarding the look and feel looking different. But 
this bug arrises due to setting of TabbedPaneScrollLayout only. If Scroll 
Layout is not meant for Aqua look and feel should not the setting of this 
parameter instead throw a helpful error saying this parameter is not accepted

  According to the JTabbedPane.setTabLayoutPolicy(int tabLayoutPolicy) javadoc: 
 
  "Some look and feels might only support a subset of the possible layout 
policies, in which case the value of this property may be ignored."
  
  Aqua L ignores WRAP_TAB_LAYOUT for JTabbedPane tabs layouting and always 
use SCROLL_TAB_LAYOUT. No exception should be thrown in this case.

 

Actually, it is doing the other way around for Aqua L It is defaulting 
WRAP_TAB_LAYOUT and setting SCROLL_TAB_LAYOUT is still setting a subclass of 
TabbedPaneLayout and not TabbedPaneScrollLayout.

   According to the JTabbedPane javadoc:
  SCROLL_TAB_LAYOUT: Tab layout policy for providing a subset of available tabs 
when all the tabs will not fit within a single run.
  WRAP_TAB_LAYOUT The tab layout policy for wrapping tabs in multiple runs when 
all tabs will not fit within a single run.
 
   The Aqua L uses only AquaTabbedPaneUI and AquaTabbedPaneContrastUI for 
tabbed pane UI and they both returns AquaTruncatingTabbedPaneLayout which has 
been designed for to place tabs according to the SCROLL_TAB_LAYOUT.

  Thanks,
  Alexandr.







  Thanks,
  Alexandr.




instead of absorbing this parameter and letting it render itself into a dummy 
node which does not proceed further with this parameter? Maybe we need to 
discuss what the expected behaviour may be. Also, thank you for the inputs 
regarding how to proceed with removing duplicate code.

 

With Regards,

Avik Niyogi

On 19-Mar-2016, at 1:52 am, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


I would think about something like:
-
public class TabbedPaneLayout implements LayoutManager {

protected int basePreferredTabAreaWidth(final int tabPlacement, final 
int height) {
// TabbedPaneLayout preferredTabAreaWidth implementation
}

protected int truncatingPreferredTabAreaWidth(final int tabPlacement, 
final int height) {
if (tabPlacement == SwingConstants.LEFT || tabPlacement == 
SwingConstants.RIGHT) {
return basePreferredTabAreaWidth(tabPlacement, height);
}

return basePreferredTabAreaWidth(tabPlacement, height);
}

protected int preferredTabAreaWidth(final int tabPlacement, final int 
height) {
return basePreferredTabAreaWidth(tabPlacement, height);
}

}

class TabbedPaneScrollLayout extends TabbedPaneLayout {
@Override
protected int basePreferredTabAreaWidth(int tabPlacement, int height) {
// TabbedPaneScrollLayout preferredTabAreaWidth implementation
}
}

protected class AquaTruncatingTabbedPaneLayout extends 
AquaTabbedPaneCopyFromBasicUI.TabbedPaneLayout {

@Override
protected int preferredTabAreaWidth(final int tabPlacement, final int 
height) {
return truncatingPreferredTabAreaWidth(tabPlacement, height);
}
}

protected class AquaTruncatingTabbedScrollPaneLayout extends 
AquaTabbedPaneCopyFromBasicUI.TabbedPaneScrollLayout {

@Override
protected int preferredTabAreaWidth(final int tabPlacement, final int 
height) {

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

2016-03-24 Thread Avik Niyogi

Hi All,

Please review my code changes below as per the inputs received.

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.02/ 


As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

With Regards,
Avik Niyogi


> On 23-Mar-2016, at 4:14 pm, Alexander Scherbatiy 
>  wrote:
> 
> On 23/03/16 14:07, Avik Niyogi wrote:
>> 
>>> On 23-Mar-2016, at 3:31 pm, Alexander Scherbatiy 
>>> > 
>>> wrote:
>>> 
>>> On 21/03/16 09:19, Avik Niyogi wrote:
 Hi Alexander,
 I agree with what you said regarding the look and feel looking different. 
 But this bug arrises due to setting of TabbedPaneScrollLayout only. If 
 Scroll Layout is not meant for Aqua look and feel should not the setting 
 of this parameter instead throw a helpful error saying this parameter is 
 not accepted
>>>   According to the JTabbedPane.setTabLayoutPolicy(int tabLayoutPolicy) 
>>> javadoc:  
>>>   "Some look and feels might only support a subset of the possible layout 
>>> policies, in which case the value of this property may be ignored."
>>>   
>>>   Aqua L ignores WRAP_TAB_LAYOUT for JTabbedPane tabs layouting and 
>>> always use SCROLL_TAB_LAYOUT. No exception should be thrown in this case.
>> 
>> Actually, it is doing the other way around for Aqua L It is defaulting 
>> WRAP_TAB_LAYOUT and setting SCROLL_TAB_LAYOUT is still setting a subclass of 
>> TabbedPaneLayout and not TabbedPaneScrollLayout.
>According to the JTabbedPane javadoc:
>   SCROLL_TAB_LAYOUT: Tab layout policy for providing a subset of available 
> tabs when all the tabs will not fit within a single run.
>   WRAP_TAB_LAYOUT The tab layout policy for wrapping tabs in multiple runs 
> when all tabs will not fit within a single run.
>  
>The Aqua L uses only AquaTabbedPaneUI and AquaTabbedPaneContrastUI for 
> tabbed pane UI and they both returns AquaTruncatingTabbedPaneLayout which has 
> been designed for to place tabs according to the SCROLL_TAB_LAYOUT.
> 
>   Thanks,
>   Alexandr.
>> 
>>>   Thanks,
>>>   Alexandr.
>>> 
 instead of absorbing this parameter and letting it render itself into a 
 dummy node which does not proceed further with this parameter? Maybe we 
 need to discuss what the expected behaviour may be. Also, thank you for 
 the inputs regarding how to proceed with removing duplicate code.
 
 With Regards,
 Avik Niyogi
> On 19-Mar-2016, at 1:52 am, Alexander Scherbatiy < 
> alexandr.scherba...@oracle.com 
> > wrote:
> 
> 
> I would think about something like:
> -
> public class TabbedPaneLayout implements LayoutManager {
> 
> protected int basePreferredTabAreaWidth(final int tabPlacement, 
> final int height) {
> // TabbedPaneLayout preferredTabAreaWidth implementation
> }
> 
> protected int truncatingPreferredTabAreaWidth(final int 
> tabPlacement, final int height) {
> if (tabPlacement == SwingConstants.LEFT || tabPlacement == 
> SwingConstants.RIGHT) {
> return basePreferredTabAreaWidth(tabPlacement, height);
> }
> 
> return basePreferredTabAreaWidth(tabPlacement, height);
> }
> 
> protected int preferredTabAreaWidth(final int tabPlacement, final 
> int height) {
> return basePreferredTabAreaWidth(tabPlacement, height);
> }
> 
> }
> 
> class TabbedPaneScrollLayout extends TabbedPaneLayout {
> @Override
> protected int basePreferredTabAreaWidth(int tabPlacement, int 
> height) {
> // TabbedPaneScrollLayout preferredTabAreaWidth implementation
> }
> }
> 
> protected class AquaTruncatingTabbedPaneLayout extends 
> AquaTabbedPaneCopyFromBasicUI.TabbedPaneLayout {
> 
> @Override
> protected int preferredTabAreaWidth(final int tabPlacement, final 
> int height) {
> return truncatingPreferredTabAreaWidth(tabPlacement, height);
> }
> }
> 
> protected class AquaTruncatingTabbedScrollPaneLayout extends 
>

Re: [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

2016-03-24 Thread Semyon Sadetsky


Hi Alexander,

Could you answer one question:
Why did you choose default interface methods to implement TextUIDrawing 
and not implement them in DefaultTextUIDrawing having declarations only 
in the interface?
AFAIK the common point of view is default methods should be used rarely 
because they may lead to unreadable code.


--Semyon

On 3/18/2016 6:49 PM, Alexander Scherbatiy wrote:


Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8132119/webrev.08/

  - Public TextUIDrawing interface is added to the javax.swing.plaf 
package
  - TextUIDrawing methods description does not mention component 
properties to be more general

  - TextUIDrawing methods are made default
  - L sets an instance of the TextUIDrawing to look and feel 
defaults using "uiDrawing.text" property

  - ComponentUI class is not changed
  - Each ComponentUI reads TextUIDrawing from UI defaults
  - There is an interesting issue described in
http://mail.openjdk.java.net/pipermail/swing-dev/2016-March/005509.html
which is related to the fact that MetalLabelUI returns a static 
field from createUI() method.
TitleBorder creates a JLabel but does not put it to any component 
hierarchy. In this case SwingUtilities.updateComponentTreeUI() method 
calls MetalLabelUI.uninstallDefaults() on the static metalLabelUI 
field and sets a new LabelUI for ordinary labels. The TitleBorder 
label UI is not changed in this case and it still uses the 
metalLabelUI field which is not initialized.
It seems that other applications can also use components just for 
drawing and have the same issue.
For this case the textUIDrawing field is not cleared in the 
uninstallDefaults but just set to a static default value which should 
not lead to memory leaks.


  Thanks,
  Alexandr.

On 29/01/16 19:51, Alexander Scherbatiy wrote:

On 25/01/16 13:44, Andrej Golovnin wrote:

Hi Alexandr,


Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8132119/webrev.07/


- public TextUIDrawing interface is added to the javax.swing.plaf 
package

- public "TextUIDrawing getTextUIDrawing()" method is added to the
ComponentUI class
- L sets an instance of the TextUIDrawing to look and feel 
defaults using

"uiDrawing.text" property
- Look and Feel delegates use the instance of the TextUIDrawing for 
text

drawing and measuring

Some thoughts on the current design/implementation:

By adding a field to the ComponentUI class the current 
implementation increases
memory consumption for all Swing applications. And you get the 
feeling that
there are different implementations of TextUIDrawing per ComponentUI 
instances.

Personally I can't imagine to have different implementations of
TextUIDrawing for
a given LookAndFeel. If I would design/implement it, then I would
implement it as
a property of the LookAndFeel class (similar to LayoutStyle) and not
the ComponentUI.
Developers can use then the following code to obtain the instance of
TextUIDrawing:

UIManager.getLookAndFeel().getUIDrawing() // or
UIManager.getLookAndFeelUIDrawing() // use this static method as a
short cut for the line above.
  LayoutStyle keeps its instance per App context. The same is for the 
LookAndFeel

  when it is got through UIManager.getLookAndFeel() call.
  It means that accessing an instance of a TextUIDrawing will leads 
to a time consumption.


  There are 3 main ways of the SwingUtilities2.drawString(...) usage:
  1. ComponentUI classes
  2. Components created in UI (like BasicInternalFrameTitlePane)
  3. Public utilities methods (like WindowsGraphicsUtils.paintText())

  For the cases 1 and 2 it is possible to load and store the 
UIDrawing instance during installUI()/updateUI() calls to decrease a 
time access to it.


  For the case 3 it is necessary to get LookAndFeel instance each 
time (which is taken from an App context)
  or use the passed JComponent object. It requires to have a public 
method and the associated variable for each instance of 
JComponent/ComponentUI/... class.

You can use this methods then in JDK too.

And maybe rename the TextUIDrawing class to just UIDrawing and add
more useful methods,
e.g. a method to create a composite font, a method to convert DLUs 
to pixels.
  UIDrawing name may look like it should be used for any UI drawing, 
not only for text ones. I am afraid that it can be misleading.


  Thanks,
  Alexandr.



Best regards,
Andrej Golovnin

Re: Review Request for 6439354 : Win L: TitledBorder colors are not from desktop

2016-03-24 Thread Semyon Sadetsky


Looks good.

--Semyon

On 3/22/2016 11:56 AM, Prem Balakrishnan wrote:

Hi Sergey,

Updated test as per the review comments.

Webrev: http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.01/

Regards,
Prem


-Original Message-
From: Sergey Bylokhov
Sent: Monday, March 21, 2016 7:31 PM
To: Prem Balakrishnan; Semyon Sadetsky; Rajeev Chamyal; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re: Review Request for 6439354 : Win L: TitledBorder colors are not 
from desktop

Hi, Prem.
In the test the Swing components accessed on non-EDT thread. Probably the test 
can be simplified further?

On 21.03.16 11:20, Prem Balakrishnan wrote:

Hi*,*

Please review fix for JDK 9,

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

*Webrev: *http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.00/

*Issue:*

Win L: TitledBorder colors are not from desktop

*Cause:*

"TitledBorder.border" is set to EtchedBorder with default
highlight/shadow colors.

*Fix:*

"TitledBorder.border" is set to  EtchedBorder with Desktop
   highlight/shadow colors.

**

*Test: *Manual Test (since windows theme to be set)**

**

Regards,
Prem



--
Best regards, Sergey.

Re: [9] Review Request for several test bugs: 8150535, 8151033, 8151037 etc.

Re: [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

Re: [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

Re: Review Request for 6439354 : Win L: TitledBorder colors are not from desktop

Re: CFV: New Swing Group Member: Semyon Sadetsky

[9] JDK-8049069 : JButton incorrect behaviour on button release

Re: [9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

Re: Review Request for 6439354 : Win L: TitledBorder colors are not from desktop

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

Re: [9] Review request for 8132119 Provide public API for text related methods in SwingUtilities2

Re: Review Request for 6439354 : Win L: TitledBorder colors are not from desktop

14 matches

Site Navigation

Mail list logo

Footer information