keyboard (not mouse) multi-select on Mac

[Pete Brunet]

How do you do a mouseless (keyboard only) multi-select on Mac, e.g. in
the List Demo of SwingSet2?  On Win, to move the focus point without
moving the selection, you use ctrl + arrow.  Then when you are at the
next list item you want to select you use ctrl + space.  I tried arrow
with control, command, and option with no luck.

Control + up/down are normally Mac Mission Control shortcuts, but
disabling those does not resolve the issue.

Is this a bug?

BTW, command + mouse click does work.

Pete

Re: [9] Review request for 8078268: javax.swing.text.html.parser.Parser parseScript incorrectly optimized

[mikhail cherkasov]

Please see update version:
http://cr.openjdk.java.net/~mcherkas/8078268/webrev.02
>You don't need to create JEditorPane, you can instantiate HTMLDocument 
directly with new HTMLDocument().
without JEditorPane parser will be null in HTMLDocument, so it's the 
easiest way to init HTMLDocument right



Hi Mikhail,

Can I ask you to add one space between if, while and the opening 
parenthesis which is in according to Java Coding Style?


In your test, you use none of JFrame methods, then why do you extend 
JFrame?


You don't need to create JEditorPane, you can instantiate HTMLDocument 
directly with new HTMLDocument().


An exception thrown from Runnable in invokeLater won't stop the test, 
and the test could fail because of the timeout. I suggest storing 
IOException to a static volatile field in the catch block. If the 
exception is not null, the main thread breaks from waiting 'while' and 
rethrows the exception.


Regards,
Alexey

On 11.05.2016 17:20, mikhail cherkasov wrote:

Hi all,

Please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8078268
webrev:
http://cr.openjdk.java.net/~mcherkas/8078268/webrev.01/

The problem is in the following line:
http://hg.openjdk.java.net/jdk9/client/jdk/file/597626072716/src/java.desktop/share/classes/javax/swing/text/html/parser/Parser.java#l2127 

for each new char it creates a string with a whole text inside the 
script tag, so it's very expensive operation.
I added check for   by reading chars from input stream 
like it was done for end script tag.


Thanks,
Mikhail.

Re: [9] Review request for 8078268: javax.swing.text.html.parser.Parser parseScript incorrectly optimized

[Alexey Ivanov]

Hi Mikhail,

Can I ask you to add one space between if, while and the opening 
parenthesis which is in according to Java Coding Style?


In your test, you use none of JFrame methods, then why do you extend JFrame?

You don't need to create JEditorPane, you can instantiate HTMLDocument 
directly with new HTMLDocument().


An exception thrown from Runnable in invokeLater won't stop the test, 
and the test could fail because of the timeout. I suggest storing 
IOException to a static volatile field in the catch block. If the 
exception is not null, the main thread breaks from waiting 'while' and 
rethrows the exception.


Regards,
Alexey

On 11.05.2016 17:20, mikhail cherkasov wrote:

Hi all,

Please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8078268
webrev:
http://cr.openjdk.java.net/~mcherkas/8078268/webrev.01/

The problem is in the following line:
http://hg.openjdk.java.net/jdk9/client/jdk/file/597626072716/src/java.desktop/share/classes/javax/swing/text/html/parser/Parser.java#l2127 

for each new char it creates a string with a whole text inside the 
script tag, so it's very expensive operation.
I added check for   by reading chars from input stream 
like it was done for end script tag.


Thanks,
Mikhail.

Re: [9] Review request for 8041694: JFileChooser removes trailing spaces in the selected directory name

[Anton Litvinov]

Hi Semyon,

Thank you for this information. Current version of the regression test 
using the shell script is tested, cross platform and does not contain 
any code specific to Windows platform. This shell script is a copy of 
many other stable regression tests existing in "jdk9/jdk/test" directory 
and is different from them only in its commented test header (jtreg 
options like: @summary, @author) and 7 code lines between 
"###  YOUR TEST CODE HERE!!! #" and 
"###  END YOUR TEST CODE ! ".


What is so bad in usage of the well established approach based on shell 
script?


Thank you,
Anton

On 5/11/2016 5:29 PM, Semyon Sadetsky wrote:

Hi Anton,

In windows you may use "\\?\" prefix with absolute path of a new folder.
For example:
new File("?\\C:\\tmp\\test2 ").mkdir();

--Semyon

On 5/11/2016 4:47 PM, Anton Litvinov wrote:

Hello Sergey,

Thank you for review of this fix. No, unfortunately, on MS Windows 
OS, if the method "java.io.File.mkdir()" is called on "java.io.File" 
instance which contains trailing space characters in the directory 
name, the corresponding directory is created but without trailing 
space characters in its name in file system.


The method "java.nio.file.Files.createDirectory(Path dir, 
FileAttribute... attrs)" cannot be used for this purpose also, 
because "java.nio.file.Path" cannot be constructed for the directory 
name ending with spaces and "java.io.File.toPath()" throws the 
exception "java.nio.file.InvalidPathException: Trailing char < > at 
index N: ".


It is possible to create a directory with such a name from the shell 
script on MS Windows OS, therefore I decided to use the shell script 
for this regression test.


Thank you,
Anton

On 5/11/2016 4:16 PM, Sergey Bylokhov wrote:

Hi, Anton.
Probably the test can create the folder w/o the shell script?

On 11.05.16 15:14, Anton Litvinov wrote:

The bug consists in the fact that the method
"JFileChooser.getSelectedFile()" returns "java.io.File" object which
does not contain trailing spaces in the directory name, in spite of 
the

fact that the corresponding directory in the file system has trailing
spaces in its name. The fix deletes the code in the method
"javax.swing.plaf.basic.BasicFileChooserUI.ApproveSelectionAction.actionPerformed" 


which deliberately modifies the selected directory string name by
removing trailing spaces from it.

All automatic regression tests from open and closed sets located in
"javax/swing/JFileChooser" directories were run on MS Windows 7 OS
during verification of the fix.

Re: [9] Review request for 8041694: JFileChooser removes trailing spaces in the selected directory name

[Semyon Sadetsky]

Hi Anton,

In windows you may use "\\?\" prefix with absolute path of a new folder.
For example:
new File("?\\C:\\tmp\\test2 ").mkdir();

--Semyon

On 5/11/2016 4:47 PM, Anton Litvinov wrote:

Hello Sergey,

Thank you for review of this fix. No, unfortunately, on MS Windows OS, 
if the method "java.io.File.mkdir()" is called on "java.io.File" 
instance which contains trailing space characters in the directory 
name, the corresponding directory is created but without trailing 
space characters in its name in file system.


The method "java.nio.file.Files.createDirectory(Path dir, 
FileAttribute... attrs)" cannot be used for this purpose also, 
because "java.nio.file.Path" cannot be constructed for the directory 
name ending with spaces and "java.io.File.toPath()" throws the 
exception "java.nio.file.InvalidPathException: Trailing char < > at 
index N: ".


It is possible to create a directory with such a name from the shell 
script on MS Windows OS, therefore I decided to use the shell script 
for this regression test.


Thank you,
Anton

On 5/11/2016 4:16 PM, Sergey Bylokhov wrote:

Hi, Anton.
Probably the test can create the folder w/o the shell script?

On 11.05.16 15:14, Anton Litvinov wrote:

The bug consists in the fact that the method
"JFileChooser.getSelectedFile()" returns "java.io.File" object which
does not contain trailing spaces in the directory name, in spite of the
fact that the corresponding directory in the file system has trailing
spaces in its name. The fix deletes the code in the method
"javax.swing.plaf.basic.BasicFileChooserUI.ApproveSelectionAction.actionPerformed" 


which deliberately modifies the selected directory string name by
removing trailing spaces from it.

All automatic regression tests from open and closed sets located in
"javax/swing/JFileChooser" directories were run on MS Windows 7 OS
during verification of the fix.

[9] Review request for 8136998: JComboBox prevents wheel mouse scrolling of JScrollPane

[Alexey Ivanov]

Hello Swing team,

Could you please review the fix for jdk9:
bug: https://bugs.openjdk.java.net/browse/JDK-8136998
webrev: http://cr.openjdk.java.net/~aivanov/8136998/jdk9/webrev.00/


Problem description:
When JComboBox is added into a JScrollPane, scroll pane is not scrolled 
by mouse wheel if mouse over JComboBox.


The fix to https://bugs.openjdk.java.net/browse/JDK-8033069 added 
MouseWheelListener to JComboBox, and combo box consumes MOUSE_WHEEL 
events therefore these events do not reach JScrollPane beneath JComboBox.


The fix:
Remove MouseWheelListener from JComboBox. To prevent the combo box popup 
from being closed by rotating mouse wheel over the JComboBox, 
MouseWheelListener is added when the popup is about to be displayed and 
is removed when the popup is about to be hidden.



Regards,
Alexey

[9] Review request for 8078268: javax.swing.text.html.parser.Parser parseScript incorrectly optimized

[mikhail cherkasov]

Hi all,

Please review a fix for
https://bugs.openjdk.java.net/browse/JDK-8078268
webrev:
http://cr.openjdk.java.net/~mcherkas/8078268/webrev.01/

The problem is in the following line:
http://hg.openjdk.java.net/jdk9/client/jdk/file/597626072716/src/java.desktop/share/classes/javax/swing/text/html/parser/Parser.java#l2127
for each new char it creates a string with a whole text inside the 
script tag, so it's very expensive operation.
I added check for   by reading chars from input stream like 
it was done for end script tag.


Thanks,
Mikhail.

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

[Alexander Scherbatiy]

  Hello,

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

   Public methods to handle fractional text position are added to Swing.
   - methods which use integer position now has a pair with the same 
name and fractional position
   - methods which use Rectangle and Point objects have corresponding 
methods with 2D suffix.
 The DefaultCaret extends directly Rectangle so there is no way to 
change it to use Rectangle2D without breaking a backward compatibility.

 The new DefaultCaret2D is a subject of a separate fix.
  - methods which only returns integer value have corresponding methods 
with 'fractional' prefix


  The new methods are added only as stubs now. The real implementation 
will be provided in the corresponding fixes like:

http://mail.openjdk.java.net/pipermail/swing-dev/2016-May/005828.html

  The discussed problem that text width can differ for different 
graphics transforms is filled as:

JDK-8156758 Text width depends on the graphics transform
https://bugs.openjdk.java.net/browse/JDK-8156758

  Thanks,
  Alexandr.

On 4/28/2016 2:31 PM, Sergey Bylokhov wrote:

On 28.04.16 12:20, Sergey Bylokhov wrote:

The problem below and a suggestion of using float has the similar
symptoms to the situation when fm-on, when actual rendering and metrics
in "int" are different. The suggestion of using float is correct, but
why it should be used when fm-off? according to the spec all metrics in
this case should be calculated as "int". I still think that all these
problems are occurs because we incorrectly calculate "int" metrics when
fm-off(actually from the users point of view fm-off mode is useless,
because even if fm-off the application get fractional data instead of
expected ints).


I suggest to implement it like on OSX, then tweak the algorithm of 
rounding, and if after that someone will complains then add a new 
render hint to round the text according the user space.






When Swing detects complex text it turns over editing and highlighting
to TextLayout, does it not ?
Perhaps doing the same for hi-dpi would work as well.

-phil.

On 04/27/2016 01:41 PM, Alexandr Scherbatiy wrote:



  I just want to highlight one more problem which happen even font
metrics with right transform are used.
  It refers to the text selection.

  Suppose there is a graphics with scale 2. The char 'a' advance can
be 13 in device space and 13 / 2 = 6.5 in user space.
  The task is to highlight a selected text in the middle of a string

  Let's the selected index will be 11. The x coordinate to draw the
selected text is FontMetrics.charsWidth(chars, 0, index) = 72.
   The selected text drawn from this position will be shifted to the
right because real x coordinate in user space is 6.5 * 11 = 71.5.
  This leads that text is jumping when it is selected and deselected.

  To properly draw a selected text in this case there should be API
which allows both return float chars width and draw a text from float
position.
  But we have only API which gets a chars width and draws a text from
int position in the user space.

  Below is a code which illustrate the text selection issue.
  The image shows two strings which are selected from index 10 and 11.
The second selection is shifted to the right:
http://cr.openjdk.java.net/~alexsch/8142966/images/text-selection.png

  
int imgWidth = 400;
int imgHeight = 150;

BufferedImage buffImage = new BufferedImage(imgWidth, 
imgHeight,

BufferedImage.TYPE_INT_RGB);

Graphics2D g = buffImage.createGraphics();
g.setColor(Color.WHITE);
g.fillRect(0, 0, imgWidth, imgHeight);

int x = 10;
int y1 = 20;
int y2 = 40;

g.scale(2, 2);
String text = "";
g.setColor(Color.BLUE);
char[] chars = text.toCharArray();
g.drawChars(chars, 0, chars.length, x, y1);
g.drawChars(chars, 0, chars.length, x, y2);

int selIndex = 10;
int selNum = 6;

FontMetrics fm = g.getFontMetrics(g.getFont());
g.setColor(Color.RED);
g.drawChars(chars, selIndex, selNum, x + fm.charsWidth(chars,
0, selIndex), y1);
selIndex++;
g.drawChars(chars, selIndex, selNum, x + fm.charsWidth(chars,
0, selIndex), y2);

g.dispose();
  


On 4/27/2016 6:17 AM, Philip Race wrote:

Applications cannot change the device transform and expect the same
user space
metrics unless they specify fractional metrics to be ON.

In your example below you may already have a scaled graphics context
if you were printing (for example).

If you want no change to the user space positions or [as also
implied] shapes of the glyphs
as the transform changes then you are asking for text that will not
look
right at other resolutions. Printed text will look poorly spaced.

Changing what happens in the font system is not an option since 

Re: [9] Review request for 8041694: JFileChooser removes trailing spaces in the selected directory name

[Sergey Bylokhov]

On 11.05.16 16:47, Anton Litvinov wrote:

The method "java.nio.file.Files.createDirectory(Path dir,
FileAttribute... attrs)" cannot be used for this purpose also,
because "java.nio.file.Path" cannot be constructed for the directory
name ending with spaces and "java.io.File.toPath()" throws the exception
"java.nio.file.InvalidPathException: Trailing char < > at index N:
".


Then how the File which was returned from getSelectedFile() can be 
used(what operations will be allowed?) by the application if such files 
are unsupported by the corelib?




It is possible to create a directory with such a name from the shell
script on MS Windows OS, therefore I decided to use the shell script for
this regression test.


Note sure that the case for windows correct:
Windows_95 | Windows_98 |  Windows_NT | Windows_ME


--
Best regards, Sergey.

Re: [9] Review request for 8041694: JFileChooser removes trailing spaces in the selected directory name

[Anton Litvinov]

Hello Sergey,

Thank you for review of this fix. No, unfortunately, on MS Windows OS, 
if the method "java.io.File.mkdir()" is called on "java.io.File" 
instance which contains trailing space characters in the directory name, 
the corresponding directory is created but without trailing space 
characters in its name in file system.


The method "java.nio.file.Files.createDirectory(Path dir, 
FileAttribute... attrs)" cannot be used for this purpose also, 
because "java.nio.file.Path" cannot be constructed for the directory 
name ending with spaces and "java.io.File.toPath()" throws the exception 
"java.nio.file.InvalidPathException: Trailing char < > at index N: 
".


It is possible to create a directory with such a name from the shell 
script on MS Windows OS, therefore I decided to use the shell script for 
this regression test.


Thank you,
Anton

On 5/11/2016 4:16 PM, Sergey Bylokhov wrote:

Hi, Anton.
Probably the test can create the folder w/o the shell script?

On 11.05.16 15:14, Anton Litvinov wrote:

The bug consists in the fact that the method
"JFileChooser.getSelectedFile()" returns "java.io.File" object which
does not contain trailing spaces in the directory name, in spite of the
fact that the corresponding directory in the file system has trailing
spaces in its name. The fix deletes the code in the method
"javax.swing.plaf.basic.BasicFileChooserUI.ApproveSelectionAction.actionPerformed" 


which deliberately modifies the selected directory string name by
removing trailing spaces from it.

All automatic regression tests from open and closed sets located in
"javax/swing/JFileChooser" directories were run on MS Windows 7 OS
during verification of the fix.

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Sergey Bylokhov]

Code change looks fine.
I think that the spec of isHeavyWeightPopup() should be updated. the 
"true" means that the HW popup will be forced, but the false does not 
mean that LW will be used(I guess in this case some default type 
will/can be selected).


On 11.05.16 14:45, Rajeev Chamyal wrote:

Hello Alexandr,

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.04/

Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy
Sent: 11 May 2016 16:29
To: Rajeev Chamyal
Cc: swing-dev@openjdk.java.net; Sergey Bylokhov; Alan Snyder
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:

Hello All,

Please review the updated webrev.
Bug : https://bugs.openjdk.java.net/browse/JDK-8147521

Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/

Update: Added a new protected method getpop in PopupFactory.java with a Boolean 
parameter for specifying heavy weight popup.


The provided method looks good to me.

I would leave the javadoc review to the native speakers. What I only see that there is a missed 
space in {@codey}, typo in "paramaters" word and probably there should be a comma before 
"otherwise false".

   Thanks,
   Alexandr.



Regards,
Rajeev Chamyal

-Original Message-
From: Alan Snyder [mailto:javali...@cbfiddle.com]
Sent: 11 May 2016 03:39
To: Alexandr Scherbatiy
Cc: swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx]
Internal API Usage: setPopupType used to force creation of heavyweight
popup

I use only heavy weight.



On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
 wrote:


Do you need to use medium-weight popups in your application or 
light/heavy-weight popups are enough?

Thanks,
Alexandr.

On 5/10/2016 10:23 PM, Alan Snyder wrote:

On May 10, 2016, at 5:58 AM, Sergey Bylokhov  wrote:

Hi, Alan.
Can you please take a look to the proposed solutions? Thanks.



Approach 2 matches what I currently do. The problem noted by Rajeev does not 
happen because my popup factory calls setPopupType() on each call to the public 
getPopup() before invoking the superclass method.

I think the original version of Approach 1 would work. My factory would 
override the public getPopup() method and pass true to the five parameter 
method.

I think the revised version of Approach 1 does not work for me because the new 
flag is only tested if the first attempt to create a popup fails.






--
Best regards, Sergey.

[9] Review request for 8041694: JFileChooser removes trailing spaces in the selected directory name

[Anton Litvinov]

Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8041694
Webrev: http://cr.openjdk.java.net/~alitvinov/8041694/jdk9/webrev.00

The bug consists in the fact that the method 
"JFileChooser.getSelectedFile()" returns "java.io.File" object which 
does not contain trailing spaces in the directory name, in spite of the 
fact that the corresponding directory in the file system has trailing 
spaces in its name. The fix deletes the code in the method 
"javax.swing.plaf.basic.BasicFileChooserUI.ApproveSelectionAction.actionPerformed" 
which deliberately modifies the selected directory string name by 
removing trailing spaces from it.


All automatic regression tests from open and closed sets located in 
"javax/swing/JFileChooser" directories were run on MS Windows 7 OS 
during verification of the fix.


Thank you,
Anton

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello Alexandr,

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.04/

Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy 
Sent: 11 May 2016 16:29
To: Rajeev Chamyal
Cc: swing-dev@openjdk.java.net; Sergey Bylokhov; Alan Snyder
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:
> Hello All,
>
> Please review the updated webrev.
> Bug : https://bugs.openjdk.java.net/browse/JDK-8147521
>
> Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/
>
> Update: Added a new protected method getpop in PopupFactory.java with a 
> Boolean parameter for specifying heavy weight popup.

The provided method looks good to me.

I would leave the javadoc review to the native speakers. What I only see 
that there is a missed space in {@codey}, typo in "paramaters" word and 
probably there should be a comma before "otherwise false".

   Thanks,
   Alexandr.

>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Alan Snyder [mailto:javali...@cbfiddle.com]
> Sent: 11 May 2016 03:39
> To: Alexandr Scherbatiy
> Cc: swing-dev@openjdk.java.net
> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
> Internal API Usage: setPopupType used to force creation of heavyweight 
> popup
>
> I use only heavy weight.
>
>
>> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
>>  wrote:
>>
>>
>> Do you need to use medium-weight popups in your application or 
>> light/heavy-weight popups are enough?
>>
>> Thanks,
>> Alexandr.
>>
>> On 5/10/2016 10:23 PM, Alan Snyder wrote:
 On May 10, 2016, at 5:58 AM, Sergey Bylokhov  
 wrote:

 Hi, Alan.
 Can you please take a look to the proposed solutions? Thanks.


>>> Approach 2 matches what I currently do. The problem noted by Rajeev does 
>>> not happen because my popup factory calls setPopupType() on each call to 
>>> the public getPopup() before invoking the superclass method.
>>>
>>> I think the original version of Approach 1 would work. My factory would 
>>> override the public getPopup() method and pass true to the five parameter 
>>> method.
>>>
>>> I think the revised version of Approach 1 does not work for me because the 
>>> new flag is only tested if the first attempt to create a popup fails.
>>>

Re: [9] Review request for 8154431: Allow source and target based validation for the focus transfer between two JComponents.

[Semyon Sadetsky]

On 5/11/2016 12:17 AM, Phil Race wrote:

On 04/22/2016 01:11 AM, Semyon Sadetsky wrote:
Forget to attach the link: 
http://cr.openjdk.java.net/~ssadetsky/8154431/webrev.01/


--Semyon

On 4/22/2016 11:08 AM, Semyon Sadetsky wrote:

Please review the updated webrev:


32 * need to ensure that the components are in valid state before 
allowing the "are in a .."


actually the class doc needs cleaning up and so does the example
but before we do that maybe there are some other things to discuss.

The fix is updated according to the reviewers comments:

Not all how I imagined it though

> BTW I see you mis-spell over-ridden as overriden in verifyTarget(..)

I see you did not fix this.
I misspelled over-ridden as over-riden. This is a new situation is not 
it? :)




- the test was updated to check the target object and year typo was 
updated

- new shouldYieldFocus() javadoc improved


I said this should be final but it still isn't.

Since the existing one is not final, applications may be relying on Swing
calling this and intercept its behaviour and making the new one final
will ensure that doesn't happen since it is not how the class is
supposed to be used.

Sorry, if I missed your point. I didn’t mean to.
But the old shouldYieldFocus() method is not final and that was not an 
issue to intercept it before (the interceptor pattern is the way how the 
validation usually applied). OK, it may be non-final by mistake. I don't 
argue.
But it may be necessary to change the default validation logic for 
source since the method accepts two args now and so there are no 
way to do this without over-riding it. Does it make sense?



- @depricated documentation is added to the old shouldYieldFocus()


125 * @depricated use {@link #shouldYieldFocus(JComponent, JComponent)}
126 * instead.

I said this should not be deprecated.
The point about the missing javadoc tag was just that you should
have neither or both. Also you mis-spelt this.
And advice to use the other one is confusing since only Swing
is expected to call this.
I agree that the method was supposed to be called from Swing only, but 
it had been being publicly non-final (as all other methods in this 
class), so it was not prohibited from over-riding or using it from the 
user code. Now the old  shouldYieldFocus() is not needed since it is not 
called by Swing anymore (its overloaded version is called instead) and 
now it only proxies the verify() method for the backward compatibility, 
because it might potentially be over-ridden or used in the user code 
before the fix.
Assuming that your expectations are correct and the method is not 
actually tied to any user code, then we are free to remove old 
shouldYieldFocus().


It needs additional re-specifying to note that the focus
is not transferred out if the verifyTarget() method returns false.

That is true for the basic implementation of the shouldYieldFocus() only.


Also I wonder if making verifyTarget independent is ideal
for people who want to verify both as they may want
a chance to see the src and target before they decide.
Right. That is why shouldYieldFocus() should not be final since one may 
need to validate both the source state in connection with the current 
target state in some cases. And it is the only place where we have both 
components.

With the proposed design they get two separate calls
hopefully close together but in an un-specified order
and have to piece together what is happening.

The following looks tempting

public boolean verify(JComponent src, JComponent dst) {
 
}

but then this independence was useful here as you were
able to maintain calling verify(JComponent)

If we try this

129 public boolean shouldYieldFocus(JComponent input) {
  130 return verify(input);
  131 }

public boolean verify(JComponent src, JComponent dst) {
   return shouldYieldFocus(src);
}

public final boolean shouldYieldFocus(JComponent source, JComponent 
target) {

return verify(src, dst);
  }


and a client over-rides verify(src, dst), where does that leave
the existing verify(src) which they have to implement as it is abstract ?
So that is unsatisfactory.

Stronger guarantees that we call verify() then verifyTarget() might help
a little. Or verify(src, dst) could be actually speced like 
"verifyTarget()" but

just take the additional parameter so developers can examine the src
before making a final decision. Maybe that is the way to go ?

The code would look like :-

public boolean verify(JComponent src, JComponent dst) {
   return true;
}

public final boolean shouldYieldFocus(JComponent source, JComponent 
target) {

return shouldYieldFocus(src) && verify(source, target);
  }

I see that we are talking about the same target design that may work 
equally well for source and for source+target validation and be 
transparent and clear for the user. And one constraint that we need to 
add to this set of equations is the backward compatibility with the 
current user code.

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Alexander Scherbatiy]

On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:

Hello All,

Please review the updated webrev.
Bug : https://bugs.openjdk.java.net/browse/JDK-8147521

Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/

Update: Added a new protected method getpop in PopupFactory.java with a Boolean 
parameter for specifying heavy weight popup.


   The provided method looks good to me.

   I would leave the javadoc review to the native speakers. What I only 
see that there is a missed space in {@codey}, typo in "paramaters" word 
and probably there should be a comma before "otherwise false".


  Thanks,
  Alexandr.



Regards,
Rajeev Chamyal

-Original Message-
From: Alan Snyder [mailto:javali...@cbfiddle.com]
Sent: 11 May 2016 03:39
To: Alexandr Scherbatiy
Cc: swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

I use only heavy weight.



On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
 wrote:


Do you need to use medium-weight popups in your application or 
light/heavy-weight popups are enough?

Thanks,
Alexandr.

On 5/10/2016 10:23 PM, Alan Snyder wrote:

On May 10, 2016, at 5:58 AM, Sergey Bylokhov  wrote:

Hi, Alan.
Can you please take a look to the proposed solutions? Thanks.



Approach 2 matches what I currently do. The problem noted by Rajeev does not 
happen because my popup factory calls setPopupType() on each call to the public 
getPopup() before invoking the superclass method.

I think the original version of Approach 1 would work. My factory would 
override the public getPopup() method and pass true to the five parameter 
method.

I think the revised version of Approach 1 does not work for me because the new 
flag is only tested if the first attempt to create a popup fails.

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello All,

Please review the updated webrev.
Bug : https://bugs.openjdk.java.net/browse/JDK-8147521 

Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/ 

Update: Added a new protected method getpop in PopupFactory.java with a Boolean 
parameter for specifying heavy weight popup.

Regards,
Rajeev Chamyal

-Original Message-
From: Alan Snyder [mailto:javali...@cbfiddle.com] 
Sent: 11 May 2016 03:39
To: Alexandr Scherbatiy
Cc: swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

I use only heavy weight.


> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
>  wrote:
> 
> 
> Do you need to use medium-weight popups in your application or 
> light/heavy-weight popups are enough?
> 
> Thanks,
> Alexandr.
> 
> On 5/10/2016 10:23 PM, Alan Snyder wrote:
>>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov  
>>> wrote:
>>> 
>>> Hi, Alan.
>>> Can you please take a look to the proposed solutions? Thanks.
>>> 
>>> 
>> Approach 2 matches what I currently do. The problem noted by Rajeev does not 
>> happen because my popup factory calls setPopupType() on each call to the 
>> public getPopup() before invoking the superclass method.
>> 
>> I think the original version of Approach 1 would work. My factory would 
>> override the public getPopup() method and pass true to the five parameter 
>> method.
>> 
>> I think the revised version of Approach 1 does not work for me because the 
>> new flag is only tested if the first attempt to create a popup fails.
>> 
> 

Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

[Rajeev Chamyal]

Hello Sergey,

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8152981/webrev.02/ 

  -  The empty catch block "catch (Exception e) {}, it will be better to 
re-throw an exception.
Updated the catch code to throw exception.

  - "frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE)" can cause a test 
failure in some jtreg modes(see JDK-8154365)
Removed 

  - Is it necessary to make this test "win only", can it cover other look and 
feels and platforms?
This issue is with Windows look and feel only. For other LAF and platforms it 
works fine.

Regards,
Rajeev Chamyal


-Original Message-
From: Sergey Bylokhov 
Sent: 05 May 2016 18:54
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8152981 Double icons with 
JMenuItem setHorizontalTextPosition on Win 10

The code change looks fine, a few notes about the test:
  -  The empty catch block "catch (Exception e) {}, it will be better to 
re-throw an exception.
  - "frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE)" can cause a test 
failure in some jtreg modes(see JDK-8154365)
  - Is it necessary to make this test "win only", can it cover other look and 
feels and platforms?

On 05.05.16 15:36, Rajeev Chamyal wrote:
> Hello Sergey,
>
> Please review the updated webrev.
> http://cr.openjdk.java.net/~rchamyal/8152981/webrev.01/
>
> Update: Added the check Icon install code in installDefaults to a private 
> method. Calling same method on propertychange.
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Sergey Bylokhov
> Sent: 04 May 2016 19:47
> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re:  [9] Review request for JDK-8152981 Double 
> icons with JMenuItem setHorizontalTextPosition on Win 10
>
> Hi, Rajeev.
> Is it necessary to reinstall UI for the component from the UI itself, 
> probably it will be possible to reconfigure the current one?
>
> On 03.05.16 15:08, Rajeev Chamyal wrote:
>> Hello All,
>>
>>
>>
>> Please review the below webrev.
>>
>>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152981
>>
>> Webrev : http://cr.openjdk.java.net/~rchamyal/8152981/webrev.00/
>>
>>
>>
>> Issue : Setting horizontal text position on a menuItem with icon 
>> results in Icon appearing twice on menuItem.
>>
>>
>>
>> Cause: When HorizontalTextPosition is set to LEFT/CENTER on MenuItem 
>> with an Icon, MenuItemUI defaults are not updated to set correct 
>> columnlayout .
>>
>> Which results in setting of check Icon on menuItem and 2 icons are 
>> shown on MenuItem.
>>
>>
>>
>> Fix : installing the UI defaults again if HorizontalTextPosition is 
>> set on MenuItem.
>>
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.