Re: RFR: 8272806: [macOS] "Apple AWT Internal Exception" when input method is changed

[Dmitry Markov]

On Sun, 22 Aug 2021 19:46:49 GMT, Phil Race  wrote:

> There's a long eval in the bug report : 
> https://bugs.openjdk.java.net/browse/JDK-8272806 but here's the summary
> 
> When focus is lost by the app a message is sent down to native setting a 
> native reference to the input method to null
> which is a sort of signal not to notify upwards anything to the input method 
> (which internally will NPE if there's
> no focused component as is the case after focus is lost).
> But we aren't actually setting it to null because of a check for null which 
> previously checked the wrapper for
> the JNI ref was not null but is now obsolete.  So just remove the check for 
> null.
> 
> The steps to reproduce this bug are very manual and involve installing 
> additional input methods and toggling them
> at the system level. So I didn't see a way to write a useful regression test 
> but if you follow the bug report steps you
> should be able to verify the fix.
> 
> I've run all automated tests just for good measure and they all pass.

Marked as reviewed by dmarkov (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5211

Re: RFR: 8264344: Outdated links in JavaComponentAccessibility.m

[Dmitry Markov]

On Mon, 29 Mar 2021 08:31:48 GMT, Alexander Zuev  wrote:

> 8264344: Outdated links in JavaComponentAccessibility.m

Marked as reviewed by dmarkov (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3240

Re: RFR: 8263490: [macos] Crash occurs on JPasswordField with activated InputMethod

[Dmitry Markov]

On Fri, 12 Mar 2021 08:16:09 GMT, Toshio Nakamura  wrote:

> Hi,
> Please review the fix for the issue of JPasswordField and activated 
> InputMethod on macOS.
> I don't think this condition is usual, but I'd like to avoid crash.
> 
> It needs two additional checks in "AWTView 
> attributedSubstringForProposedRange:actualRange".
> 
> Tested test/jdk/java/awt on macOS Catalina and BigSur (both headful), and no 
> regression was occurred.

Marked as reviewed by dmarkov (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/2959

Integrated: 8262446: DragAndDrop hangs on Windows

[Dmitry Markov]

On Thu, 4 Mar 2021 10:36:56 GMT, Dmitry Markov  wrote:

> The IME functions and the DND operation must be executed on the toolkit 
> thread. If the DND operation is in progress, the IME API is invoked via 
> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
> flag works properly if the DND is performed between two Java windows. However 
> if anything is dragged from native app, (e.g. Windows FileExplorer) to Java 
> the flag is NOT set. That’s the root cause of the hang.
> 
> Fix:
> Introduce a new flag to indicate DND operation between Java and native app. 
> 
> Testing:
> mach5 green

This pull request has now been integrated.

Changeset: bf9b74d1
Author:Dmitry Markov 
URL:   https://git.openjdk.java.net/jdk/commit/bf9b74d1
Stats: 8 lines in 1 file changed: 7 ins; 0 del; 1 mod

8262446: DragAndDrop hangs on Windows

Reviewed-by: aivanov, serb, kizune

-

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows

[Dmitry Markov]

On Thu, 4 Mar 2021 10:36:56 GMT, Dmitry Markov  wrote:

> The IME functions and the DND operation must be executed on the toolkit 
> thread. If the DND operation is in progress, the IME API is invoked via 
> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
> flag works properly if the DND is performed between two Java windows. However 
> if anything is dragged from native app, (e.g. Windows FileExplorer) to Java 
> the flag is NOT set. That’s the root cause of the hang.
> 
> Fix:
> Introduce a new flag to indicate DND operation between Java and native app. 
> 
> Testing:
> mach5 green

> @dmarkov20
> Sorry I'm not reviewer.
> It seems Copyright year is not changed. Is it OK ?

I have updated the copyright

-

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows [v4]

[Dmitry Markov]

> The IME functions and the DND operation must be executed on the toolkit 
> thread. If the DND operation is in progress, the IME API is invoked via 
> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
> flag works properly if the DND is performed between two Java windows. However 
> if anything is dragged from native app, (e.g. Windows FileExplorer) to Java 
> the flag is NOT set. That’s the root cause of the hang.
> 
> Fix:
> Introduce a new flag to indicate DND operation between Java and native app. 
> 
> Testing:
> mach5 green

Dmitry Markov has updated the pull request incrementally with one additional 
commit since the last revision:

  Update copyright

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2825/files
  - new: https://git.openjdk.java.net/jdk/pull/2825/files/dbc9dac7..3810f1b0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2825=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2825=02-03

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2825.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2825/head:pull/2825

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows [v3]

[Dmitry Markov]

> The IME functions and the DND operation must be executed on the toolkit 
> thread. If the DND operation is in progress, the IME API is invoked via 
> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
> flag works properly if the DND is performed between two Java windows. However 
> if anything is dragged from native app, (e.g. Windows FileExplorer) to Java 
> the flag is NOT set. That’s the root cause of the hang.
> 
> Fix:
> Introduce a new flag to indicate DND operation between Java and native app. 
> 
> Testing:
> mach5 green

Dmitry Markov has updated the pull request incrementally with one additional 
commit since the last revision:

  Remove flag setting from DragOver

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2825/files
  - new: https://git.openjdk.java.net/jdk/pull/2825/files/d9af9879..dbc9dac7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2825=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2825=01-02

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2825.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2825/head:pull/2825

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows [v2]

[Dmitry Markov]

On Fri, 5 Mar 2021 17:01:41 GMT, Alexey Ivanov  wrote:

>> Dmitry Markov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   reuse isInDoDragDropLoop
>
> src/java.desktop/windows/native/libawt/windows/awt_DnDDT.cpp line 228:
> 
>> 226: HRESULT __stdcall AwtDropTarget::DragOver(DWORD grfKeyState, POINTL pt, 
>> DWORD __RPC_FAR *pdwEffect) {
>> 227: TRY;
>> 228: AwtToolkit::GetInstance().isInDoDragDropLoop = TRUE;
> 
> This is a new addition. Did you miss this function in previous iteration?

Good catch! Actually that line is harmless but it is really unnecessary. I will 
update PR

-

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows [v2]

[Dmitry Markov]

On Fri, 5 Mar 2021 12:53:00 GMT, Dmitry Markov  wrote:

>>> Why we cannot reuse the old flag? "isInDoDragDropLoop"? I think the 
>>> Robot.waitForIdle() will hang if isInDoDragDropLoop is not set to true 
>>> while dragging something from the native app.
>> 
>> Initially I didn’t want to touch that flag but after a closer look to 
>> “isInDoDragDropLoop” I think it is OK to reuse the flag. I will update PR 
>> shortly.
>
>> @dmarkov20
>> I'd like to confirm this issue was not fixed by 
>> [JDK-8261231](https://bugs.openjdk.java.net/browse/JDK-8261231) #2448 ?
> 
> That's right. This one and JDK-8261231 are two different issues. It is 
> expected that the changes for JDK-8261231 do not fix this.

I have update the fix: reuse isInDoDragDropLoop flag instead of introducing a 
new one.
@mrserb @aivanov-jdk 
Could you take a look, please?

-

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows [v2]

[Dmitry Markov]

On Fri, 5 Mar 2021 12:51:15 GMT, Dmitry Markov  wrote:

>> Why we cannot reuse the old flag? "isInDoDragDropLoop"? I think the 
>> Robot.waitForIdle() will hang if isInDoDragDropLoop is not set to true while 
>> dragging something from the native app.
>
>> Why we cannot reuse the old flag? "isInDoDragDropLoop"? I think the 
>> Robot.waitForIdle() will hang if isInDoDragDropLoop is not set to true while 
>> dragging something from the native app.
> 
> Initially I didn’t want to touch that flag but after a closer look to 
> “isInDoDragDropLoop” I think it is OK to reuse the flag. I will update PR 
> shortly.

> @dmarkov20
> I'd like to confirm this issue was not fixed by 
> [JDK-8261231](https://bugs.openjdk.java.net/browse/JDK-8261231) #2448 ?

That's right. This one and JDK-8261231 are two different issues. It is expected 
that the changes for JDK-8261231 do not fix this.

-

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows [v2]

[Dmitry Markov]

> The IME functions and the DND operation must be executed on the toolkit 
> thread. If the DND operation is in progress, the IME API is invoked via 
> SendMessage() call inside InvokeInputMethodFunction() to avoid a hang. The 
> flag isInDoDragDropLoop indicates whether the DND takes place or not. The 
> flag works properly if the DND is performed between two Java windows. However 
> if anything is dragged from native app, (e.g. Windows FileExplorer) to Java 
> the flag is NOT set. That’s the root cause of the hang.
> 
> Fix:
> Introduce a new flag to indicate DND operation between Java and native app. 
> 
> Testing:
> mach5 green

Dmitry Markov has updated the pull request incrementally with one additional 
commit since the last revision:

  reuse isInDoDragDropLoop

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2825/files
  - new: https://git.openjdk.java.net/jdk/pull/2825/files/6d7502a4..d9af9879

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2825=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2825=00-01

  Stats: 11 lines in 3 files changed: 1 ins; 2 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2825.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2825/head:pull/2825

PR: https://git.openjdk.java.net/jdk/pull/2825

Re: RFR: 8262446: DragAndDrop hangs on Windows [v2]

[Dmitry Markov]

On Fri, 5 Mar 2021 01:34:04 GMT, Sergey Bylokhov  wrote:

> Why we cannot reuse the old flag? "isInDoDragDropLoop"? I think the 
> Robot.waitForIdle() will hang if isInDoDragDropLoop is not set to true while 
> dragging something from the native app.

Initially I didn’t want to touch that flag but after a closer look to 
“isInDoDragDropLoop” I think it is OK to reuse the flag. I will update PR 
shortly.

-

PR: https://git.openjdk.java.net/jdk/pull/2825

RFR: 8262446: DragAndDrop hangs on Windows

[Dmitry Markov]

The IME functions and the DND operation must be executed on the toolkit thread. 
If the DND operation is in progress, the IME API is invoked via SendMessage() 
call inside InvokeInputMethodFunction() to avoid a hang. The flag 
isInDoDragDropLoop indicates whether the DND takes place or not. The flag works 
properly if the DND is performed between two Java windows. However if anything 
is dragged from native app, (e.g. Windows FileExplorer) to Java the flag is NOT 
set. That’s the root cause of the hang.

Fix:
Introduce a new flag to indicate DND operation between Java and native app. 

Testing:
mach5 green

-

Commit messages:
 - 8262446: DragAndDrop hangs on Windows

Changes: https://git.openjdk.java.net/jdk/pull/2825/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2825=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8262446
  Stats: 10 lines in 3 files changed: 9 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2825.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2825/head:pull/2825

PR: https://git.openjdk.java.net/jdk/pull/2825

Integrated: 8261231: Windows IME was disabled after DnD operation

[Dmitry Markov]

On Sun, 7 Feb 2021 08:29:57 GMT, Dmitry Markov  wrote:

> The function InvokeInputMethodFunction() is responsible for invocation of IME 
> API. Typically it uses PostMessage() to execute corresponding IME function on 
> the toolkit thread but if DnD operation takes place SendMessage() is used. 
> The state of m_inputMethodWaitEvent event object remains signalled after 
> SendMessage() execution. That causes failure of subsequent IME functions 
> calls via PostMessage().
> 
> Fix:
> SendMessage() and PostMessage() calls inside InvokeInputMethodFunction() 
> should be synchronised. The state of m_inputMethodWaitEvent event object must 
> be reseted right after SendMessage() execution.

This pull request has now been integrated.

Changeset: d6d5d9bf
Author:Dmitry Markov 
URL:   https://git.openjdk.java.net/jdk/commit/d6d5d9bf
Stats: 5 lines in 1 file changed: 3 ins; 1 del; 1 mod

8261231: Windows IME was disabled after DnD operation

Reviewed-by: kizune, serb

-

PR: https://git.openjdk.java.net/jdk/pull/2448

Re: RFR: 8261231: Windows IME was disabled after DnD operation

[Dmitry Markov]

On Mon, 8 Feb 2021 16:51:21 GMT, Alexander Zuev  wrote:

> Change looks good and i haven't found any side-effects during testing. Could 
> you please add the label to the bug noting reason for absence of the 
> regression test, like noreg-hard or something?

Thank you for the review. I have added noreg-hard to the bug

-

PR: https://git.openjdk.java.net/jdk/pull/2448

RFR: 8261231: Windows IME was disabled after DnD operation

[Dmitry Markov]

The function InvokeInputMethodFunction() is responsible for invocation of IME 
API. Typically it uses PostMessage() to execute corresponding IME function on 
the toolkit thread but if DnD operation takes place SendMessage() is used. The 
state of m_inputMethodWaitEvent event object remains signalled after 
SendMessage() execution. That causes failure of subsequent IME functions calls 
via PostMessage().

Fix:
SendMessage() and PostMessage() calls inside InvokeInputMethodFunction() should 
be synchronised. The state of m_inputMethodWaitEvent event object must be 
reseted right after SendMessage() execution.

-

Commit messages:
 - 8261231: Windows IME was disabled after DnD operation

Changes: https://git.openjdk.java.net/jdk/pull/2448/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2448=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261231
  Stats: 5 lines in 1 file changed: 3 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2448.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2448/head:pull/2448

PR: https://git.openjdk.java.net/jdk/pull/2448

Re: RFR: 8258805: Japanese characters not entered by mouse click on Windows 10 [v2]

[Dmitry Markov]

On Thu, 21 Jan 2021 21:34:25 GMT, Alexey Ivanov  wrote:

>> Dmitry Markov has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Use endComposition() instead of endCompositionNative()
>
> Marked as reviewed by aivanov (Reviewer).

> Hi,
> 
> AWT's `TextComponent` is a `peered` input client, and Swing's 
> `JTextComponent` is an `active` input client. Thus it is OK to behave 
> differently. I would expect that AWT's one behaves as the same as native 
> Windows apps, and Swing's one should commit text into the component that has 
> lost the focus.

Thank you for confirmation! With the fix we have the same behaviour as you 
described.

-

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: RFR: 8258805: Japanese characters not entered by mouse click on Windows 10

[Dmitry Markov]

On Thu, 21 Jan 2021 19:25:16 GMT, Alexey Ivanov  wrote:

> I admit I am even more confused now. To me, the description in the comment 
> above is nearly the same as in [JBS 
> comment](https://bugs.openjdk.java.net/browse/JDK-8258805?focusedCommentId=14391025=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14391025).
>  Is the difference that the original test case disabled IME for the middle 
> JTextField whereas in the test case above all JTextField support IME?

Well.. I think the main difference between tests is that the [test attached to 
the 
bug](https://bugs.openjdk.java.net/browse/JDK-8258805?focusedCommentId=14391025=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14391025)
 uses `JTextField` (Swing) and the [test provided 
above](https://github.com/openjdk/jdk/pull/2142#issuecomment-763491615) uses 
`TextField` (AWT). The same input method events are processed differently for 
Swing and AWT text components. Good example is the following test:

import java.awt.*;
import java.awt.event.*;

public class AWTTextTest1 extends Frame {
AWTTextTest1() {
setTitle("AWTTextTest1");
setLayout(new GridLayout(0, 1));
add(new TextField());
add(new TextField());
addWindowListener(new WindowAdapter() {
public void windowClosing(WindowEvent we) {
System.exit(0);
}
});
setSize(400, 300);
setVisible(true);
}
public static void main(String[] args) {
new AWTTextTest1();
}
}

1. Run test (originally it uses `TextField`)
2. Click upper `TextField`, turn on IME, type some character (In case of 
Japanese, type "aiu") 
3. Click lower `TextField`, the string is canceled. 
4. Replace `TextField` with `JTextField` in the test. Compile and run it again.
5. Click upper `JTextField`, turn on IME, type some character (In case of 
Japanese, type "aiu")
6. Click lower `JTextField`, the string is committed before focus transition.

> 
> In the updated version of the fix, we always commit the text on any focus 
> change whether the newly focused component supports IME or not.

That’s not quite right. Actually we commit the text if the current IM client is 
“active client”. For example, `JTextField` is an “active client” but 
`TextField`  - NOT. The status “active client” depends on the implementation of 
getInputMethodRequests() method.

-

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: RFR: 8258805: Japanese characters not entered by mouse click on Windows 10

[Dmitry Markov]

On Thu, 21 Jan 2021 14:18:43 GMT, Alexey Ivanov  wrote:

> > Now we will commit the composition string if there is an active client. 
> > That changes eliminates the issue described in JDK-8258805. Also the 
> > behaviour of AWTTextTest1 is the same as it used to be on the previous 
> > versions w/o the fix.
> 
> Just to confirm: the updated behaviour is similar to what was happening in 
> previous versions of Windows 10, that is if a compositing text isn't 
> committed, it remains uncommitted in the text component when the focus 
> changes instead of being committed as it was the case in the previous 
> iteration. Is my understanding correct?

I am sorry but you didn't get it right. The behaviour, which was introduced in 
previous iteration, is still in place. It is required to get rid of the problem 
described in  JDK-8258805
The purpose of of the second iteration is to eliminate negative side effect 
(more details here 
https://github.com/openjdk/jdk/pull/2142#issuecomment-763340186 ) introduced by 
the first version of the fix.

-

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: RFR: 8258805: Japanese characters not entered by mouse click on Windows 10

[Dmitry Markov]

On Wed, 20 Jan 2021 12:28:36 GMT, Alexey Ivanov  wrote:

>>> > Fix:
>>> > It is necessary to take care of unconfirmed composition string once the 
>>> > IME is going to be disabled.
>>> 
>>> The fix commits the unconfirmed composition string. Committing is better 
>>> than discarding. Is it possible to preserve the 
>> state and to leave the string uncommitted?
>> 
>> The fix reverts the previous (correct) behaviour back. It is unnecessary to 
>> store the state and keep the string. That activity may be quite complicated 
>> and requires additional resources especially if there are several components 
>> with active IME
>
>> > The fix commits the unconfirmed composition string. Committing is better 
>> > than discarding. Is it possible to preserve the
>> > state and to leave the string uncommitted?
>> 
>> The fix reverts the previous (correct) behaviour back.
> 
> According to the bug description, it used to stay in uncommitted state.
> 
>> It is unnecessary to store the state and keep the string. That activity may 
>> be quite complicated and requires additional resources especially if there 
>> are several components with active IME
> 
> I definitely agree that committing the text is better behaviour than 
> discarding uncommitted text.
> 
> I am for accepting these changes unless we can keep the text uncommitted and 
> allow the user to work with text when the focus moves back to the text 
> component.

An interesting thing is that the test (see AWTTextTest1.java) behaves like a 
native application if TextField is replaced with JTextField. It appears AWT and 
Swing components behave differently while processing input method requests. 
That’s a great topic to discuss but it is out of scope for this PR. We can open 
a separate bug for it, if any.

Going back to the current PR. I updated the fix. Now we will commit the 
composition string if there is an active client. That changes eliminates the 
issue described in JDK-8258805. Also the behaviour of AWTTextTest1 is the same 
as it used to be on the previous versions w/o the fix.

-

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: RFR: 8258805: Japanese characters not entered by mouse click on Windows 10 [v2]

[Dmitry Markov]

> Problem description:
> The IME behaviour has changed starting from recent Windows 10 builds. In 
> particular if the complex characters (Japanese, Chinese, etc.) are entered to 
> some component and the focus is transferred to another component (which does 
> not support the IM) the IM is disabled and the unconfirmed composition string 
> gets discarded.
> On previous Windows versions in the same situation the composition string is 
> not discarded.
> 
> Fix:
> It is necessary to take care of unconfirmed composition string once the IME 
> is going to be disabled.
> 
> Testing:
> All our automated regression and JCK tests passed with the proposed change.

Dmitry Markov has updated the pull request incrementally with one additional 
commit since the last revision:

  Use endComposition() instead of endCompositionNative()

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2142/files
  - new: https://git.openjdk.java.net/jdk/pull/2142/files/1f9189bf..3d486e20

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2142=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2142=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2142.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2142/head:pull/2142

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: RFR: 8258805: Japanese characters not entered by mouse click on Windows 10

[Dmitry Markov]

On Wed, 20 Jan 2021 07:35:18 GMT, Ichiroh Takiguchi  
wrote:

>> How do the native components work in that case like awt textarea or external 
>> apps like notepad?
>
>> How do the native components work in that case like awt textarea or external 
>> apps like notepad?
> 
> I tested TextField+TextField, TextArea+TextArea, TextArea+TextField.
> Without fix: Preedit string was cancel by input focus change.
> With fix: Preedit string was committed into clicked component.
> 
> For external application, I used Sakura Editor, because it can create 2 
> editor panes on one window.
> Preedit string was committed before moving input focus.

> I tried attached testcase.
> 
> ```java
> import java.awt.*;
> import java.awt.event.*;
> 
> public class AWTTextTest1 extends Frame {
> AWTTextTest1() {
> setTitle("AWTTextTest1");
> setLayout(new GridLayout(0, 1));
> add(new TextField());
> add(new TextField());
> addWindowListener(new WindowAdapter() {
> public void windowClosing(WindowEvent we) {
> System.exit(0);
> }
> });
> setSize(400, 300);
> setVisible(true);
> }
> public static void main(String[] args) {
> new AWTTextTest1();
> }
> }
> ```
> 
> 1. Start testcase
> 2. Click upper TextField, turn on IME, type some character (In case of 
> Japanese, type "aiu")
> 3. Click lower TextField, preedit string is committed into lower TextField.
> 
> Without the fix, preedit string was canceled by above operation.

Hello Ichiroh,

Thank you for the details. I can reproduce it. I will look into this..

Regards,
Dmitry

-

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: RFR: 8258805: Japanese characters not entered by mouse click on Windows 10

[Dmitry Markov]

On Tue, 19 Jan 2021 12:44:22 GMT, Alexey Ivanov  wrote:

> > Fix:
> > It is necessary to take care of unconfirmed composition string once the IME 
> > is going to be disabled.
> 
> The fix commits the unconfirmed composition string. Committing is better than 
> discarding. Is it possible to preserve the 
state and to leave the string uncommitted?

The fix reverts the previous (correct) behaviour back. It is unnecessary to 
store the state and keep the string. That activity may be quite complicated and 
requires additional resources especially if there are several components with 
active IME

-

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: [EXTERNAL] RFR: 8258805: Japanese characters not entered by mouse click on Windows 10

[Dmitry Markov]

Hi Ichiron,

I have updated the bug. Now the test, steps to reproduce and other details are 
available. Please let me know if you need anything else.

Regards,
Dmitry

> On 19 Jan 2021, at 11:44, Ichiroh Takiguchi  
> wrote:
> 
> Hello Dmitry.
> 
> The bugid seems to be private, so I don't know the details.
> I think the current code can change the candidate string after getting the 
> focus.
> If possible, could you show me the test instructions ?
> 
> Thanks,
> Ichiroh Takiguchi
> 
> On 2021-01-19 20:16, Dmitry Markov wrote:
>> Problem description:
>> The IME behaviour has changed starting from recent Windows 10 builds.
>> In particular if the complex characters (Japanese, Chinese, etc.) are
>> entered to some component and the focus is transferred to another
>> component (which does not support the IM) the IM is disabled and the
>> unconfirmed composition string gets discarded.
>> On previous Windows versions in the same situation the composition
>> string is not discarded.
>> Fix:
>> It is necessary to take care of unconfirmed composition string once
>> the IME is going to be disabled.
>> Testing:
>> All our automated regression and JCK tests passed with the proposed change.
>> -
>> Commit messages:
>> - 8258805: Japanese characters not entered by mouse click on Windows 10
>> Changes:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.openjdk.java.net_jdk_pull_2142_files=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=TWSRKjAEFvjdtcldX_CzGUCRvo92UQrQaevJPjdE9G0=pSPWtUQC712ERQcCWzl97FRkUYd8Rgu1S4ejmCvqfho=tQjpctdFBD-VNGT6Sz99HLO87e4Il0g5UPyncfk05xQ=
>> Webrev:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__webrevs.openjdk.java.net_-3Frepo-3Djdk-26pr-3D2142-26range-3D00=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=TWSRKjAEFvjdtcldX_CzGUCRvo92UQrQaevJPjdE9G0=pSPWtUQC712ERQcCWzl97FRkUYd8Rgu1S4ejmCvqfho=uS7Zv56IELOfi9JydPxkC8cn11F2sm2rQIzygpVRKGU=
>>  Issue:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8258805=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=TWSRKjAEFvjdtcldX_CzGUCRvo92UQrQaevJPjdE9G0=pSPWtUQC712ERQcCWzl97FRkUYd8Rgu1S4ejmCvqfho=2ceq871Y2SbUmo-W1IRN4YDFPxeXMq1Y1GSY4YgkyNY=
>>  Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod
>>  Patch:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.openjdk.java.net_jdk_pull_2142.diff=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=TWSRKjAEFvjdtcldX_CzGUCRvo92UQrQaevJPjdE9G0=pSPWtUQC712ERQcCWzl97FRkUYd8Rgu1S4ejmCvqfho=V5Hl37kt-btSEbTU2y1pD1-_e-z1N_QDDSl2tRIc6HI=
>>  Fetch: git fetch
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.openjdk.java.net_jdk=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=TWSRKjAEFvjdtcldX_CzGUCRvo92UQrQaevJPjdE9G0=pSPWtUQC712ERQcCWzl97FRkUYd8Rgu1S4ejmCvqfho=v3cOrMgmRWxELPv_w6aY_bwvvM6_qqYJY5yaIW6tADY=
>> pull/2142/head:pull/2142
>> PR:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.openjdk.java.net_jdk_pull_2142=DwIBaQ=jf_iaSHvJObTbx-siA1ZOg=TWSRKjAEFvjdtcldX_CzGUCRvo92UQrQaevJPjdE9G0=pSPWtUQC712ERQcCWzl97FRkUYd8Rgu1S4ejmCvqfho=XG8QFzRvrJv3WP3HW32IfaG7xG8WqxAlQkk2Fp3u708=

RFR: 8258805: Japanese characters not entered by mouse click on Windows 10

[Dmitry Markov]

Problem description:
The IME behaviour has changed starting from recent Windows 10 builds. In 
particular if the complex characters (Japanese, Chinese, etc.) are entered to 
some component and the focus is transferred to another component (which does 
not support the IM) the IM is disabled and the unconfirmed composition string 
gets discarded.
On previous Windows versions in the same situation the composition string is 
not discarded.

Fix:
It is necessary to take care of unconfirmed composition string once the IME is 
going to be disabled.

Testing:
All our automated regression and JCK tests passed with the proposed change.

-

Commit messages:
 - 8258805: Japanese characters not entered by mouse click on Windows 10

Changes: https://git.openjdk.java.net/jdk/pull/2142/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2142=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8258805
  Stats: 6 lines in 1 file changed: 5 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2142.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2142/head:pull/2142

PR: https://git.openjdk.java.net/jdk/pull/2142

Re: [16] RFR 8252470: java/awt/dnd/DisposeFrameOnDragCrash/DisposeFrameOnDragTest.java fails on Windows

[Dmitry Markov]

Hi Phil,

I ran all client test using mach5. All related test passed, see 
https://mach5.us.oracle.com/mdash/jobs?search=id:dmarkov-jdk-client-20200828-1219-13803609
 
<https://mach5.us.oracle.com/mdash/jobs?search=id:dmarkov-jdk-client-20200828-1219-13803609>

Regards,
Dmitry

> On 28 Aug 2020, at 21:00, Philip Race  wrote:
> 
> Looks Ok to me but please confirm that all related tests pass.
> 
> -phil. 
> 
> On 8/28/20, 12:54 PM, Dmitry Markov wrote:
>> 
>> Hello,
>> 
>> Could you review the fix for JDK 16, please?
>> 
>>  bug: https://bugs.openjdk.java.net/browse/JDK-8252470 
>> <https://bugs.openjdk.java.net/browse/JDK-8252470>
>>  webrev: http://cr.openjdk.java.net/~dmarkov/8252470/webrev.00/ 
>> <http://cr.openjdk.java.net/%7Edmarkov/8252470/webrev.00/>
>> 
>> Problem description:
>> After integration of JDK-8232114 if an IME API is invoked from a DND call a 
>> hang may take place. 
>> The DND operation runs on a toolkit thread. The IME functions should be 
>> executed on the toolkit thread, as well. So if, for example, the IME 
>> function is called from one of the methods of DropTargetListener the 
>> corresponding IME message will be blocked in the toolkit thread message 
>> queue because because the toolkit thread is busy executing DND operation.
>> 
>> Fix: 
>> If DND operation is active then we shouldn’t post the IME messages to the 
>> toolkit thread queue and use SendMessage() function instead.
>> 
>> Regards,
>> Dmitry

[16] RFR 8252470: java/awt/dnd/DisposeFrameOnDragCrash/DisposeFrameOnDragTest.java fails on Windows

[Dmitry Markov]

Hello,

Could you review the fix for JDK 16, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8252470 

webrev: http://cr.openjdk.java.net/~dmarkov/8252470/webrev.00/ 


Problem description:
After integration of JDK-8232114 if an IME API is invoked from a DND call a 
hang may take place. 
The DND operation runs on a toolkit thread. The IME functions should be 
executed on the toolkit thread, as well. So if, for example, the IME function 
is called from one of the methods of DropTargetListener the corresponding IME 
message will be blocked in the toolkit thread message queue because because the 
toolkit thread is busy executing DND operation.

Fix: 
If DND operation is active then we shouldn’t post the IME messages to the 
toolkit thread queue and use SendMessage() function instead.

Regards,
Dmitry

Re: [16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Dmitry Markov]

Thank you, Anton!

Sergey,
Any remarks regarding the updated fix?
http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/ 
<http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/>

Thanks in advance,
Dmitry

> On 25 Aug 2020, at 00:35, Anton Litvinov  wrote:
> 
> Hi Dmitry,
> 
> The fix looks good. Thank you for taking into account my suggestions.
> 
> Thank you,
> Anton
> 
> On 24/08/2020 10:10, Dmitry Markov wrote:
>> Hi Anton,
>> 
>> Thank you for review. I have updated the fix based on your suggestions: 
>>  - Modified the function InvokeInputMethodFunction() to take into account 
>> the result of PostMessage() call
>>  - Replaced InvokeInputMethodFunction() with SendMessage() for 
>> WM_AWT_HANDLE_NATIVE_IME_EVENT and WM_AWT_ACTIVATEKEYBOARDLAYOUT. I didn’t 
>> update the code related to WM_AWT_OPENCANDIDATEWINDOW message because we can 
>> call IME API during its processing, (e.g. AwtToolkit::WndProc() -> 
>> AwtComponent::OpenCandidateWindow() -> AwtComponent::SetCandidateWindow() -> 
>> Imm*).
>> 
>> The new web rev is located at 
>> http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/ 
>> <http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/>
>> 
>> Regards,
>> Dmitry
>> 
>>> On 20 Aug 2020, at 15:14, Anton Litvinov >> <mailto:anton.litvi...@oracle.com>> wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> The fix looks well, but I have next a few comments relatively to it:
>>> 
>>> 1) In the new function "AwtToolkit::InvokeInputMethodFunction(UINT, WPARAM, 
>>> LPARAM)", which you created, the call "PostMessage(msg, wParam, lParam);" 
>>> may fail and return "0", so in that case the next call 
>>> "::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE);" may become 
>>> blocked forever. I suggest to handle this possible failure, for example in 
>>> the next way:
>>> 
>>> if (PostMessage(msg, wParam, lParam)) {
>>> ::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE);
>>> return m_inputMethodData;
>>> }
>>> return 0;
>>> 
>>> 2) In "AwtToolkit::WndProc" function in the code handling the messages: 
>>> "WM_AWT_HANDLE_NATIVE_IME_EVENT", "WM_AWT_ACTIVATEKEYBOARDLAYOUT", 
>>> "WM_AWT_OPENCANDIDATEWINDOW" I do not find calls to "Imm*" system 
>>> functions. Therefore maybe there is no need to send these messages through 
>>> "PostMessage"?
>>> 
>>> Thank you,
>>> Anton
>>> 
>>> On 18/08/2020 06:46, Dmitry Markov wrote:
>>>> Thank you, Sergey!
>>>> 
>>>> Looking for one more "+1”. Any volunteers?
>>>> 
>>>> Regards,
>>>> Dmitry
>>>>> On 17 Aug 2020, at 21:06, Sergey Bylokhov >>>> <mailto:sergey.bylok...@oracle.com>> wrote:
>>>>> 
>>>>> Looks fine.
>>>>> 
>>>>> On 17.08.2020 02:32, Dmitry Markov wrote:
>>>>>> Hi Sergey,
>>>>>> I have added that information to InvokeInputMethodFunction(). Please 
>>>>>> find the new webrev here: 
>>>>>> http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/ 
>>>>>> <http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/>
>>>>>> Regards,
>>>>>> Dmitry
>>>>>>> On 15 Aug 2020, at 03:05, Sergey Bylokhov >>>>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>>>>>> 
>>>>>>> On 12.08.2020 05:09, Dmitry Markov wrote:
>>>>>>>> TranslateMessage() does not invoke PeekMessage(). In our case 
>>>>>>>> TranslateMessage() is called by AWT. IME functionality may call 
>>>>>>>> PeekMessage() during TranslateMessage() execution. However that 
>>>>>>>> PeekMessage() call is intended for processing non-queued  messages, 
>>>>>>>> (i.e. the messages send via SendMessage() call).
>>>>>>>> I contacted Microsoft regarding this problem and one of their 
>>>>>>>> suggestions was to use PostMessage() instead of SendMessage() for IME 
>>>>>>>> messages to avoid IME internal data corruption and the crash.
>>>>>>>> The proposed fix was tested by the stress test 

Re: [16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Dmitry Markov]

Hi Anton,

Thank you for review. I have updated the fix based on your suggestions: 
 - Modified the function InvokeInputMethodFunction() to take into account the 
result of PostMessage() call
 - Replaced InvokeInputMethodFunction() with SendMessage() for 
WM_AWT_HANDLE_NATIVE_IME_EVENT and WM_AWT_ACTIVATEKEYBOARDLAYOUT. I didn’t 
update the code related to WM_AWT_OPENCANDIDATEWINDOW message because we can 
call IME API during its processing, (e.g. AwtToolkit::WndProc() -> 
AwtComponent::OpenCandidateWindow() -> AwtComponent::SetCandidateWindow() -> 
Imm*).

The new web rev is located at 
http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/ 
<http://cr.openjdk.java.net/~dmarkov/8232114/webrev.02/>

Regards,
Dmitry

> On 20 Aug 2020, at 15:14, Anton Litvinov  wrote:
> 
> Hi Dmitry,
> 
> The fix looks well, but I have next a few comments relatively to it:
> 
> 1) In the new function "AwtToolkit::InvokeInputMethodFunction(UINT, WPARAM, 
> LPARAM)", which you created, the call "PostMessage(msg, wParam, lParam);" may 
> fail and return "0", so in that case the next call 
> "::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE);" may become blocked 
> forever. I suggest to handle this possible failure, for example in the next 
> way:
> 
> if (PostMessage(msg, wParam, lParam)) {
> ::WaitForSingleObject(m_inputMethodWaitEvent, INFINITE);
> return m_inputMethodData;
> }
> return 0;
> 
> 2) In "AwtToolkit::WndProc" function in the code handling the messages: 
> "WM_AWT_HANDLE_NATIVE_IME_EVENT", "WM_AWT_ACTIVATEKEYBOARDLAYOUT", 
> "WM_AWT_OPENCANDIDATEWINDOW" I do not find calls to "Imm*" system functions. 
> Therefore maybe there is no need to send these messages through "PostMessage"?
> 
> Thank you,
> Anton
> 
> On 18/08/2020 06:46, Dmitry Markov wrote:
>> Thank you, Sergey!
>> 
>> Looking for one more "+1”. Any volunteers?
>> 
>> Regards,
>> Dmitry
>>> On 17 Aug 2020, at 21:06, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com>> wrote:
>>> 
>>> Looks fine.
>>> 
>>> On 17.08.2020 02:32, Dmitry Markov wrote:
>>>> Hi Sergey,
>>>> I have added that information to InvokeInputMethodFunction(). Please find 
>>>> the new webrev here: 
>>>> http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/ 
>>>> <http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/>
>>>> Regards,
>>>> Dmitry
>>>>> On 15 Aug 2020, at 03:05, Sergey Bylokhov >>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>>>> 
>>>>> On 12.08.2020 05:09, Dmitry Markov wrote:
>>>>>> TranslateMessage() does not invoke PeekMessage(). In our case 
>>>>>> TranslateMessage() is called by AWT. IME functionality may call 
>>>>>> PeekMessage() during TranslateMessage() execution. However that 
>>>>>> PeekMessage() call is intended for processing non-queued  messages, 
>>>>>> (i.e. the messages send via SendMessage() call).
>>>>>> I contacted Microsoft regarding this problem and one of their 
>>>>>> suggestions was to use PostMessage() instead of SendMessage() for IME 
>>>>>> messages to avoid IME internal data corruption and the crash.
>>>>>> The proposed fix was tested by the stress test for several weeks and no 
>>>>>> issues were observed. So I feel quite confident that it eliminates the 
>>>>>> issue.
>>>>>> There is no exact message which triggers the crash. Usually the crash is 
>>>>>> caused by one of the following messages: WM_AWT_ASSOCIATECONTEXT or 
>>>>>> WM_AWT_SETOPENSTATUS but several times I observed that it was triggered 
>>>>>> by WM_AWT_DESTROYCONTEXT or WM_AWT_CREATECONTEXT. It looks like almost 
>>>>>> every IME-related message may cause the crash.  I think SendMessage() 
>>>>>> call should be substituted by PostMessage() for all IME messages.
>>>>> 
>>>>> Ok, then please add this(or similar) information to the new method 
>>>>> "InvokeInputMethodFunction",
>>>>> otherwise it could be removed in the future/replaced back to the 
>>>>> sendMessage.
>>>>> 
>>>>>> Regards,
>>>>>> Dmitry
>>>>>>> On 12 Aug 2020, 

Re: [16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Dmitry Markov]

Thank you, Sergey!

Looking for one more "+1”. Any volunteers?

Regards,
Dmitry
> On 17 Aug 2020, at 21:06, Sergey Bylokhov  wrote:
> 
> Looks fine.
> 
> On 17.08.2020 02:32, Dmitry Markov wrote:
>> Hi Sergey,
>> I have added that information to InvokeInputMethodFunction(). Please find 
>> the new webrev here: http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/ 
>> <http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/>
>> Regards,
>> Dmitry
>>> On 15 Aug 2020, at 03:05, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>> 
>>> On 12.08.2020 05:09, Dmitry Markov wrote:
>>>> TranslateMessage() does not invoke PeekMessage(). In our case 
>>>> TranslateMessage() is called by AWT. IME functionality may call 
>>>> PeekMessage() during TranslateMessage() execution. However that 
>>>> PeekMessage() call is intended for processing non-queued  messages, (i.e. 
>>>> the messages send via SendMessage() call).
>>>> I contacted Microsoft regarding this problem and one of their suggestions 
>>>> was to use PostMessage() instead of SendMessage() for IME messages to 
>>>> avoid IME internal data corruption and the crash.
>>>> The proposed fix was tested by the stress test for several weeks and no 
>>>> issues were observed. So I feel quite confident that it eliminates the 
>>>> issue.
>>>> There is no exact message which triggers the crash. Usually the crash is 
>>>> caused by one of the following messages: WM_AWT_ASSOCIATECONTEXT or 
>>>> WM_AWT_SETOPENSTATUS but several times I observed that it was triggered by 
>>>> WM_AWT_DESTROYCONTEXT or WM_AWT_CREATECONTEXT. It looks like almost every 
>>>> IME-related message may cause the crash.  I think SendMessage() call 
>>>> should be substituted by PostMessage() for all IME messages.
>>> 
>>> Ok, then please add this(or similar) information to the new method 
>>> "InvokeInputMethodFunction",
>>> otherwise it could be removed in the future/replaced back to the 
>>> sendMessage.
>>> 
>>>> Regards,
>>>> Dmitry
>>>>> On 12 Aug 2020, at 06:16, Sergey Bylokhov >>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>>>> 
>>>>> Hi, Dmitry.
>>>>> 
>>>>> On 11.08.2020 01:07, Dmitry Markov wrote:
>>>>>> Problem description:
>>>>>> The root cause of the crash is the lack of synchronisation in 
>>>>>> imjpapi.dll. In particular when IME messages are processed in the 
>>>>>> message loop and another message triggered through a SendMessage() call, 
>>>>>> this clears the buffer context so on further processing the message loop 
>>>>>> in IME context will point to invalid memory buffer. Microsoft article 
>>>>>> devoted to this issue: 
>>>>>> https://docs.microsoft.com/en-us/troubleshoot/windows/win32/ime-crash-processing-cross-thread-sent-message
>>>>> 
>>>>> The documentation above also states that PeekMessage, called by the 
>>>>> TranslateMessage when the IME is ON, can proceed the posted messages as 
>>>>> well if that true then the current fix does not help.
>>>>> 
>>>>>> Fix:
>>>>>> Replace SendMessage() with PostMessage() for IME messages and implement 
>>>>>> event based mechanism to notify the sender that the message processing 
>>>>>> is completed.
>>>>> 
>>>>> What exact message broke the IME, the "WM_AWT_DESTROYCONTEXT"?
>>>>> 
>>>>>> Testing:
>>>>>> mach5 client tests (jtreg headful, jck, etc.) are green.
>>>>>> Regards,
>>>>>> Dmitry
>>>>> 
>>>>> 
>>>>> --
>>>>> Best regards, Sergey.
>>> 
>>> 
>>> --
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Dmitry Markov]

Hi Sergey,

I have added that information to InvokeInputMethodFunction(). Please find the 
new webrev here: http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/ 
<http://cr.openjdk.java.net/~dmarkov/8232114/webrev.01/>

Regards,
Dmitry

> On 15 Aug 2020, at 03:05, Sergey Bylokhov  wrote:
> 
> On 12.08.2020 05:09, Dmitry Markov wrote:
>> TranslateMessage() does not invoke PeekMessage(). In our case 
>> TranslateMessage() is called by AWT. IME functionality may call 
>> PeekMessage() during TranslateMessage() execution. However that 
>> PeekMessage() call is intended for processing non-queued  messages, (i.e. 
>> the messages send via SendMessage() call).
>> I contacted Microsoft regarding this problem and one of their suggestions 
>> was to use PostMessage() instead of SendMessage() for IME messages to avoid 
>> IME internal data corruption and the crash.
>> The proposed fix was tested by the stress test for several weeks and no 
>> issues were observed. So I feel quite confident that it eliminates the issue.
>> There is no exact message which triggers the crash. Usually the crash is 
>> caused by one of the following messages: WM_AWT_ASSOCIATECONTEXT or 
>> WM_AWT_SETOPENSTATUS but several times I observed that it was triggered by 
>> WM_AWT_DESTROYCONTEXT or WM_AWT_CREATECONTEXT. It looks like almost every 
>> IME-related message may cause the crash.  I think SendMessage() call should 
>> be substituted by PostMessage() for all IME messages.
> 
> Ok, then please add this(or similar) information to the new method 
> "InvokeInputMethodFunction",
> otherwise it could be removed in the future/replaced back to the sendMessage.
> 
>> Regards,
>> Dmitry
>>> On 12 Aug 2020, at 06:16, Sergey Bylokhov  
>>> wrote:
>>> 
>>> Hi, Dmitry.
>>> 
>>> On 11.08.2020 01:07, Dmitry Markov wrote:
>>>> Problem description:
>>>> The root cause of the crash is the lack of synchronisation in imjpapi.dll. 
>>>> In particular when IME messages are processed in the message loop and 
>>>> another message triggered through a SendMessage() call, this clears the 
>>>> buffer context so on further processing the message loop in IME context 
>>>> will point to invalid memory buffer. Microsoft article devoted to this 
>>>> issue: 
>>>> https://docs.microsoft.com/en-us/troubleshoot/windows/win32/ime-crash-processing-cross-thread-sent-message
>>> 
>>> The documentation above also states that PeekMessage, called by the 
>>> TranslateMessage when the IME is ON, can proceed the posted messages as 
>>> well if that true then the current fix does not help.
>>> 
>>>> Fix:
>>>> Replace SendMessage() with PostMessage() for IME messages and implement 
>>>> event based mechanism to notify the sender that the message processing is 
>>>> completed.
>>> 
>>> What exact message broke the IME, the "WM_AWT_DESTROYCONTEXT"?
>>> 
>>>> Testing:
>>>> mach5 client tests (jtreg headful, jck, etc.) are green.
>>>> Regards,
>>>> Dmitry
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Dmitry Markov]

Hi Sergey,

Thank you for looking into that.

TranslateMessage() does not invoke PeekMessage(). In our case 
TranslateMessage() is called by AWT. IME functionality may call PeekMessage() 
during TranslateMessage() execution. However that PeekMessage() call is 
intended for processing non-queued  messages, (i.e. the messages send via 
SendMessage() call). 
I contacted Microsoft regarding this problem and one of their suggestions was 
to use PostMessage() instead of SendMessage() for IME messages to avoid IME 
internal data corruption and the crash.
The proposed fix was tested by the stress test for several weeks and no issues 
were observed. So I feel quite confident that it eliminates the issue.

There is no exact message which triggers the crash. Usually the crash is caused 
by one of the following messages: WM_AWT_ASSOCIATECONTEXT or 
WM_AWT_SETOPENSTATUS but several times I observed that it was triggered by 
WM_AWT_DESTROYCONTEXT or WM_AWT_CREATECONTEXT. It looks like almost every 
IME-related message may cause the crash.  I think  SendMessage() call should be 
substituted by PostMessage() for all IME messages.

Regards,
Dmitry

> On 12 Aug 2020, at 06:16, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> 
> On 11.08.2020 01:07, Dmitry Markov wrote:
>> Problem description:
>> The root cause of the crash is the lack of synchronisation in imjpapi.dll. 
>> In particular when IME messages are processed in the message loop and 
>> another message triggered through a SendMessage() call, this clears the 
>> buffer context so on further processing the message loop in IME context will 
>> point to invalid memory buffer. Microsoft article devoted to this issue: 
>> https://docs.microsoft.com/en-us/troubleshoot/windows/win32/ime-crash-processing-cross-thread-sent-message
> 
> The documentation above also states that PeekMessage, called by the 
> TranslateMessage when the IME is ON, can proceed the posted messages as well 
> if that true then the current fix does not help.
> 
>> Fix:
>> Replace SendMessage() with PostMessage() for IME messages and implement 
>> event based mechanism to notify the sender that the message processing is 
>> completed.
> 
> What exact message broke the IME, the "WM_AWT_DESTROYCONTEXT"?
> 
>> Testing:
>> mach5 client tests (jtreg headful, jck, etc.) are green.
>> Regards,
>> Dmitry
> 
> 
> -- 
> Best regards, Sergey.

[16] RFR 8232114: JVM crashed at imjpapi.dll in native code

[Dmitry Markov]

Hello,

Could you review the fix for JDK 16, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8232114 

webrev: http://cr.openjdk.java.net/~dmarkov/8232114/webrev.00/ 


Problem description:
The root cause of the crash is the lack of synchronisation in imjpapi.dll. In 
particular when IME messages are processed in the message loop and another 
message triggered through a SendMessage() call, this clears the buffer context 
so on further processing the message loop in IME context will point to invalid 
memory buffer. Microsoft article devoted to this issue: 
https://docs.microsoft.com/en-us/troubleshoot/windows/win32/ime-crash-processing-cross-thread-sent-message
 


Fix: 
Replace SendMessage() with PostMessage() for IME messages and implement event 
based mechanism to notify the sender that the message processing is completed.

Testing:
mach5 client tests (jtreg headful, jck, etc.) are green.

Regards,
Dmitry

Re: RFR: 8078228 Default file manager and web browser didn't launch and got SecurityException

[Dmitry Markov]

Hi Sergey,

The fix looks good to me. One minor remark: shall we update "@bug” section with 
this bug id in the test? I do not need the new web rev with this change.

Regards,
Dmitry

> On 5 Aug 2020, at 07:31, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for jdk/client.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8078228
> Fix: http://cr.openjdk.java.net/~serb/8078228/webrev.00
> 
> The closed manual shell test didn't take into account that the new
> Actions were added to the Desktop class in jdk9.
> 
> The test is changed to be automatic and moved to the open repo.
> 
> -- 
> Best regards, Sergey.

Re: [15] Review Request: 8237049 Rollback the workaround for JDK-4533057

[Dmitry Markov]

Hi Sergey,

The changes look good to me.

Thanks,
Dmitry

> On 19 Jan 2020, at 23:18, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 15.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8237049
> Fix: http://cr.openjdk.java.net/~serb/8237049/webrev.00
> 
> Long-time ago we added a workaround(JDK-4280257 [1]) for
> the bug in the java plugin(JDK-4533057 [2]).
> 
> Since the the plugin was dropped and the bug is not
> reproducible any more we can delete the workaround as well.
> 
> [1] 
> https://bugs.openjdk.java.net/browse/JDK-4280257?focusedCommentId=12554208=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-12554208
> [2] https://bugs.openjdk.java.net/browse/JDK-4533057
> 
> -- 
> Best regards, Sergey.

Re: [15] Review request for 8230926: [macosx] Two apostrophes are entered instead of one with "U.S. International - PC" layout

[Dmitry Markov]

Hi Anton,

Thank you for the detailed explanation. The initial version of the fix, (i.e. 
http://cr.openjdk.java.net/~alitvinov/8230926/jdk15/webrev.00 
<http://cr.openjdk.java.net/~alitvinov/8230926/jdk15/webrev.00> ) looks good to 
me.

—Dmitry

> On 13 Jan 2020, at 17:01, Anton Litvinov  wrote:
> 
> Hello Dmitry,
> 
> Thank you for review of this fix. I tried to address your remark and suggest 
> not to encode all 5 modifier keys and leave the fix as is, because the 
> defined character array will be not readable in comparison with its variant 
> in the first fix version.
> 
> Current variant:
> "final char[] modifierKeys = {‘\’‘, ‘“’, ‘`’, ‘\u02DC’, ‘\u02C6’};"
> 
> New variant which is compilable and working:
> "final char[] modifierKeys = {‘\u005C\u0027’, ‘\u0022’, ‘\u0060’, ‘\u02DC’, 
> ‘\u02C6’};"
> 
> Not readable character in this new representation of the array is 
> ‘\u005C\u0027’. Apostrophe cannot be specified as '\u0027’, because the 
> compilation fails with error, from which it is clear that apostrophe must be 
> shielded with backslash character. '\\u0027' also cannot be compiled, because 
> the compiler states that backslash is not followed by the character that can 
> be shielded. In order to make the code compilable it is necessary to specify 
> both backslash and apostrophe in hexadecimal Unicode values what is 
> ‘\u005C\u0027’.
> 
> In the first fix version I decided to represent small tilde ˜ and circumflex 
> accent ˆ through their hexadecimal Unicode values, because they are not in 
> ASCII code table and are not plain tilde ~ and plain circumflex accent ^. On 
> macOS simple tilde ~ and simple circumflex accent ^ are not modifier keys in 
> "U.S. International - PC" keyboard layout, where instead of them as modifier 
> keys those small tilde ˜ and circumflex accent ˆ are used, which are located 
> far beyond the ASCII table. Java source code files in JDK are saved in ASCII 
> encoding, therefore in order to store ˜ and ˆ not encoded through hexadecimal 
> Unicode values, I would need to save the whole file "LWCToolkit.java" in 
> UTF-8 encoding and I did not want to change the encoding of the source file. 
> Thus I encoded just those 2 problem symbols in the array "modifierKeys".
> 
> Therefore I suggest to stay with the first fix version. Dmitry, what is your 
> opinion about this suggestion?
> 
> Thank you,
> Anton
> 
> On 07/01/2020 13:07, Dmitry Markov wrote:
>> Hi Anton,
>> 
>> The fix looks good to me.
>> 
>> Just a minor remark: I would recommend using unicode instead of character 
>> during array definition inside isCharModifierKeyInUSInternationalPC() 
>> method, (i.e. replace ‘\’’ with ‘u\0027’ for apostrophe and so on). That’s 
>> quite trivial change and I don’t need a new webrev for it.
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 4 Jan 2020, at 19:43, Sergey Bylokhov  wrote:
>>> 
>>> Looks fine.
>>> 
>>> On 12/20/19 6:53 pm, Anton Litvinov wrote:
>>>> Hi Sergey,
>>>> Thank you for review of this fix. I am not sure, that a similar issue does 
>>>> not exist for other keyboard layout with same or other modifier keys in 
>>>> JDK for macOS, because I do not know all keyboard layouts and do not know 
>>>> all their peculiarities. But I am sure that this JDK code related to input 
>>>> methods on macOS is very fragile and depends on many flags and "if" 
>>>> conditions in Objective-C and Java source code, and that is why all bugs 
>>>> related to input methods for last several years were fixed in narrow ways 
>>>> to address only those uncovered problematic scenarios.
>>>> This particular bug exists in Oracle JDK from the very first release, the 
>>>> issue is reproducible with JDK 7u4 b21, this bug always affected "U.S. 
>>>> International - PC" layout in JDK. The issue is also reproducible with 
>>>> Apple JDK 1.6.0_65-b14-468.
>>>> To my mind, a source of these problems with input methods is the fact that 
>>>> JDK receives explicit direction from macOS to insert certain characters 
>>>> through the method "(void) insertText:(id)aString 
>>>> replacementRange:(NSRange)replacementRange" in "AWTView.m" file, and in 
>>>> some cases JDK generates "java.awt.event.InputMethodEvent" as a result of 
>>>> this call and in some cases does not generate "InputMethodEvent" with the 
>>>> intention to generate "java.awt.event.KeyEvent" events instead of it as 
>>>> part

Re: [15] Review request for 8230926: [macosx] Two apostrophes are entered instead of one with "U.S. International - PC" layout

[Dmitry Markov]

Hi Anton,

The fix looks good to me. 

Just a minor remark: I would recommend using unicode instead of character 
during array definition inside isCharModifierKeyInUSInternationalPC() method, 
(i.e. replace ‘\’’ with ‘u\0027’ for apostrophe and so on). That’s quite 
trivial change and I don’t need a new webrev for it.

Thanks,
Dmitry 

> On 4 Jan 2020, at 19:43, Sergey Bylokhov  wrote:
> 
> Looks fine.
> 
> On 12/20/19 6:53 pm, Anton Litvinov wrote:
>> Hi Sergey,
>> Thank you for review of this fix. I am not sure, that a similar issue does 
>> not exist for other keyboard layout with same or other modifier keys in JDK 
>> for macOS, because I do not know all keyboard layouts and do not know all 
>> their peculiarities. But I am sure that this JDK code related to input 
>> methods on macOS is very fragile and depends on many flags and "if" 
>> conditions in Objective-C and Java source code, and that is why all bugs 
>> related to input methods for last several years were fixed in narrow ways to 
>> address only those uncovered problematic scenarios.
>> This particular bug exists in Oracle JDK from the very first release, the 
>> issue is reproducible with JDK 7u4 b21, this bug always affected "U.S. 
>> International - PC" layout in JDK. The issue is also reproducible with Apple 
>> JDK 1.6.0_65-b14-468.
>> To my mind, a source of these problems with input methods is the fact that 
>> JDK receives explicit direction from macOS to insert certain characters 
>> through the method "(void) insertText:(id)aString 
>> replacementRange:(NSRange)replacementRange" in "AWTView.m" file, and in some 
>> cases JDK generates "java.awt.event.InputMethodEvent" as a result of this 
>> call and in some cases does not generate "InputMethodEvent" with the 
>> intention to generate "java.awt.event.KeyEvent" events instead of it as part 
>> of invocation of, for example, "(void) keyDown: (NSEvent *)event" method in 
>> "AWTView.m" file.
>> In this bug with "U.S. International - PC" layout, after pressing " ' ", " p 
>> " characters JDK's method "insertText" from "AWTView.m" file is invoked by 
>> macOS two times: during the first time JDK is asked to insert " ' ", and 
>> during the second time it is asked to insert " p " character. So if JDK 
>> followed that direction from macOS, there would not be a bug, but JDK has 
>> own requirement to generate only key events for Latin characters, and "U.S. 
>> International - PC" layout relies only on key codes corresponding to Latin 
>> characters while at the same time provides input method functionality for 
>> insertion of characters with diacritics through pressing these modifier keys 
>> " '`"˜ˆ " followed by required regular Latin character keys.
>> I had been debugging this bug and looking at it periodically for three 
>> years, of course I was trying to fix this bug from different sides. But this 
>> fix is the only well explained and reliable fix for this bug which I managed 
>> to create and it excludes a possibility of breaking JDK work with any other 
>> keyboard layouts on macOS.
>> Thank you,
>> Anton
>> On 20/12/2019 06:36, Sergey Bylokhov wrote:
>>> Hi, Anton.
>>> 
>>> How we can be sure that this problem exists only in case of "U.S. 
>>> International - PC"?
>>> Did you try to check other layouts and or possible fix variation w/o 
>>> hardcoded check for the layout?
>>> 
>>> On 12/16/19 7:01 pm, Anton Litvinov wrote:
 Hello,
 
 Could you please review the following fix for the bug.
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8230926
 Webrev: http://cr.openjdk.java.net/~alitvinov/8230926/jdk15/webrev.00
 
 The bug consists in the fact that if on macOS with currently set "U.S. 
 International - PC" keyboard layout a user presses any of 5 modifier keys: 
 ' (APOSTROPHE), " (QUOTATION MARK), ` (ACCENT GRAVE), ˜ (SMALL TILDE), ˆ 
 (CIRCUMFLEX ACCENT) and then presses any consonant Latin letter key, for 
 example "p" key, then 2 consecutive modifier keys are entered in AWT or 
 Swing text field and the consonant letter is not entered.
 
 CAUSE OF THE BUG:
 
 The second extra modifier key is entered in the text field instead of the 
 required letter during execution of the function "(void) keyDown: (NSEvent 
 *)event" in "src/java.desktop//macosx/native/libawt_lwawt/awt/AWTView.m" 
 for "NSEvent" instance representing pressing on the consonant letter key, 
 for example that "p" key, through the next sequence of function/method 
 calls:
 
 "(void) keyDown: (NSEvent *)event" - "AWTView.m" file.
 |-> "(void) deliverJavaKeyEventHelper: (NSEvent *) event" - "AWTView.m" 
 file
|-> "sun.lwawt.macosx.CPlatformView.deliverKeyEvent(NSEvent)" - 
 "CPlatformView.java" file
  |-> "sun.lwawt.macosx.CPlatformResponder.handleKeyEvent" - 
 "CPlatformResponder.java" file
 
 The instance of "NSEvent" being handled which corresponds to press on 

Re: [14] Review Request: 8236545 Compilation error in mach5 java/awt/FileDialog/MacOSGoToFolderCrash.java

[Dmitry Markov]

Looks good to me.

Thanks,
Dmitry

> On 30 Dec 2019, at 23:07, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 14.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8236545
> Fix: http://cr.openjdk.java.net/~serb/8236545/webrev.00
> 
> This test may fail due to compilation error "unmappable character" for 
> encoding US-ASCII.
> 
> I could not reproduce it locally but our mach5 systems are affected, so I
> would like to fix it in jdk 14. The fix is trivial and affects the text in
> the comment.
> 
> -- 
> Best regards, Sergey.

Re: [14] Review Request: 8235739 Rare NPE at WComponentPeer.getGraphics()

[Dmitry Markov]

+1

Thanks,
Dmitry

> On 13 Dec 2019, at 19:12, Alexey Ivanov  wrote:
> 
> On 12/12/2019 23:14, Sergey Bylokhov wrote:
>> On 12/12/19 7:58 am, Alexey Ivanov wrote:
>>> Shall we return null if wpeer is already null?
>>> Rather than falling back to this.surfaceData.
>> 
>> wpeer is a peer of the window which contains the current component,
>> probably if it is null then the window was disposed of already, but
>> since the current method is in the peer as well(the peer of the
>> component) then we will try to use its surface if it was not
>> disposed of yet.
> 
> Makes sense.
> Looks good to me.
> 
> Regards,
> Alexey
> 
>> 
>>> 
>>> On 11/12/2019 22:31, Sergey Bylokhov wrote:
 webrev is re-uploaded.
 
 On 12/10/19 9:23 pm, Sergey Bylokhov wrote:
> Hello.
> Please review the fix for JDK 14.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8235739
> Fix: http://cr.openjdk.java.net/~serb/8235739/webrev.00
> 
> In the WComponentPeer.getGraphics() we may get NPE because
> AWTAccessorgetPeer("window") can return null if the "window"
> was disposed already and its peer was set to null.

Re: [14] Review Request: 8235638 NPE in LWWindowPeer.getOnscreenGraphics()

[Dmitry Markov]

Hi Sergey,

The fix looks good to me.

Assume that this one and the fix for 8235739 [1] will be integrated together to 
avoid test failure on Windows platform.

Thanks,
Dmitry 

[1] - https://bugs.openjdk.java.net/browse/JDK-8235739

> On 11 Dec 2019, at 22:11, Sergey Bylokhov  wrote:
> 
> I am sorry, I have uploaded it from one system, and then accidentally removed
> it by synchronization from another system. webrev is accessible now.
> 
> On 12/11/19 9:58 am, Phil Race wrote:
>> Maybe Sergey forgot to post it.
>> -phil.
>> On 12/11/19 1:53 AM, Prasanta Sadhukhan wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> It seems that webrev is not accessible anymore, but I guess it will be good 
>>> if both the fix can be clubbed together.
>>> 
>>> Regards
>>> 
>>> Prasanta
>>> 
>>> On 11-Dec-19 3:15 PM, Dmitry Markov wrote:
>>>> Hi Prasanta,
>>>> 
>>>> I guess the NPE, you observed, is already addressed by 
>>>> https://bugs.openjdk.java.net/browse/JDK-8235739 (which is on review now, 
>>>> see 
>>>> http://mail.openjdk.java.net/pipermail/awt-dev/2019-December/015621.html).
>>>> Can you try with fix for 8235739, please?
>>>> 
>>>> Thanks,
>>>> Dmitry
>>>> 
>>>>> On 11 Dec 2019, at 09:16, Prasanta Sadhukhan 
>>>>> mailto:prasanta.sadhuk...@oracle.com> 
>>>>> <mailto:prasanta.sadhuk...@oracle.com 
>>>>> <mailto:prasanta.sadhuk...@oracle.com>>> wrote:
>>>>> 
>>>>> Hi Sergey,
>>>>> 
>>>>> The regression test fails for me on windows with NPE
>>>>> 
>>>>> java.lang.NullPointerException
>>>>>   at 
>>>>> java.desktop/sun.awt.windows.WComponentPeer.getGraphics(WComponentPeer.java:582)
>>>>>   at java.desktop/java.awt.Component.getGraphics(Component.java:3124)
>>>>>   at GetGraphicsStressTest.lambda$test$2(GetGraphicsStressTest.java:63)
>>>>>   at java.base/java.lang.Thread.run(Thread.java:833)
>>>>> Probably we need to do a null check there in windows too.
>>>>> 
>>>>> Regards
>>>>> Prasanta
>>>>> On 11-Dec-19 1:02 AM, Sergey Bylokhov wrote:
>>>>>> Hello.
>>>>>> Please review the fix for JDK 14.
>>>>>> 
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8235638
>>>>>> Fix: http://cr.openjdk.java.net/~serb/8235638/webrev.00
>>>>>> 
>>>>>> I have found a root cause of intermittent failures of some stress tests 
>>>>>> in the JDK on macOS.
>>>>>> Such tests usually show/hide a lot of frames, and fails because of NPE 
>>>>>> in the
>>>>>> LWWindowPeer.getOnscreenGraphics()
>>>>>> The reason is incorrect null check. We should read the surfaceData to 
>>>>>> the local var apply a
>>>>>> null check and then use it, otherwise, the data may be changed to null 
>>>>>> after the check.
>>>>>> 
>>>>>> 
>>>> 
> 
> 
> -- 
> Best regards, Sergey.

Re: [14] Review Request: 8235638 NPE in LWWindowPeer.getOnscreenGraphics()

[Dmitry Markov]

Hi Prasanta,

I guess the NPE, you observed, is already addressed by 
https://bugs.openjdk.java.net/browse/JDK-8235739 
 (which is on review now, see 
http://mail.openjdk.java.net/pipermail/awt-dev/2019-December/015621.html 
).
Can you try with fix for 8235739, please?

Thanks,
Dmitry

> On 11 Dec 2019, at 09:16, Prasanta Sadhukhan  
> wrote:
> 
> Hi Sergey,
> 
> The regression test fails for me on windows with NPE
> 
> java.lang.NullPointerException
>   at 
> java.desktop/sun.awt.windows.WComponentPeer.getGraphics(WComponentPeer.java:582)
>   at java.desktop/java.awt.Component.getGraphics(Component.java:3124)
>   at GetGraphicsStressTest.lambda$test$2(GetGraphicsStressTest.java:63)
>   at java.base/java.lang.Thread.run(Thread.java:833)
> Probably we need to do a null check there in windows too.
> 
> Regards
> Prasanta
> On 11-Dec-19 1:02 AM, Sergey Bylokhov wrote:
>> Hello. 
>> Please review the fix for JDK 14. 
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8235638 
>>  
>> Fix: http://cr.openjdk.java.net/~serb/8235638/webrev.00 
>>  
>> 
>> I have found a root cause of intermittent failures of some stress tests in 
>> the JDK on macOS. 
>> Such tests usually show/hide a lot of frames, and fails because of NPE in 
>> the 
>> LWWindowPeer.getOnscreenGraphics() 
>> The reason is incorrect null check. We should read the surfaceData to the 
>> local var apply a 
>> null check and then use it, otherwise, the data may be changed to null after 
>> the check. 
>> 
>> 

Re: [14] Review Request: 8234522 [macos] Crash with use of native file dialog

[Dmitry Markov]

Hi Sergey,

I agree, it would be better to replace NSApplicationDefined with 
NSEventTypeApplicationDefined under separate task.

The fix still looks good to me.

Thanks,
Dmitry

PS
Do not forget to add a summary to the regression test

> On 10 Dec 2019, at 18:20, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> 
> We probably can replace NSApplicationDefined by 
> NSEventTypeApplicationDefined, but then we will need
> to replace NSApplicationDefinedMask by NSEventMaskApplicationDefined, and 
> probably something else.
> So I leave it for some future deprecation-cleanup.
> 
> On 12/10/19 4:03 am, Dmitry Markov wrote:
>> Hi Sergey,
>> The changes look OK, but I have a few remarks. I would recommend using 
>> NSEventTypeApplicationDefined due to deprecation of NSApplicationDefined. 
>> Also it would be good to add some kind of summary to the regression test. I 
>> do not need a new webrev with suggested changes.
>> Thanks,
>> Dmitry
>>> On 6 Dec 2019, at 23:05, Sergey Bylokhov  wrote:
>>> 
>>> Hello.
>>> Please review the fix for JDK 14.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8234522
>>> Fix: http://cr.openjdk.java.net/~serb/8234522/webrev.00
>>> 
>>> This bug is tricky, we have a special mechanism to postpone deallocation
>>> of the objects on the Appkit thread.
>>> It solves the next problem: the native window pointer is disposed in the
>>> inner AppKit loop, while it is still referenced on the stack in the native
>>> Cocoa method which caused the mentioned inner loop. When the inner loop is
>>> exited Cocoa crashes while dereferencing this window pointer.
>>> 
>>> This mechanism is implemented via posting NSEvent using NSApplicationDefined
>>> type and filter out this event in the nested event loop.
>>> 
>>> But for some reason, macOS send us exactly the same event(type+subtype), 
>>> which
>>> we try to use for deallocation and of course, this attempt will crashes.
>>> (probably because all constants of NSEvent we use were deprecated?)
>>> 
>>> In the fix I have added additional marks(via data fields) to the event to 
>>> make
>>> it easy to check it was created by us, also I have bump the initial value of
>>> subtype to minimize possible collisions.
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [14] Review Request: 8234522 [macos] Crash with use of native file dialog

[Dmitry Markov]

Hi Sergey,

The changes look OK, but I have a few remarks. I would recommend using 
NSEventTypeApplicationDefined due to deprecation of NSApplicationDefined. Also 
it would be good to add some kind of summary to the regression test. I do not 
need a new webrev with suggested changes.

Thanks,
Dmitry

> On 6 Dec 2019, at 23:05, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 14.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8234522
> Fix: http://cr.openjdk.java.net/~serb/8234522/webrev.00
> 
> This bug is tricky, we have a special mechanism to postpone deallocation
> of the objects on the Appkit thread.
> It solves the next problem: the native window pointer is disposed in the
> inner AppKit loop, while it is still referenced on the stack in the native
> Cocoa method which caused the mentioned inner loop. When the inner loop is
> exited Cocoa crashes while dereferencing this window pointer.
> 
> This mechanism is implemented via posting NSEvent using NSApplicationDefined
> type and filter out this event in the nested event loop.
> 
> But for some reason, macOS send us exactly the same event(type+subtype), which
> we try to use for deallocation and of course, this attempt will crashes.
> (probably because all constants of NSEvent we use were deprecated?)
> 
> In the fix I have added additional marks(via data fields) to the event to make
> it easy to check it was created by us, also I have bump the initial value of
> subtype to minimize possible collisions.
> 
> 
> -- 
> Best regards, Sergey.

Re: RFC: JDK-8231991: Mouse wheel change focus on awt/swing windows

[Dmitry Markov]

Hi Mario,

The fix looks good to me.

Thanks,
Dmitry

> On 11 Nov 2019, at 09:42, Mario Torre  wrote:
> 
> On Thu, 2019-10-31 at 12:00 +0100, Mario Torre wrote:
>> On Wed, 2019-10-30 at 15:45 -0700, Sergey Bylokhov wrote:
>>> Looks fine.
>> 
>> Thanks!
>> 
>> I believe I still need a second reviewer's approval? (or, is this
>> rule still in place?)
>> 
> 
> Ping?
> 
> Cheers,
> Mario
> -- 
> Mario Torre
> Associate Manager, Software Engineering
> Red Hat GmbH 
> 9704 A60C B4BE A8B8 0F30  9205 5D7E 4952 3F65 7898
> 

Re: [14] RFR JDK-8233703:java/awt/Frame/FrameLocation.java fails on mac

[Dmitry Markov]

Thanks for the clarification. If the issue always happens on the same machine 
it makes sense to look into this host. May be some of its settings are 
incorrect and it should be excluded from the list of hosts.

Thanks,
Dmitry

> On 6 Nov 2019, at 15:38, Prasanta Sadhukhan  
> wrote:
> 
> Hi Dmitry.
> 
> My ubuntu18.04 is running on Gnome and I think mach5 linux is also running 
> same, I do not think mach5 linux system is running on Compiz window manager, 
> where the issue was seen earlier.
> 
> Regards
> Prasanta
> On 06-Nov-19 8:29 PM, Dmitry Markov wrote:
>> Hi Prasanta,
>> 
>> I do not see any problems in the test. Its current version looks fine. The 
>> proposed modifications (in particular omitting sleep() invocation) may 
>> disguise some real problems (see my previous email).
>> Yes, the original fix is still in place but I guess something must have 
>> changed and the issue got visible again. According to JDK-6895647 
>> <https://bugs.openjdk.java.net/browse/JDK-6895647> the original issue was 
>> only reproducible with specific window manager. That may explain why it does 
>> not take place on your local system but happens on some another.
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 6 Nov 2019, at 14:25, Prasanta Sadhukhan >> <mailto:prasanta.sadhuk...@oracle.com>> wrote:
>>> 
>>> 
>>> 
>>> On 06-Nov-19 7:50 PM, Prasanta Sadhukhan wrote:
>>>> Hi Dmitry,
>>>> 
>>>> Why do you think the problem is not in the test? I guess this exact issue 
>>>> of incorrect frame location is fixed by JDK-6895647 
>>>> <https://bugs.openjdk.java.net/browse/JDK-6895647> in linux and the fix 
>>>> still is there in XDecoratedPeer.java.
>>>> 
>>> Also, the test pass always in local ubuntu18.04 system that I have. it only 
>>> fails on mach5 linux system (that too not every time) which makes me think 
>>> it's a test issue.
>>>> Regards
>>>> Prasanta
>>>> On 06-Nov-19 7:13 PM, Dmitry Markov wrote:
>>>>> Hi Prasanta,
>>>>> 
>>>>> If I got it right, we might not have enough time to show frame on the 
>>>>> screen without sleep() invocation. As a result incorrect frame location 
>>>>> will be retrieved. So I think we should NOT get rid of sleep() call here.
>>>>> 
>>>>> Also it appears that the problem is not in the test at all.
>>>>> 
>>>>> Thanks,
>>>>> Dmitry
>>>>> 
>>>>>> On 6 Nov 2019, at 13:09, Prasanta Sadhukhan 
>>>>>>  <mailto:prasanta.sadhuk...@oracle.com> 
>>>>>> wrote:
>>>>>> 
>>>>>> Actually the failure is on linux. I have updated the JBS summary.
>>>>>> 
>>>>>> On 06-Nov-19 5:27 PM, Prasanta Sadhukhan wrote:
>>>>>>> Hi All,
>>>>>>> 
>>>>>>> Please review a fix for an issue where it is seen the frame location is 
>>>>>>> sometimes wrong in mac on mach5 headful nightly run.
>>>>>>> 
>>>>>>> It seems to be a timing issue as it shows 10 frames by calling 
>>>>>>> frame.removeNotify(),frame.addNotify() repeatedly, which by the way are 
>>>>>>> not supposed to be called by programs directly.
>>>>>>> 
>>>>>>> Proposed fix is to make not to make thread sleep every time 
>>>>>>> removeNotify,addNotify is called so that there is no delay. I have ran 
>>>>>>> mach5 job (in JBS) for 3 consecutive runs on all 3 platforms and they 
>>>>>>> pass.
>>>>>>> 
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233703 
>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8233703>
>>>>>>> 
>>>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8233703/webrev.0/ 
>>>>>>> <http://cr.openjdk.java.net/~psadhukhan/8233703/webrev.0/>
>>>>>>> 
>>>>>>> Regards
>>>>>>> Prasanta
>> 

Re: [14] RFR JDK-8233703:java/awt/Frame/FrameLocation.java fails on mac

[Dmitry Markov]

Hi Prasanta,

I do not see any problems in the test. Its current version looks fine. The 
proposed modifications (in particular omitting sleep() invocation) may disguise 
some real problems (see my previous email).
Yes, the original fix is still in place but I guess something must have changed 
and the issue got visible again. According to JDK-6895647 
<https://bugs.openjdk.java.net/browse/JDK-6895647> the original issue was only 
reproducible with specific window manager. That may explain why it does not 
take place on your local system but happens on some another.

Thanks,
Dmitry

> On 6 Nov 2019, at 14:25, Prasanta Sadhukhan  
> wrote:
> 
> 
> 
> On 06-Nov-19 7:50 PM, Prasanta Sadhukhan wrote:
>> Hi Dmitry,
>> 
>> Why do you think the problem is not in the test? I guess this exact issue of 
>> incorrect frame location is fixed by JDK-6895647 
>> <https://bugs.openjdk.java.net/browse/JDK-6895647> in linux and the fix 
>> still is there in XDecoratedPeer.java.
>> 
> Also, the test pass always in local ubuntu18.04 system that I have. it only 
> fails on mach5 linux system (that too not every time) which makes me think 
> it's a test issue.
>> Regards
>> Prasanta
>> On 06-Nov-19 7:13 PM, Dmitry Markov wrote:
>>> Hi Prasanta,
>>> 
>>> If I got it right, we might not have enough time to show frame on the 
>>> screen without sleep() invocation. As a result incorrect frame location 
>>> will be retrieved. So I think we should NOT get rid of sleep() call here.
>>> 
>>> Also it appears that the problem is not in the test at all.
>>> 
>>> Thanks,
>>> Dmitry
>>> 
>>>> On 6 Nov 2019, at 13:09, Prasanta Sadhukhan 
>>>>  <mailto:prasanta.sadhuk...@oracle.com> 
>>>> wrote:
>>>> 
>>>> Actually the failure is on linux. I have updated the JBS summary.
>>>> 
>>>> On 06-Nov-19 5:27 PM, Prasanta Sadhukhan wrote:
>>>>> Hi All,
>>>>> 
>>>>> Please review a fix for an issue where it is seen the frame location is 
>>>>> sometimes wrong in mac on mach5 headful nightly run.
>>>>> 
>>>>> It seems to be a timing issue as it shows 10 frames by calling 
>>>>> frame.removeNotify(),frame.addNotify() repeatedly, which by the way are 
>>>>> not supposed to be called by programs directly.
>>>>> 
>>>>> Proposed fix is to make not to make thread sleep every time 
>>>>> removeNotify,addNotify is called so that there is no delay. I have ran 
>>>>> mach5 job (in JBS) for 3 consecutive runs on all 3 platforms and they 
>>>>> pass.
>>>>> 
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233703 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8233703>
>>>>> 
>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8233703/webrev.0/ 
>>>>> <http://cr.openjdk.java.net/~psadhukhan/8233703/webrev.0/>
>>>>> 
>>>>> Regards
>>>>> Prasanta

Re: [14] RFR JDK-8233703:java/awt/Frame/FrameLocation.java fails on mac

[Dmitry Markov]

Hi Prasanta,

If I got it right, we might not have enough time to show frame on the screen 
without sleep() invocation. As a result incorrect frame location will be 
retrieved. So I think we should NOT get rid of sleep() call here.

Also it appears that the problem is not in the test at all.

Thanks,
Dmitry

> On 6 Nov 2019, at 13:09, Prasanta Sadhukhan  
> wrote:
> 
> Actually the failure is on linux. I have updated the JBS summary.
> 
> On 06-Nov-19 5:27 PM, Prasanta Sadhukhan wrote:
>> Hi All,
>> 
>> Please review a fix for an issue where it is seen the frame location is 
>> sometimes wrong in mac on mach5 headful nightly run.
>> 
>> It seems to be a timing issue as it shows 10 frames by calling 
>> frame.removeNotify(),frame.addNotify() repeatedly, which by the way are not 
>> supposed to be called by programs directly.
>> 
>> Proposed fix is to make not to make thread sleep every time 
>> removeNotify,addNotify is called so that there is no delay. I have ran mach5 
>> job (in JBS) for 3 consecutive runs on all 3 platforms and they pass.
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233703
>> 
>> webrev: http://cr.openjdk.java.net/~psadhukhan/8233703/webrev.0/
>> 
>> Regards
>> Prasanta

Re: RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Dmitry Markov]

Thank you, Magnus!

Dmitry

> On 4 Nov 2019, at 10:24, Magnus Ihse Bursie  
> wrote:
> 
> 
> 
> On 2019-11-03 16:38, Dmitry Markov wrote:
>> Alexey and Sergey, thank you for the approval!
>> I wonder whether it is enough or I need one more “+1” from build-folk.
> You really don't need it, but here you get one. :-)
> 
> I think it was good that you continued polish the addition. The latest 
> version looks much better than the original suggestion!
> 
> /Magnus
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 1 Nov 2019, at 20:58, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com>> wrote:
>>> 
>>> +1
>>> 
>>> On 11/1/19 7:41 am, Alexey Ivanov wrote:
>>>> Thank you, Dmitry!
>>>> The changes look good to me.
>>>> On 01/11/2019 13:09, Dmitry Markov wrote:
>>>>> Hi Alexey,
>>>>> 
>>>>> I have updated the fix. Please find the new version here: 
>>>>> http://cr.openjdk.java.net/~dmarkov/8232880/webrev.03/ 
>>>>> <http://cr.openjdk.java.net/~dmarkov/8232880/webrev.03/>
>>>>> 
>>>>> Thanks,
>>>>> Dmitry
>>>>> 
>>>>>> On 31 Oct 2019, at 16:27, Alexey Ivanov >>>>> <mailto:alexey.iva...@oracle.com> <mailto:alexey.iva...@oracle.com 
>>>>>> <mailto:alexey.iva...@oracle.com>>> wrote:
>>>>>> 
>>>>>> Hi Dmitry,
>>>>>> 
>>>>>> 437 …by the operating system. …
>>>>>> 
>>>>>> I'd modify the following text a bit:
>>>>>> To run the test correctly, the default global key shortcut should be 
>>>>>> disabled. Follow the steps above, and then deselect "Turn keyboard 
>>>>>> access on or off" property which is responsible for `CTRL + F1` 
>>>>>> combination.
>>>>>> 
>>>>>> Does it sound clearer?
>>>>>> I'd not use backticks on the "Turn keyboard access on or off" because 
>>>>>> it's not something user is typing, nor is it a piece of code. Is the 
>>>>>> word “property” correct? Does “shortcut” or “option” fit better?
>>>>>> 
>>>>>> I'd recommend adding quotes around the option to look for:
>>>>>> 448 in the right-side pane look for "Turn off Windows key hotkeys" and 
>>>>>> double click on it;
>>>>>> 
>>>>>> Consider adding an empty line before this line
>>>>>> 450 Note: restart is required to make the settings take effect.
>>>>>> to make it a separate paragraph in HTML.
>>>>>> 
>>>>>> 
>>>>>> Regards,
>>>>>> Alexey
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
>> 
> 

Re: RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Dmitry Markov]

Alexey and Sergey, thank you for the approval!
I wonder whether it is enough or I need one more “+1” from build-folk.

Thanks,
Dmitry

> On 1 Nov 2019, at 20:58, Sergey Bylokhov  wrote:
> 
> +1
> 
> On 11/1/19 7:41 am, Alexey Ivanov wrote:
>> Thank you, Dmitry!
>> The changes look good to me.
>> On 01/11/2019 13:09, Dmitry Markov wrote:
>>> Hi Alexey,
>>> 
>>> I have updated the fix. Please find the new version here: 
>>> http://cr.openjdk.java.net/~dmarkov/8232880/webrev.03/
>>> 
>>> Thanks,
>>> Dmitry
>>> 
>>>> On 31 Oct 2019, at 16:27, Alexey Ivanov >>> <mailto:alexey.iva...@oracle.com>> wrote:
>>>> 
>>>> Hi Dmitry,
>>>> 
>>>> 437 …by the operating system. …
>>>> 
>>>> I'd modify the following text a bit:
>>>> To run the test correctly, the default global key shortcut should be 
>>>> disabled. Follow the steps above, and then deselect "Turn keyboard access 
>>>> on or off" property which is responsible for `CTRL + F1` combination.
>>>> 
>>>> Does it sound clearer?
>>>> I'd not use backticks on the "Turn keyboard access on or off" because it's 
>>>> not something user is typing, nor is it a piece of code. Is the word 
>>>> “property” correct? Does “shortcut” or “option” fit better?
>>>> 
>>>> I'd recommend adding quotes around the option to look for:
>>>> 448 in the right-side pane look for "Turn off Windows key hotkeys" and 
>>>> double click on it;
>>>> 
>>>> Consider adding an empty line before this line
>>>> 450 Note: restart is required to make the settings take effect.
>>>> to make it a separate paragraph in HTML.
>>>> 
>>>> 
>>>> Regards,
>>>> Alexey
> 
> 
> -- 
> Best regards, Sergey.

Re: RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Dmitry Markov]

Hi Alexey,

I have updated the fix. Please find the new version here: 
http://cr.openjdk.java.net/~dmarkov/8232880/webrev.03/ 
<http://cr.openjdk.java.net/~dmarkov/8232880/webrev.03/>

Thanks,
Dmitry

> On 31 Oct 2019, at 16:27, Alexey Ivanov  wrote:
> 
> Hi Dmitry,
> 
> 437 …by the operating system. …
> 
> I'd modify the following text a bit:
> To run the test correctly, the default global key shortcut should be 
> disabled. Follow the steps above, and then deselect "Turn keyboard access on 
> or off" property which is responsible for `CTRL + F1` combination.
> 
> Does it sound clearer?
> I'd not use backticks on the "Turn keyboard access on or off" because it's 
> not something user is typing, nor is it a piece of code. Is the word 
> “property” correct? Does “shortcut” or “option” fit better?
> 
> I'd recommend adding quotes around the option to look for:
> 448 in the right-side pane look for "Turn off Windows key hotkeys" and double 
> click on it;
> 
> Consider adding an empty line before this line
> 450 Note: restart is required to make the settings take effect.
> to make it a separate paragraph in HTML.
> 
> 
> Regards,
> Alexey
> 
> On 31/10/2019 10:56, Dmitry Markov wrote:
>> Hi Alexey,
>> 
>> I have updated the fix based on your recommendation. The new version is 
>> located at: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.02/
>> Also please find my answers inline.
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 29 Oct 2019, at 19:29, Alexey Ivanov >> <mailto:alexey.iva...@oracle.com>> wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> Shall we drop hyphen in the header: “Client UI Tests”?
>>> 
>>> I think there should be no definite article in this sentence: “use -the- 
>>> key sequences”.
>>> 
>>> “…Turn off Windowss key hotkeys…”, there's an extra ‘s’ in Windows.
>>> 
>>> ”Note: restart is required to make the settings take effect.”
>>> Just to confirm: is signing out and signing in not enough?
>> According to Microsoft site: restart is required but I guess signing out/in 
>> should work too. Unfortunately I do not have Windows on hand to check it out.
>> 
>>> 
>>> I'd use backticks for gpedit markup: “Type `gpedit` in the Search…”
>>> 
>>> 
>>> Does it make sense to move the example into macOS section? Then the steps 
>>> to disable the shortcut can be reduced to the required option only. The 
>>> steps themselves should not be listed as code, i.e. no backticks.
>>> 
>>> (For my understanding, "Turn keyboard access on or off" turns off only one 
>>> specific shortcut, i.e. Ctrl+F1?)
>> Yes, that’s right. I have added clarification to the doc.
>> 
>>> 
>>> 
>>> Regards,
>>> Alexey

Re: RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Dmitry Markov]

Hi Alexey,

I have updated the fix based on your recommendation. The new version is located 
at: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.02/ 
<http://cr.openjdk.java.net/~dmarkov/8232880/webrev.02/>
Also please find my answers inline.

Thanks,
Dmitry

> On 29 Oct 2019, at 19:29, Alexey Ivanov  wrote:
> 
> Hi Dmitry,
> 
> Shall we drop hyphen in the header: “Client UI Tests”?
> 
> I think there should be no definite article in this sentence: “use -the- key 
> sequences”.
> 
> “…Turn off Windowss key hotkeys…”, there's an extra ‘s’ in Windows.
> 
> ”Note: restart is required to make the settings take effect.”
> Just to confirm: is signing out and signing in not enough?
According to Microsoft site: restart is required but I guess signing out/in 
should work too. Unfortunately I do not have Windows on hand to check it out. 

> 
> I'd use backticks for gpedit markup: “Type `gpedit` in the Search…”
> 
> 
> Does it make sense to move the example into macOS section? Then the steps to 
> disable the shortcut can be reduced to the required option only. The steps 
> themselves should not be listed as code, i.e. no backticks.
> 
> (For my understanding, "Turn keyboard access on or off" turns off only one 
> specific shortcut, i.e. Ctrl+F1?)
Yes, that’s right. I have added clarification to the doc.

> 
> 
> Regards,
> Alexey
> 
> On 28/10/2019 17:23, Dmitry Markov wrote:
>> Hi Alexey, Erik, Sergey,
>> Thank you for your feedback. I have updated the fix based on your 
>> suggestions: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.01/ 
>> <http://cr.openjdk.java.net/~dmarkov/8232880/webrev.01/>
>> Can you take a look, please?
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 26 Oct 2019, at 00:55, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com>> wrote:
>>> 
>>> Hi, Dmitry.
>>> 
>>> I suggest to make this block more generic, and describe the  "### Client UI 
>>> - Tests".
>>> It would be useful to mention that the tests in this category use different 
>>> key combinations,
>>> which might be registered as a system shortcuts, and it could cause a test 
>>> failure. It is suggested
>>> to disable these shortcuts:
>>> - macOS config location.
>>> - Linux config location.
>>> - On window some other application may have global shortcuts
>>> 
>>> As an example of such issue you can provide CTRL+F1 which is used
>>> 
>>> 
>>> On 10/25/19 3:27 am, Dmitry Markov wrote:
>>>> Hello,
>>>> Could you review the fix for jdk14, please?
>>>>bug: https://bugs.openjdk.java.net/browse/JDK-8232880 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8232880> 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8232880 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8232880>>
>>>>webrev: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/ 
>>>> <http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/> 
>>>> <http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/ 
>>>> <http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/>>
>>>> Some Client UI tests use the following key sequence to show/hide tooltip 
>>>> message: “CTRL” + “F1". However this key combination is reserved by 
>>>> operating system on OS X platform. As a results the tests fail.
>>>> Test documentation should be updated with some notes regarding this.
>>>> Thanks,
>>>> Dmitry
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
>> 
> 

Re: RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Dmitry Markov]

Hi Alexey, Erik, Sergey,
Thank you for your feedback. I have updated the fix based on your suggestions: 
http://cr.openjdk.java.net/~dmarkov/8232880/webrev.01/ 
<http://cr.openjdk.java.net/~dmarkov/8232880/webrev.01/>
Can you take a look, please?

Thanks,
Dmitry

> On 26 Oct 2019, at 00:55, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> 
> I suggest to make this block more generic, and describe the  "### Client UI - 
> Tests".
> It would be useful to mention that the tests in this category use different 
> key combinations,
> which might be registered as a system shortcuts, and it could cause a test 
> failure. It is suggested
> to disable these shortcuts:
> - macOS config location.
> - Linux config location.
> - On window some other application may have global shortcuts
> 
> As an example of such issue you can provide CTRL+F1 which is used
> 
> 
> On 10/25/19 3:27 am, Dmitry Markov wrote:
>> Hello,
>> Could you review the fix for jdk14, please?
>>  bug: https://bugs.openjdk.java.net/browse/JDK-8232880 
>> <https://bugs.openjdk.java.net/browse/JDK-8232880>
>>  webrev: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/ 
>> <http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/>
>> Some Client UI tests use the following key sequence to show/hide tooltip 
>> message: “CTRL” + “F1". However this key combination is reserved by 
>> operating system on OS X platform. As a results the tests fail.
>> Test documentation should be updated with some notes regarding this.
>> Thanks,
>> Dmitry
> 
> 
> -- 
> Best regards, Sergey.

RFR 8232880: Update test documentation with additional settings for client UI tooltip tests

[Dmitry Markov]

Hello,

Could you review the fix for jdk14, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8232880 

webrev: http://cr.openjdk.java.net/~dmarkov/8232880/webrev.00/ 


Some Client UI tests use the following key sequence to show/hide tooltip 
message: “CTRL” + “F1". However this key combination is reserved by operating 
system on OS X platform. As a results the tests fail.
Test documentation should be updated with some notes regarding this.

Thanks,
Dmitry

Re: [14] Review Request: 7124404 [macosx] no awt.multiClickInterval desktop property

[Dmitry Markov]

Hi Sergey,

The fix looks fine.
Just a minor remark - the function description in LWCToolkit.m says: 
“Signature: ()Z “; but the function returns int and not boolean. I don’t need a 
new webrev for that correction.

Thanks,
Dmitry

> On 29 Sep 2019, at 06:57, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 14.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-7124404
> Fix: http://cr.openjdk.java.net/~serb/7124404/webrev.01
> 
> The desktop property "awt.multiClickInterval" is implemented on macOS
> via [NSEvent doubleClickInterval]:
> https://developer.apple.com/documentation/appkit/nsevent/1528384-doubleclickinterval?language=objc
> 
> The test is reimplemented and open.
> 
> -- 
> Best regards, Sergey.

Re: [14] RFR 8230782: Robot.createScreenCapture() fails if “awt.robot.gtk” is set to false

[Dmitry Markov]

Thank you for the review, Phil!
I am afraid we cannot create a regression test for this since the issue takes 
place only in VNC environment. I have added “noreg-hard” to the bug.

Thanks,
Dmitry

> On 19 Sep 2019, at 17:58, Phil Race  wrote:
> 
> 
> +1 to the fix but any ideas for a regression test.
> There are no related keywords on the bug ..
> 
> -phil.
> 
> 
> On 9/19/19 9:12 AM, Dmitry Markov wrote:
>> Hello,
>> 
>> Could you review a fix for jdk14, please?
>> 
>>  bug: https://bugs.openjdk.java.net/browse/JDK-8230782 
>> <https://bugs.openjdk.java.net/browse/JDK-8230782>
>>  webrev: http://cr.openjdk.java.net/~dmarkov/8230782/webrev.00/ 
>> <http://cr.openjdk.java.net/~dmarkov/8230782/webrev.00/>
>> 
>> Problem description:
>> After integration of JDK-8210776 [1] we started to use malloc() function for 
>> allocation of XColor array in QueryColorMap() (see multiVis.c). However 
>> malloc() does not initialise allocated memory. So if the memory contains 
>> some “garbage” we may get incorrect data inside the array. As a result 
>> Robot.createsScreenCapture() retrieves incorrect colours.
>> 
>> Fix:
>> It is necessary to invoke calloc() for allocation of the array as it used to 
>> be before XWD upgrade
>> 
>> Thanks,
>> Dmitry
>> 
>> [1] - https://bugs.openjdk.java.net/browse/JDK-8210776 
>> <https://bugs.openjdk.java.net/browse/JDK-8210776>

[14] RFR 8230782: Robot.createScreenCapture() fails if “awt.robot.gtk” is set to false

[Dmitry Markov]

Hello,

Could you review a fix for jdk14, please?

 bug: https://bugs.openjdk.java.net/browse/JDK-8230782
 webrev: http://cr.openjdk.java.net/~dmarkov/8230782/webrev.00/

Problem description:
After integration of JDK-8210776 [1] we started to use malloc() function for 
allocation of XColor array in QueryColorMap() (see multiVis.c). However 
malloc() does not initialise allocated memory. So if the memory contains some 
“garbage” we may get incorrect data inside the array. As a result 
Robot.createsScreenCapture() retrieves incorrect colours.

Fix:
It is necessary to invoke calloc() for allocation of the array as it used to be 
before XWD upgrade

Thanks,
Dmitry

[1] - https://bugs.openjdk.java.net/browse/JDK-8210776

Re: [14] RFR JDK-8225805 - Java Access Bridge does not close the logger

[Dmitry Markov]

Hi Pankaj,

The fix looks good to me.

Thanks,
Dmitry

> On 10 Sep 2019, at 11:06, Pankaj Bansal  wrote:
> 
> Hi All,
> 
> Please review the following fix for jdk14.
> 
> Bug: 
> https://bugs.openjdk.java.net/browse/JDK-8225805 
> 
>  
> webrev
> http://cr.openjdk.java.net/~pbansal/8225805/webrev01/ 
> 
>  
>  
> Issue:
> The Windows WindowsAccessBridge Log file is not being closed anywhere. We 
> should always close opened resources.
> It is not causing any issue as all files opened by the program are closed 
> when program exits normally (JAWS in this case). But case be an issue in case 
> the program terminates abnormally. We should explicitly close the log file.
>  
> Fix:
> Closed the WindowsAccessBridge log file in destructor.
>  
> 
> Regards,
> Pankaj Bansal

Re: [14] Review Request: 8229515 [macos] access to window property of NSView on wrong thread

[Dmitry Markov]

Hi Sergey,

The fix looks good to me.

Thanks,
Dmitry

> On 30 Aug 2019, at 21:59, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 14.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8229515
> Fix: http://cr.openjdk.java.net/~serb/8229515/webrev.00
> 
> The access to the "window" property of the NSView was moved to the appkit 
> thread,
> since the Cocoa components should be used on this thread only.
> 
> -- 
> Best regards, Sergey.

Re: [13] Review Request: 8144125 [macOS] java/awt/event/ComponentEvent/MovedResizedTwiceTest/MovedResizedTwiceTest.java failed automatically

[Dmitry Markov]

Hi Sergey,

The fix looks fine. Shall we add ‘headful’ key to the test?

Thanks,
Dmitry

> On 29 Apr 2019, at 11:27, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 13.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8144125
> Fix: http://cr.openjdk.java.net/~serb/8144125/webrev.00
> 
> One of the tests fails on macOS because we send two events for the component 
> is resized/moved.
> When the LWComponentPeer was implemented I was not aware that the Component 
> class can send such events:
> http://hg.openjdk.java.net/jdk/client/file/6b1bac001aaf/src/java.desktop/share/classes/java/awt/Component.java#l2387
> 
> Currently, LWComponentPeer sends these events even if the target Component 
> should not be updated. As a fix I suggest to post these events by the peer 
> only if the target should be updated as well, which will mean that we got 
> this change from some other code, not the component(and the component will 
> not fire these events).
> 
> -- 
> Best regards, Sergey.

Re: [13] Review Request: 7141393 [macosx] CARemoteLayer code refactoring and unit test

[Dmitry Markov]

Hi Sergey,

The changes look good to me.

Thanks,
Dmitry

> On 8 May 2019, at 07:54, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 13.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-7141393
> Fix: http://cr.openjdk.java.net/~serb/7141393/webrev.00
> 
> AWT on macOS contains some code that is not compiled by default (protected 
> with #ifdef REMOTELAYER) and included to the workspace for testing purposes 
> only. It allows developers to test CALayer support in AWT/Java2D without need 
> to build or download Java Plugin.
> 
> The description of this bug suggests to move this code to the separate test 
> suite, but I think we can drop it because cross process rendering is not used 
> anymore.
> 
> -- 
> Best regards, Sergey.

Re: [13] Review Request: 8214046 [macosx] Undecorated Frame does not Iconify when set to

[Dmitry Markov]

Hi Sergey,

The fix looks good to me.

Thanks,
Dmitry

> On 15 Apr 2019, at 05:13, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for JDK 13.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214046
> Fix: http://cr.openjdk.java.net/~serb/8214046/webrev.00
> 
> After we upgrade our compilers and sdk in the JDK 11, the method 
> NSWindow#miniaturize which we use to iconify the frame stopped working.
> Now, this method pays attention to the NSMiniaturizableWindowMask flag, but 
> we set this flag only for decorated windows(this is how we hide the minimize 
> button on the title).
> 
> In the fix, we set this flag for decorated and undecorated windows.
> 
> -- 
> Best regards, Sergey.

Re: [13] Review Request: 8219504 Test for JDK-8211435 can be run on all platforms

[Dmitry Markov]

Hi Sergey,

The fix looks good to me.

Thanks,
Dmitry

> On 7 Mar 2019, at 06:59, Sergey Bylokhov  wrote:
> 
> Thank you for review!
> Looking for the second +1 from someone else.
> 
> On 20/02/2019 22:20, Krishna Addepalli wrote:
>> +1
>> Krishna
>> -Original Message-
>> From: Sergey Bylokhov
>> Sent: Thursday, February 21, 2019 8:01 AM
>> To: awt-dev@openjdk.java.net
>> Subject:  [13] Review Request: 8219504 Test for JDK-8211435 can be 
>> run on all platforms
>> Hello.
>> Please review the fix for jdk 13.
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8219504
>> Fix: http://cr.openjdk.java.net/~serb/8219504/webrev.00
>> Currently the test for JDK-8211435 is executed on macOS only, because of 
>> JDK-8204142[1][2]:
>> But since JDK-8204142 was fixed we can run it on all platforms.
>> [1] https://bugs.openjdk.java.net/browse/JDK-8204142
>> [2] http://mail.openjdk.java.net/pipermail/awt-dev/2018-October/014492.html
> 
> 
> -- 
> Best regards, Sergey.

Re: [13] RFR 8214109: XToolkit is not correctly displayed color on 16-bit high color setting

[Dmitry Markov]

That’s right. I ran corresponding regression tests on Ubuntu and I didn’t see 
any issues.

Thanks,
Dmitry

> On 23 Feb 2019, at 23:43, Philip Race  wrote:
> 
> This doesn't regress anything with the OGL pipeline on Linux, does it ?
> 
> -phil.
> 
> On 2/18/19, 12:23 AM, Dmitry Markov wrote:
>> 
>> Thank you, Sergey!
>> Looking for the second +1 from someone else.
>> 
>> Dmitry
>> 
>>> On 16 Feb 2019, at 02:10, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com>> wrote:
>>> 
>>> Looks fine.
>>> 
>>> On 15/02/2019 04:06, Dmitry Markov wrote:
>>>> Hi Sergey,
>>>> I think we can just replace ColorModel based calculation of the pixel 
>>>> value with SurfaceData.pixelFor(). The usage of ColorModel is intended for 
>>>> old Solaris platforms which are not supported any more. Please find the 
>>>> new version here: http://cr.openjdk.java.net/~dmarkov/8214109/webrev.01/ 
>>>> <http://cr.openjdk.java.net/%7Edmarkov/8214109/webrev.01/>
>>>> Also I ran all regression tests and didn’t observe any new failures.
>>>> Thanks,
>>>> Dmitry
>>>>> On 31 Jan 2019, at 22:25, Sergey Bylokhov >>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>>>> 
>>>>> Hi, Dmitry.
>>>>> On 30/01/2019 05:02, Dmitry Markov wrote:
>>>>> 
>>>>>> I understand your intention to get rid of “the check of the current 2d 
>>>>>> pipeline” but it appears impossible to move the related code to java2d 
>>>>>> in particular OGL.
>>>>> 
>>>>> But it will be good to move it to java2d code, since this is ogl and 
>>>>> solaris specific.(if this is really solaris specific then it looks like a 
>>>>> bug in OGL pipeline)
>>>>> 
>>>>>> Currently OGL uses ArgbPre pixel converter for rendering. Default pixel 
>>>>>> converter is used for calculation of pixel value when background colour 
>>>>>> is set because ArgbPre does not return the correct value for OGL on 
>>>>>> Solaris (according to JDK-6304250).
>>>>>> I do not see any way to distinguish between setting of background colour 
>>>>>> and other rendering operations from java2d code.
>>>>> 
>>>>> It is unclear why it is not possible to get this color since the current 
>>>>> fix has a code to calculate this color.
>>>>> 
>>>>> 
>>>>> -- 
>>>>> Best regards, Sergey.
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
>> 

Re: [13] RFR 8214109: XToolkit is not correctly displayed color on 16-bit high color setting

[Dmitry Markov]

Thank you, Sergey!
Looking for the second +1 from someone else.

Dmitry

> On 16 Feb 2019, at 02:10, Sergey Bylokhov  wrote:
> 
> Looks fine.
> 
> On 15/02/2019 04:06, Dmitry Markov wrote:
>> Hi Sergey,
>> I think we can just replace ColorModel based calculation of the pixel value 
>> with SurfaceData.pixelFor(). The usage of ColorModel is intended for old 
>> Solaris platforms which are not supported any more. Please find the new 
>> version here: http://cr.openjdk.java.net/~dmarkov/8214109/webrev.01/ 
>> <http://cr.openjdk.java.net/~dmarkov/8214109/webrev.01/>
>> Also I ran all regression tests and didn’t observe any new failures.
>> Thanks,
>> Dmitry
>>> On 31 Jan 2019, at 22:25, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>> 
>>> Hi, Dmitry.
>>> On 30/01/2019 05:02, Dmitry Markov wrote:
>>> 
>>>> I understand your intention to get rid of “the check of the current 2d 
>>>> pipeline” but it appears impossible to move the related code to java2d in 
>>>> particular OGL.
>>> 
>>> But it will be good to move it to java2d code, since this is ogl and 
>>> solaris specific.(if this is really solaris specific then it looks like a 
>>> bug in OGL pipeline)
>>> 
>>>> Currently OGL uses ArgbPre pixel converter for rendering. Default pixel 
>>>> converter is used for calculation of pixel value when background colour is 
>>>> set because ArgbPre does not return the correct value for OGL on Solaris 
>>>> (according to JDK-6304250).
>>>> I do not see any way to distinguish between setting of background colour 
>>>> and other rendering operations from java2d code.
>>> 
>>> It is unclear why it is not possible to get this color since the current 
>>> fix has a code to calculate this color.
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [13] RFR 8214109: XToolkit is not correctly displayed color on 16-bit high color setting

[Dmitry Markov]

Hi Sergey,

I think we can just replace ColorModel based calculation of the pixel value 
with SurfaceData.pixelFor(). The usage of ColorModel is intended for old 
Solaris platforms which are not supported any more. Please find the new version 
here: http://cr.openjdk.java.net/~dmarkov/8214109/webrev.01/ 
<http://cr.openjdk.java.net/~dmarkov/8214109/webrev.01/>

Also I ran all regression tests and didn’t observe any new failures.

Thanks,
Dmitry

> On 31 Jan 2019, at 22:25, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> On 30/01/2019 05:02, Dmitry Markov wrote:
> 
>> I understand your intention to get rid of “the check of the current 2d 
>> pipeline” but it appears impossible to move the related code to java2d in 
>> particular OGL.
> 
> But it will be good to move it to java2d code, since this is ogl and solaris 
> specific.(if this is really solaris specific then it looks like a bug in OGL 
> pipeline)
> 
>> Currently OGL uses ArgbPre pixel converter for rendering. Default pixel 
>> converter is used for calculation of pixel value when background colour is 
>> set because ArgbPre does not return the correct value for OGL on Solaris 
>> (according to JDK-6304250).
>> I do not see any way to distinguish between setting of background colour and 
>> other rendering operations from java2d code.
> 
> It is unclear why it is not possible to get this color since the current fix 
> has a code to calculate this color.
> 
> 
> -- 
> Best regards, Sergey.

[13] RFR 8214109: XToolkit is not correctly displayed color on 16-bit high color setting

[Dmitry Markov]

Hello,

Could you review a fix for jdk13, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8214109 

webrev: http://cr.openjdk.java.net/~dmarkov/8214109/webrev.00/ 


Problem description:  
On xvnc4 environment the colour of the background and the colour of the repaint 
area may be different (and it’s visually noticeable) even if the same colour is 
specified. The root cause of this behaviour is that we always use ColorModel to 
calculate a pixel value in XWindow.xSetBackground() and at the same time the 
pixel value for repaint area is calculated using SurfaceData methods, (e.g. 
SurfaceData.pixelFor()). Actually this is regression caused by JDK-6304250 
which introduced ColorModel based calculation of the pixel value to fix problem 
for OpenGL pipeline.   

Fix:  
It is necessary to modify XWindow.xSetBackground(): if OpenGL pipeline is 
enabled we should use ColorModel to calculate the pixel value; otherwise we 
should use SurfaceData.pixelFor() for calculation.

Thanks,
Dmitry 

Re: [13] Review Request: 8215756 Memory leaks in the AWT on macOS

[Dmitry Markov]

Hi Sergey,

Still looks good.

Thanks,
Dmitry

> On 3 Jan 2019, at 01:32, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> Can you please review a simplified version of the fix.
> 
> The "javaPlatformWindow" is declared in AWTWindow.h this way:
>   @property (nonatomic, retain) JNFWeakJObjectWrapper *javaPlatformWindow;
> 
> So it is not necessary to release it manually and just assigned to nil is 
> enough.
> 
> http://cr.openjdk.java.net/~serb/8215756/webrev.01/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m.sdiff.html
> 
> 
> On 21/12/2018 10:33, Dmitry Markov wrote:
>> Hi Sergey,
>> The fix looks good to me.
>> Thanks,
>> Dmitry
>>> On 21 Dec 2018, at 00:20, Sergey Bylokhov  
>>> wrote:
>>> 
>>> Hello.
>>> Please review the fix for jdk 13.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8215756
>>> Webrev: http://cr.openjdk.java.net/~serb/8215756/webrev.00
>>> 
>>> Two memory leaks were fixed:
>>> 
>>> - AWTView.m: we should release the NSTrackingArea, usually we do this
>>>   when we change NSTrackingArea from one to another:
>>>   
>>> http://hg.openjdk.java.net/jdk/jdk/file/3791fee4df3b/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m#l437
>>>   But we also need to do this when the window is deallocated
>>> - AWTWindow.m: We have a JNFWeakJObjectWrapper which holds the reference to 
>>> the java object,
>>>   when the window is deallocated we clear the reference to the java 
>>> object, but
>>>   we also need to release the JNFWeakJObjectWrapper object itself
>>> 
>>> I have checked by the Instruments that the test attached to the bug will 
>>> not produce any other leaks after the fix.
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [13] Review Request: 8215756 Memory leaks in the AWT on macOS

[Dmitry Markov]

Hi Sergey,

The fix looks good to me.

Thanks,
Dmitry

> On 21 Dec 2018, at 00:20, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for jdk 13.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8215756
> Webrev: http://cr.openjdk.java.net/~serb/8215756/webrev.00
> 
> Two memory leaks were fixed:
> 
> - AWTView.m: we should release the NSTrackingArea, usually we do this
>   when we change NSTrackingArea from one to another:
>   
> http://hg.openjdk.java.net/jdk/jdk/file/3791fee4df3b/src/java.desktop/macosx/native/libawt_lwawt/awt/AWTView.m#l437
>   But we also need to do this when the window is deallocated
> - AWTWindow.m: We have a JNFWeakJObjectWrapper which holds the reference to 
> the java object,
>   when the window is deallocated we clear the reference to the java 
> object, but
>   we also need to release the JNFWeakJObjectWrapper object itself
> 
> I have checked by the Instruments that the test attached to the bug will not 
> produce any other leaks after the fix.
> 
> 
> -- 
> Best regards, Sergey.

Re: [12] Review Request: 8215200 IllegalArgumentException in sun.lwawt.macosx.CPlatformWindow

[Dmitry Markov]

Hi Sergey,

The fix looks good.

Thanks,
Dmitry

> On 14 Dec 2018, at 07:32, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for jdk 12.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8215200
> Webrev: http://cr.openjdk.java.net/~serb/8215200/webrev.00
> 
> In the fix for JDK-8190230 we started to sort the list
> of owned windows to maintain their z-order. But there is a small
> issue in the compactor, because of this code:
> 
> ===
>  if (p1 instanceof LWWindowPeer && p2 instanceof LWWindowPeer) {
>.
>  }
> return 0;
> ===
> 
> It means that the window without the peer(p1==null or p2==null)
> is equal to any other windows, and this breaks the timsort.
> 
> After the fix a windows without peers will be equal to each other only, and 
> their
> lastBecomeMainTime timestamp will be zero.
> 
> -- 
> Best regards, Sergey.

[12] RFR 8213583: Error while opening the JFileChooser when desktop contains shortcuts pointing to deleted files

[Dmitry Markov]

Hello,

Could you review a fix for jdk12, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8213583 

webrev: http://cr.openjdk.java.net/~dmarkov/8213583/webrev.00/ 


Problem description:
On Windows when a SecurityManager is defined and JFileChooser is opened for the 
folder which contains a broken shortcut the system popup message is displayed. 
The current implementation of ShellFolder for Windows calls 
IShellLinkW::Resolve without any flags. So the appearance of the dialog with 
system message for the broken shortcut is quite expected, see 
https://docs.microsoft.com/en-us/windows/desktop/api/shobjidl_core/nf-shobjidl_core-ishelllinkw-resolve
 


Fix:
It is necessary to use SLR_NO_UI flag to suppress any system dialogs during 
IShellLinkW::Resolve invocation.

Thanks,
Dmitry 

Re: [12] RFR 8213983: [macosx] Keyboard shortcut “cmd +`” stops working properly if popup window is displayed

[Dmitry Markov]

Hi Sergey,

I am sorry, but the problem is not related to popup windows at all. Actually 
the root cause of the issue is that we perform ordering operation for the 
window which has only invisible child windows. I have updated the fix: 
http://cr.openjdk.java.net/~dmarkov/8213983/webrev.02/ 
<http://cr.openjdk.java.net/~dmarkov/8213983/webrev.02/>
Could you review the new version, please?

Thanks,
Dmitry

> On 21 Nov 2018, at 23:22, Sergey Bylokhov  wrote:
> 
> On 21/11/2018 12:51, Dmitry Markov wrote:
>> When we are going to display the popup, all related windows, (i.e. the 
>> owner, its child windows and their child an so on) are already ordered. They 
>> were ordered once the owner received focus.
> 
> I guess it is always true if the user click on the window
> and popup was shown, but how it will work if the popup window
> was shown programmatically without making window focused?
> 
> 
> -- 
> Best regards, Sergey.

Re: [12] RFR 8213983: [macosx] Keyboard shortcut “cmd +`” stops working properly if popup window is displayed

[Dmitry Markov]

Hi Sergey,

When we are going to display the popup, all related windows, (i.e. the owner, 
its child windows and their child an so on) are already ordered. They were 
ordered once the owner received focus. So there is no reason to order them 
again because we place the popup to separate window level which is located 
above other windows.

Thanks,
Dmitry

> On 21 Nov 2018, at 20:36, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> On 21/11/2018 08:40, Dmitry Markov wrote:
>> On OS X the keyboard shortcut “cmd +`” stops working properly if a popup 
>> window is displayed. That behaviour is caused by window ordering operation. 
>> However it is really unnecessary to perform ordering for popup windows 
>> because they are located at another window level, (i.e. 
>> NSPopUpMenuWindowLevel) and always placed above their parents.
> 
> If ordering of windows is not necessary for popup only, then why we skip 
> ordering for other child windows/and their child and so on?
> 
> -- 
> Best regards, Sergey.

Re: [12] RFR 8213983: [macosx] Keyboard shortcut “cmd +`” stops working properly if popup window is displayed

[Dmitry Markov]

Hi Krishna,

You are right. I have updated the fix: now we skip ordering if one of the child 
windows is popup. Please find new version here: 
http://cr.openjdk.java.net/~dmarkov/8213983/webrev.01/ 
<http://cr.openjdk.java.net/~dmarkov/8213983/webrev.01/>

Thanks,
Dmitry

> On 21 Nov 2018, at 17:43, Krishna Addepalli  
> wrote:
> 
> Hi Dmitry,
> 
> While I understand that you donot want to order the windows when the popup 
> window is present as a child of the root window, I did not understand, why 
> you need to check if the total number of windows should be only one.
> If I understand correctly, if a modal dialog/any other child window has a 
> ComboBox component in it, then PopupMenu will be an additional window, and 
> the length may not necessarily be equal to one.
> 
> Thanks,
> Krishna
> 
>> On 21-Nov-2018, at 10:10 PM, Dmitry Markov > <mailto:dmitry.mar...@oracle.com>> wrote:
>> 
>> Hello,
>> 
>> Could you review a fix for jdk12, please?
>> 
>>  bug: https://bugs.openjdk.java.net/browse/JDK-8213983 
>> <https://bugs.openjdk.java.net/browse/JDK-8213983>
>>  webrev: http://cr.openjdk.java.net/~dmarkov/8213983/webrev.00/ 
>> <http://cr.openjdk.java.net/~dmarkov/8213983/webrev.00/>
>> 
>> On OS X the keyboard shortcut “cmd +`” stops working properly if a popup 
>> window is displayed. That behaviour is caused by window ordering operation. 
>> However it is really unnecessary to perform ordering for popup windows 
>> because they are located at another window level, (i.e. 
>> NSPopUpMenuWindowLevel) and always placed above their parents.
>>  
>> Thanks,
>> Dmitry 
> 

[12] RFR 8213983: [macosx] Keyboard shortcut “cmd +`” stops working properly if popup window is displayed

[Dmitry Markov]

Hello,

Could you review a fix for jdk12, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8213983 

webrev: http://cr.openjdk.java.net/~dmarkov/8213983/webrev.00/ 


On OS X the keyboard shortcut “cmd +`” stops working properly if a popup window 
is displayed. That behaviour is caused by window ordering operation. However it 
is really unnecessary to perform ordering for popup windows because they are 
located at another window level, (i.e. NSPopUpMenuWindowLevel) and always 
placed above their parents.
 
Thanks,
Dmitry 

Re: RFR 8213532: add missing LocalFree calls after using FormatMessage(A) [windows]

[Dmitry Markov]

Hi Matthias,

The fix looks good.

Thanks,
Dmitry

> On 14 Nov 2018, at 07:48, Baesken, Matthias  wrote:
> 
> Hi  Sergey,  thanks for  the review !
> 
> Could I have a second review please,  before I push the change ?
> 
> Best regards, Matthias
> 
> 
>> -Original Message-
>> From: Sergey Bylokhov 
>> Sent: Freitag, 9. November 2018 21:50
>> To: Baesken, Matthias ; awt-
>> d...@openjdk.java.net
>> Subject: Re: RFR 8213532: add missing LocalFree calls after using
>> FormatMessage(A) [windows] - was RE:  FW: RFR [XS] : 8213366:
>> (fs) avoid handle leak in
>> Java_sun_nio_fs_WindowsNativeDispatcher_FindFirstFile0
>> 
>> Looks fine.
>> 
>> On 08/11/2018 08:20, Baesken, Matthias wrote:
>>> Hello, looks like I used the wrong DTRACE_PRINTLN* call  at one place in
>> src/java.desktop/windows/native/libawt/windows/ThemeReader.cpp .
>>> New webrev :
>>> 
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8213532.1/
>>> 
>>> Regards, Matthias
>>> 
>>> 
> 

Re: Swing dev>[12] Review request for JDK-8189253: [macos] JPopupMenu is inadvertently shown when using setComponentPopupMenu

[Dmitry Markov]

Hi Manajit,

The usage of addChildWindow will introduce some undesirable behaviour. For 
example, parent and child windows are created on the same screen; on the build 
with your fix if the parent window is moved the child will be moved too. It is 
better to not use addChildWindow without a good reasons.

Will your fix work if the parent with popup menu and the child are created on 
different screens?

Thanks,
Dmitry

> On 31 Oct 2018, at 16:41, Manajit Halder  wrote:
> 
> Changed the targeted review version to 12.
> 
> Regards,
> Manajit
> On 31/10/18 10:09 PM, Manajit Halder wrote:
>> Hi Dmitry/Sergey,
>> 
>> I have a different fix for this issue in Mac OS specific code. In this 
>> proposed fix I am trying to fix the window addition/ordering with respect to 
>> the screen on which the window is created. As per my debugging this issue 
>> was introduced while fixing JDK-8080729. The problem was introduced due to 
>> the use of orderWindow by replacing it with addChildWindow. The difference 
>> between these two Mac OS methods:
>> 
>> addChildWindow: After the childWin is added as a child of the window, it is 
>> maintained in relative position indicated by place for subsequent ordering 
>> operations involving either window. While this attachment is active, moving 
>> childWin will not cause the window to move (as in sliding a drawer in or 
>> out), but moving the window will cause childWin to move.
>> https://developer.apple.com/documentation/appkit/nswindow/1419152-addchildwindow?language=objc
>>  
>> <https://developer.apple.com/documentation/appkit/nswindow/1419152-addchildwindow?language=objc>
>> orderWindow: Repositions the window’s window device in the window server’s 
>> screen list.
>> https://developer.apple.com/documentation/appkit/nswindow/1419672-orderwindow?language=occ
>>  
>> <https://developer.apple.com/documentation/appkit/nswindow/1419672-orderwindow?language=occ>
>> The main difference which was causing this issue is: addChildWindow attaches 
>> the child window to its parent, but orderWindow re positions it. 
>> 
>> Proposed fix:
>> In my proposed fix I am trying to add/order the window based on which 
>> screen it is created. If the windows (parent and child) are created on same 
>> screen then addChildWindow is used to order child window on top of parent 
>> window. And orderWindow is used for the other case when windows are created 
>> on different screens.
>> 
>> Please review the fix:
>> http://cr.openjdk.java.net/~mhalder/8189253/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Emhalder/8189253/webrev.01/>
>> Regards,
>> Manajit
>> On 03/05/18 12:06 AM, Dmitry Markov wrote:
>>> Hi Manajit,
>>> 
>>> If I got it right, the problem is OSX specific and you suggest fixing it in 
>>> common code. I am sorry but that does not look like as a good approach 
>>> unless it has reasonable explanation.
>>> I recommend that you should move the fix to more suitable place, (i.e. OSX 
>>> specific code). For example, LWWindowPeer.notifyMouseEvent() where we 
>>> receive mouse events from the platform and generate ours mouse enter/exit 
>>> events if necessary. I guess it is possible to implement something similar 
>>> to your current fix in LWWindowPeer.
>>> 
>>> Thanks,
>>> Dmitry 
>>> 
>>>> On 28 Apr 2018, at 00:54, Sergey Bylokhov  
>>>> <mailto:sergey.bylok...@oracle.com> wrote:
>>>> 
>>>> Hi, Manajit.
>>>> Please clarify why this bug is occurred after JDK-8080729, how=20
>>>> orderWindow/addChildWindow are related to the mouse events which we get?=20
>>>> Did we start to get more event or less events or we get events in the=20
>>>> different order, what is the difference?
>>>> 
>>>> On 04/01/2018 10:22, Manajit Halder wrote:
>>>>> Hi Semyon,
>>>>> =20
>>>>> Fix for issue JDK-8080729=20
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8080729> 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8080729> has caused this=20
>>>>> regression due to changes in method setVisible(boolean visible) in file=
>>>> =20
>>>>> CPlatformWindow.java
>>>>> orderWindow is causing the issue here, if we revert to addChildWindow=20
>>>>> then the issue is not observed but then the fix for issue JDK-8080729 f=
>>>> ails.
>>>>> Before this change the child window used to be added on to the parent a=
>>>> 

Re: [12] Review request for JDK-8208543: [macos] Support for apple.awt.documentModalSheet incomplete

[Dmitry Markov]

Hi Sergey,

You are right, the support for “apple.awt.documentModalShee” property is 
incomplete. If the property is set for a window, the window appears as a sheet 
but it is not attached to its owner. I have opened JDK-8213197 [1] for this.

Thanks,
Dmitry

[1] - https://bugs.openjdk.java.net/browse/JDK-8213197

> On 31 Oct 2018, at 01:14, Sergey Bylokhov  wrote:
> 
> Hello,
> Can somebody explain how the dialog which have the "documentModalSheet" 
> property should behave?
> I thought this dialogs should looks like this:
> https://stackoverflow.com/questions/13777067/swing-native-look-and-feel-for-jdialog-in-macos
> 
> But when I run the manual testcase which is attached to the fix, it looks 
> differently. It shows the undecorated dialog in the top/left corner, which is 
> not attached to the window, and which cannot be moved.
> 
> For example in the code below, should the dialog be inside the frame(I guess 
> this can be checked by the automated test)?
> ===>
>Window frame = new JFrame();
>frame.setSize(300,300);
>frame.setLocationRelativeTo(null);
>frame.setVisible(true);
>JDialog dialog =
>new JDialog(frame, null, Dialog.ModalityType.DOCUMENT_MODAL);
>dialog.getRootPane().putClientProperty("apple.awt.documentModalSheet",
>   Boolean.TRUE);
>dialog.add(new JLabel("Hello world!"));
>dialog.pack();
>dialog.setVisible(true);
> ===>
> 
> On 30/10/2018 04:16, Krishna Addepalli wrote:
>> +1
>> Krishna
>> -Original Message-
>> From: Dmitry Markov
>> Sent: Friday, October 26, 2018 4:57 PM
>> To: Manajit Halder 
>> Cc: awt-dev@openjdk.java.net
>> Subject: Re:  [12] Review request for JDK-8208543: [macos] 
>> Support for apple.awt.documentModalSheet incomplete
>> Hi Manajit,
>> Looks good to me.
>> Thanks,
>> Dmitry
>>> On 26 Oct 2018, at 11:02, Manajit Halder  wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> I have corrected the test case, now it fails if timeout takes place. Please 
>>> review the webrev:
>>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.02/ 
>>> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.02/>
>>> 
>>> Regards,
>>> Manajit
>>> 
>>> 
>>> 
>>> On 25/10/18 7:18 PM, Dmitry Markov wrote:
>>>> Hi Manajit,
>>>> 
>>>> Currently the test is marked as ‘passed’ if timeout takes place. I think 
>>>> we should indicate an error or mark it as ‘failed’ in such case.
>>>> 
>>>> Thanks,
>>>> Dmitry
>>>> 
>>>>> On 25 Oct 2018, at 11:35, Manajit Halder  
>>>>> wrote:
>>>>> 
>>>>> Hi Dmitry,
>>>>> 
>>>>> Thanks for your comments. I have addressed all your review comments in 
>>>>> the new webrev.
>>>>> Additional changes:
>>>>> NSDocModalWindowMask is deprecated and hence changed it to 
>>>>> NSWindowStyleMaskDocModalWindow.
>>>>> Window is created a Panel, required for style mask 
>>>>> NSWindowStyleMaskDocModalWindow.
>>>>> Test case was modified to add a case for the failed scenario "Dialog 
>>>>> without owner".
>>>>> 
>>>>> Please review the modified webrev:
>>>>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.01/ 
>>>>> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.01/>
>>>>> 
>>>>> Regards,
>>>>> Manajit
>>>>> 
>>>>> On 13/10/18 12:14 AM, Dmitry Markov wrote:
>>>>>> Hi Manajit,
>>>>>> 
>>>>>> There is an inconsistency between the proposed implementation and Apple 
>>>>>> JDK: if the property applied to the dialog which does not have an owner 
>>>>>> on the build with your changes it appears as sheet, but on Apple JDK it 
>>>>>> appears as a window.
>>>>>> 
>>>>>> I think every frame/dialog inside dispose() method in the regression 
>>>>>> test should be checked for null-value before usage.
>>>>>> 
>>>>>> I noticed that the regression test uses Timer API (see 
>>>>>> createAndShowModalSheet() method). Shall we stop/cancel the timer when 
>>>>>> “Pass”/“Fail” button is press?
>>>>>> 
>&g

Re: [12] Review request for JDK-8208543: [macos] Support for apple.awt.documentModalSheet incomplete

[Dmitry Markov]

Hi Manajit,

Looks good to me.

Thanks,
Dmitry

> On 26 Oct 2018, at 11:02, Manajit Halder  wrote:
> 
> Hi Dmitry,
> 
> I have corrected the test case, now it fails if timeout takes place. Please 
> review the webrev:
> http://cr.openjdk.java.net/~mhalder/8208543/webrev.02/ 
> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.02/>
> 
> Regards,
> Manajit
> 
> 
> 
> On 25/10/18 7:18 PM, Dmitry Markov wrote:
>> Hi Manajit,
>> 
>> Currently the test is marked as ‘passed’ if timeout takes place. I think we 
>> should indicate an error or mark it as ‘failed’ in such case.
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 25 Oct 2018, at 11:35, Manajit Halder  wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> Thanks for your comments. I have addressed all your review comments in the 
>>> new webrev.
>>> Additional changes:
>>> NSDocModalWindowMask is deprecated and hence changed it to 
>>> NSWindowStyleMaskDocModalWindow.
>>> Window is created a Panel, required for style mask 
>>> NSWindowStyleMaskDocModalWindow.
>>> Test case was modified to add a case for the failed scenario "Dialog 
>>> without owner".
>>> 
>>> Please review the modified webrev:
>>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.01/ 
>>> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.01/>
>>> 
>>> Regards,
>>> Manajit
>>> 
>>> On 13/10/18 12:14 AM, Dmitry Markov wrote:
>>>> Hi Manajit,
>>>> 
>>>> There is an inconsistency between the proposed implementation and Apple 
>>>> JDK: if the property applied to the dialog which does not have an owner on 
>>>> the build with your changes it appears as sheet, but on Apple JDK it 
>>>> appears as a window.
>>>> 
>>>> I think every frame/dialog inside dispose() method in the regression test 
>>>> should be checked for null-value before usage.
>>>> 
>>>> I noticed that the regression test uses Timer API (see 
>>>> createAndShowModalSheet() method). Shall we stop/cancel the timer when 
>>>> “Pass”/“Fail” button is press?
>>>> 
>>>> I suppose it is better to declare createAndShowModalSheet() and 
>>>> createAndShowInstructionFrame() as static. In such case the creation of 
>>>> class instance may be omitted.
>>>> 
>>>> Thanks,
>>>> Dmitry
>>>> 
>>>>> On 12 Oct 2018, at 05:36, Manajit Halder  
>>>>> wrote:
>>>>> 
>>>>> Hi Dmitry,
>>>>> 
>>>>> Could you please review this fix related to Modal sheet on Mac OS?
>>>>> 
>>>>> Regards,
>>>>> Manajit
>>>>> 
>>>>> 
>>>>> On 10/10/18 3:33 PM, Manajit Halder wrote:
>>>>>> Hi All,
>>>>>> 
>>>>>> Kindly review the fix for JDK12.
>>>>>> 
>>>>>> Bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8208543
>>>>>> 
>>>>>> 
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.00/ 
>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.00/>
>>>>>> 
>>>>>> Problem:
>>>>>> "apple.awt.documentModalSheet" was getting set on the Dialog while its 
>>>>>> creations, but appearance of Dialog was not changing.
>>>>>> 
>>>>>> Fix:
>>>>>> Setting "apple.awt.documentModalSheet" on Window after its creation.
>>>>>> 
>>>>>> Regards,
>>>>> Manajit
> 

Re: [12] Review request for JDK-8208543: [macos] Support for apple.awt.documentModalSheet incomplete

[Dmitry Markov]

Hi Manajit,

Currently the test is marked as ‘passed’ if timeout takes place. I think we 
should indicate an error or mark it as ‘failed’ in such case.

Thanks,
Dmitry

> On 25 Oct 2018, at 11:35, Manajit Halder  wrote:
> 
> Hi Dmitry,
> 
> Thanks for your comments. I have addressed all your review comments in the 
> new webrev.
> Additional changes:
> NSDocModalWindowMask is deprecated and hence changed it to 
> NSWindowStyleMaskDocModalWindow.
> Window is created a Panel, required for style mask 
> NSWindowStyleMaskDocModalWindow.
> Test case was modified to add a case for the failed scenario "Dialog 
> without owner".
> 
> Please review the modified webrev:
> http://cr.openjdk.java.net/~mhalder/8208543/webrev.01/ 
> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.01/>
> 
> Regards,
> Manajit
> 
> On 13/10/18 12:14 AM, Dmitry Markov wrote:
>> Hi Manajit,
>> 
>> There is an inconsistency between the proposed implementation and Apple JDK: 
>> if the property applied to the dialog which does not have an owner on the 
>> build with your changes it appears as sheet, but on Apple JDK it appears as 
>> a window.
>> 
>> I think every frame/dialog inside dispose() method in the regression test 
>> should be checked for null-value before usage.
>> 
>> I noticed that the regression test uses Timer API (see 
>> createAndShowModalSheet() method). Shall we stop/cancel the timer when 
>> “Pass”/“Fail” button is press?
>> 
>> I suppose it is better to declare createAndShowModalSheet() and 
>> createAndShowInstructionFrame() as static. In such case the creation of 
>> class instance may be omitted.
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 12 Oct 2018, at 05:36, Manajit Halder  wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> Could you please review this fix related to Modal sheet on Mac OS?
>>> 
>>> Regards,
>>> Manajit
>>> 
>>> 
>>> On 10/10/18 3:33 PM, Manajit Halder wrote:
>>>> Hi All,
>>>> 
>>>> Kindly review the fix for JDK12.
>>>> 
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8208543
>>>> 
>>>> 
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.00/ 
>>>> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.00/>
>>>> 
>>>> Problem:
>>>> "apple.awt.documentModalSheet" was getting set on the Dialog while its 
>>>> creations, but appearance of Dialog was not changing.
>>>> 
>>>> Fix:
>>>> Setting "apple.awt.documentModalSheet" on Window after its creation.
>>>> 
>>>> Regards,
>>> Manajit
> 

Re: [12] Review Request: 8211435 Exception in thread "AWT-EventQueue-1" java.lang.IllegalArgumentException: null source

[Dmitry Markov]

Hi Sergey,

The fix looks good to me.
I think you can make the test generic once JDK-8204142 is fixed.

Thanks,
Dmitry

> On 24 Oct 2018, at 22:31, Sergey Bylokhov  wrote:
> 
> Hello.
> Please review the fix for jdk 12.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8211435
> Webrev: http://cr.openjdk.java.net/~serb/8211435/webrev.00
> 
> Bug description:
> 
>  In the DefaultKeyboardFocusManager class we have a special field 
> "activeWindow", which stores the currently active window. It is used in two 
> similar cases:
> 1. If the java window gets "WINDOW_ACTIVATED" event it will try to send 
> "WINDOW_DEACTIVATED" to the old active window, which is stored in the 
> "activeWindow" field.
> 2. If the java component lost the focus, and the opposite component is not a 
> java component, then it will try to send "WINDOW_DEACTIVATED" to the old 
> active window, which is stored in the "activeWindow" field.
> 
> The difference in these two cases is that in "case 1" we check the old active 
> window to null[1], and the second case has no such check. The bug is 
> reproduced in non-standalone mode, when we have a few Appcontexts and this 
> field might be updated by different EDT in parallel.
> 
> Note that the test is for OSX only, because of another bug: JDK-8204142[2]
> 
> 
> [1] 
> http://hg.openjdk.java.net/jdk/jdk/file/ad9077f044be/src/java.desktop/share/classes/java/awt/DefaultKeyboardFocusManager.java#l527
> [2] https://bugs.openjdk.java.net/browse/JDK-8204142
> 
> 
> -- 
> Best regards, Sergey.

Re: [12] Review request for JDK-8208543: [macos] Support for apple.awt.documentModalSheet incomplete

[Dmitry Markov]

Hi Manajit,

There is an inconsistency between the proposed implementation and Apple JDK: if 
the property applied to the dialog which does not have an owner on the build 
with your changes it appears as sheet, but on Apple JDK it appears as a window.

I think every frame/dialog inside dispose() method in the regression test 
should be checked for null-value before usage.

I noticed that the regression test uses Timer API (see 
createAndShowModalSheet() method). Shall we stop/cancel the timer when 
“Pass”/“Fail” button is press?

I suppose it is better to declare createAndShowModalSheet() and 
createAndShowInstructionFrame() as static. In such case the creation of class 
instance may be omitted.

Thanks,
Dmitry

> On 12 Oct 2018, at 05:36, Manajit Halder  wrote:
> 
> Hi Dmitry,
> 
> Could you please review this fix related to Modal sheet on Mac OS?
> 
> Regards,
> Manajit
> 
> 
> On 10/10/18 3:33 PM, Manajit Halder wrote:
>> Hi All,
>> 
>> Kindly review the fix for JDK12.
>> 
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8208543
>> 
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.00/ 
>> 
>> 
>> Problem:
>> "apple.awt.documentModalSheet" was getting set on the Dialog while its 
>> creations, but appearance of Dialog was not changing.
>> 
>> Fix:
>> Setting "apple.awt.documentModalSheet" on Window after its creation.
>> 
>> Regards,
> Manajit
>> 
> 

[8u] RFR: 8210891: Remove unused extutil.h from JDK8u sources

[Dmitry Markov]

Hello,

Could you review the following fix, please?

bug: https://bugs.openjdk.java.net/browse/JDK-8210891 

webrev: http://cr.openjdk.java.net/~dmarkov/8210891/webrev.00/ 


Removed extutil.h and updated TPRMs accordingly.

Thanks in advance,
Dmitry 

Re: RFR: 8210886: Remove references in xwindows.md to non-existent files.

[Dmitry Markov]

Hi Phil,

The changes look good to me.

Thanks,
Dmitry 

> On 26 Sep 2018, at 19:23, Phil Race  wrote:
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8210886
> webrev: http://cr.openjdk.java.net/~prr/8210886/
> 
> the legal file xwindows.md gives attribution for several source files
> from the X11 distribution which in fact are not present anywhere.
> Some really ancient (1.5 ?) version of Oracle JDK had some of them
> and extitil.h was removed during the JDK 9 cycle since it was unused
> but that didn't stop it being listed in the legal file.
> 
> -phil.
> 
> 

Re: [12] Review Request: 8210286 Drop of sun.awt.HToolkit class

[Dmitry Markov]

Looks good to me.

Thanks,
Dmitry

> On 21 Sep 2018, at 22:43, Sergey Bylokhov  wrote:
> 
> Any volunteers for review? =)
> 
> On 03/09/2018 16:36, Sergey Bylokhov wrote:
>> Hello.
>> Please review the fix for jdk 12.
>> This class was used as a headless toolkit for embedded purposes.
>>  - It was added in the JDK-7030063
>>  - Its usage was removed in JDK-8140723 and JDK-8025673.
>> The usage of strings "sun.awt.HToolkit" and "javaplugin.version" in 
>> GraphicsEnvironment were also dropped, because both are never use.
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210286
>> Webrev: http://cr.openjdk.java.net/~serb/8210286/webrev.00
> 
> 
> -- 
> Best regards, Sergey.

Re: RFR: 8210866: Remove HPKeysym.h from JDK sources

[Dmitry Markov]

The changes look good to me.

Thanks,
Dmitry

> On 18 Sep 2018, at 22:49, Phil Race  wrote:
> 
> Thanks,
> 
> However I also need to remove the now obsolete
> comment which explained why we had our own copy ..
> http://cr.openjdk.java.net/~prr/8210866.1
> 
> -phil.
> 
> On 09/18/2018 01:14 PM, Sergey Bylokhov wrote:
>> Looks fine.
>> 
>> On 18/09/2018 12:50, Phil Race wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8210880
>>> Webrev : http://cr.openjdk.java.net/~prr/8210866/
>>> 
>>> As described in the bug, we can rely on the build platform for this file,
>>> as we do for most X11 include files. Builds pass on our core platforms.
>>> Since this is an X11 protocol file, I expect very few people will find they
>>> are missing this and have to install it ..
>>> 
>>> -phil.
>> 
>> 
> 

Re: [12] RFR JDK-8191178:[macos] Problem with input of yen symbol

[Dmitry Markov]

That’s right. The changes look good to me.

Thanks,
Dmitry

> On 17 Sep 2018, at 17:01, Prasanta Sadhukhan  
> wrote:
> 
> Hi Dmitry,
> I guess fullwidth currency symbols are 
> U+FF04 <https://www.fileformat.info/info/unicode/char/ff04/index.htm> 
> FULLWIDTH DOLLAR SIGN
> U+FFE1 <https://www.fileformat.info/info/unicode/char/ffe1/index.htm> 
> FULLWIDTH POUND SIGN
> U+FFE5 <https://www.fileformat.info/info/unicode/char/ffe5/index.htm> 
> FULLWIDTH YEN SIGN
> etc which are already part of this check 
> (codePoint >= 0xFF00) && (codePoint <= 0xFFEF)
> right?
> 
> Regards
> Prasanta
> On 17-Sep-18 9:22 PM, Dmitry Markov wrote:
>> Hi Prasanta,
>> 
>> I think it would be wise to generate InputMethodEvent for ‘Fullwidth 
>> currency symbols’, as well.
>> 
>> Thanks,
>> Dmitry 
>> 
>>> On 17 Sep 2018, at 10:02, Prasanta Sadhukhan >> <mailto:prasanta.sadhuk...@oracle.com>> wrote:
>>> 
>>> Hi All,
>>> 
>>> Please review a fix for an issue where
>>> when "yen" symbol is entered from a keyboard using Romaji keyboard layout 
>>> using "backslash" character, it was showing a "backslash" character 
>>> rather than "yen" symbol.
>>> 
>>> This is a regression of JDK-8068283 
>>> <https://bugs.openjdk.java.net/browse/JDK-8068283> where the check to 
>>> control JNI invocation of 
>>> "sun.lwawt.macosx.CInputMethod.insertText(String)" is changed from 
>>> "if ([self hasMarkedText] || !fProcessingKeystroke || (utf8Length > 1))
>>> to 
>>> if ([self hasMarkedText] || !fProcessingKeystroke || (utf16Length > 2))
>>> 
>>> Now, in this case for "yen" symbol, the utf16Length is 2 so 
>>> InputMethodEvent is not generated, rather a KeyEvent is generated for "\" 
>>> character.
>>> This check was again modified for JDK-8132503 
>>> <https://bugs.openjdk.java.net/browse/JDK-8132503> where the check for 
>>> unichar belongs to certain unicode block is introduced
>>> 
>>> (utf8Length > 1) && [self 
>>> isCodePointInUnicodeBlockNeedingIMEvent:[useString characterAtIndex:0]]
>>> 
>>> Now, there although utf8Length is 2 the check for codepoint is complex or 
>>> not does not take into account "unicode" currency symbols. 
>>> It only takes into account CJK symbols and punctuations and "Halfwidth and 
>>> Fullwidth Forms' Unicode block.
>>> 
>>> Proposed fix also add "currency" symbol unicode 
>>> [[https://www.fileformat.info/info/unicode/category/Sc/list.htm 
>>> <https://www.fileformat.info/info/unicode/category/Sc/list.htm>]
>>> in the mix so that we can have InputmethodEvent generated for currency 
>>> symbols.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8191178 
>>> <https://bugs.openjdk.java.net/browse/JDK-8191178>
>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8191178/webrev.0/ 
>>> <http://cr.openjdk.java.net/%7Epsadhukhan/8191178/webrev.0/>
>>> 
>>> Regards
>>> Prasanta
>> 
> 

Re: [12] RFR JDK-8191178:[macos] Problem with input of yen symbol

[Dmitry Markov]

Hi Prasanta,

I think it would be wise to generate InputMethodEvent for ‘Fullwidth currency 
symbols’, as well.

Thanks,
Dmitry 

> On 17 Sep 2018, at 10:02, Prasanta Sadhukhan  
> wrote:
> 
> Hi All,
> 
> Please review a fix for an issue where
> when "yen" symbol is entered from a keyboard using Romaji keyboard layout 
> using "backslash" character, it was showing a "backslash" character 
> rather than "yen" symbol.
> 
> This is a regression of JDK-8068283 
>  where the check to control 
> JNI invocation of "sun.lwawt.macosx.CInputMethod.insertText(String)" is 
> changed from 
> "if ([self hasMarkedText] || !fProcessingKeystroke || (utf8Length > 1))
> to 
> if ([self hasMarkedText] || !fProcessingKeystroke || (utf16Length > 2))
> 
> Now, in this case for "yen" symbol, the utf16Length is 2 so InputMethodEvent 
> is not generated, rather a KeyEvent is generated for "\" character.
> This check was again modified for JDK-8132503 
>  where the check for 
> unichar belongs to certain unicode block is introduced
> 
> (utf8Length > 1) && [self isCodePointInUnicodeBlockNeedingIMEvent:[useString 
> characterAtIndex:0]]
> 
> Now, there although utf8Length is 2 the check for codepoint is complex or not 
> does not take into account "unicode" currency symbols. 
> It only takes into account CJK symbols and punctuations and "Halfwidth and 
> Fullwidth Forms' Unicode block.
> 
> Proposed fix also add "currency" symbol unicode 
> [[https://www.fileformat.info/info/unicode/category/Sc/list.htm 
> ]
> in the mix so that we can have InputmethodEvent generated for currency 
> symbols.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8191178 
> 
> webrev: http://cr.openjdk.java.net/~psadhukhan/8191178/webrev.0/ 
> 
> 
> Regards
> Prasanta

Re: [12] Review request for JDK-8206392: [macosx] Cycling through windows (JFrames) does not work with keyboard shortcut

[Dmitry Markov]

Hi Manajit,

No problem, I will sponsor your commit to JDK 8u once it is ready and approved.

Thanks,
Dmitry 

> On 10 Sep 2018, at 13:19, Manajit Halder  wrote:
> 
> Hi Dmitry,
> 
> Sure, I can port it to JDK 8u, but would require your help in check-in, as I 
> don't have permission to check-in JDK 8u.
> Thanks,
> 
> Manajit
> 
> 
> On 09/09/18 3:35 PM, Dmitry Markov wrote:
>> Hi Manajit,
>> 
>> The fix looks good to me. Can you port it to JDK 8u too, please?
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 7 Sep 2018, at 12:22, Manajit Halder >> <mailto:manajit.hal...@oracle.com>> wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> Thanks for the test scenarios. I have run all the AWT, Swing and JCK 
>>> automatic (open and closed) test cases along with manual test cases related 
>>> to Modal Dialog and ordering to ensure that fix doesn't cause any 
>>> regression. These JCK interactive test were run 
>>> "api/java_awt/interactive/ModalDialogTests.html", 
>>> "api/java_awt/interactive/FileDialogTests.html", 
>>> "api/java_awt/interactive/PageDialogTest.html" and 
>>> "api/java_awt/interactive/WindowTest.html" (tests toFront and toBack 
>>> between a window and a Frame). 
>>> For example, manual test 
>>> "open/test/jdk/java/awt/Modal/WsDisabledStyle/CloseBlocker/CloseBlocker.java"
>>>  tests the scenario specified by you. Also there are many automatic test 
>>> cases in "open/test/jdk/java/awt/Modal/ToBack" and 
>>> "open/test/jdk/java/awt/Modal/ToFront" which tests different scenarios 
>>> related to Modal Dialog and Frame/Window ordering.
>>> 
>>> Please let me know if you have any other test scenarios to be tested.
>>> 
>>> Thanks,
>>> Manajit
>>> On 05/09/18 4:47 PM, Dmitry Markov wrote:
>>>> Hi Manajit,
>>>> 
>>>> Thank you for the clarification.
>>>> 
>>>> Actually I worry about the case when a window is blocked by another window 
>>>> or a modal dialog. We call orderAboveSiblings() for the blocker
>>>>  window in several places, (e.g. inside setVisible(), 
>>>> checkBlockingAndOrder(), etc.) and I guess some of these call might fail 
>>>> with the new implementation of orderAboveSiblings(), (i.e. the modal 
>>>> dialog might be placed behind the blocked window).
>>>> 
>>>> For example, let’s say we have the window which is blocked by the modal 
>>>> dialog. What will be the position of the dialog (above or behind the 
>>>> blocked window) once the window is activated? Please note: there are 
>>>> several ways to activate the window: mouse click, toFront() call, etc. Can 
>>>> you verify that these scenarios still work properly on the build with your 
>>>> fix, please?
>>>> 
>>>> Also it would be good to execute related AWT/Swing jtreg tests to ensure 
>>>> that nothing is broken.
>>>> 
>>>> Thanks,
>>>> Dmitry 
>>>> 
>>>>> On 4 Sep 2018, at 18:54, Manajit Halder >>>> <mailto:manajit.hal...@oracle.com>> wrote:
>>>>> 
>>>>> Hi Dmitry,
>>>>> 
>>>>> Thanks for your reply. Please see my reply inline.
>>>>> 
>>>>> Thanks,
>>>>> Manajit
>>>>> 
>>>>> On 01/09/18 12:02 AM, Dmitry Markov wrote:
>>>>>> Hi Manajit,
>>>>>> 
>>>>>> The current implementation assumes that orderAboveSiblings() places the 
>>>>>> window in front of other windows located at the same level. The proposed 
>>>>>> fix introduces new behaviour: if the window does not have an owner and 
>>>>>> child windows it won’t be ordered, (i.e. in certain conditions the 
>>>>>> window may be obscured by other windows even after orderAboveSibling() 
>>>>>> execution). Most likely such approach will break existed functionality - 
>>>>>> some windows will be incorrectly placed behind other windows.
>>>>> If I am not wrong windows (other than Window.Type.POPUP) are already 
>>>>> ordered in setVisible method at line no 632 while creation. While 
>>>>> debugging I observed that orderFront is called twice when the window is 
>>>>> displayed for the first time (in method setVisible and in method 
>>>&

Re: [12] Review request for JDK-8206392: [macosx] Cycling through windows (JFrames) does not work with keyboard shortcut

[Dmitry Markov]

Hi Manajit,

The fix looks good to me. Can you port it to JDK 8u too, please?

Thanks,
Dmitry

> On 7 Sep 2018, at 12:22, Manajit Halder  wrote:
> 
> Hi Dmitry,
> 
> Thanks for the test scenarios. I have run all the AWT, Swing and JCK 
> automatic (open and closed) test cases along with manual test cases related 
> to Modal Dialog and ordering to ensure that fix doesn't cause any regression. 
> These JCK interactive test were run 
> "api/java_awt/interactive/ModalDialogTests.html", 
> "api/java_awt/interactive/FileDialogTests.html", 
> "api/java_awt/interactive/PageDialogTest.html" and 
> "api/java_awt/interactive/WindowTest.html" (tests toFront and toBack between 
> a window and a Frame). 
> For example, manual test 
> "open/test/jdk/java/awt/Modal/WsDisabledStyle/CloseBlocker/CloseBlocker.java" 
> tests the scenario specified by you. Also there are many automatic test cases 
> in "open/test/jdk/java/awt/Modal/ToBack" and 
> "open/test/jdk/java/awt/Modal/ToFront" which tests different scenarios 
> related to Modal Dialog and Frame/Window ordering.
> 
> Please let me know if you have any other test scenarios to be tested.
> 
> Thanks,
> Manajit
> On 05/09/18 4:47 PM, Dmitry Markov wrote:
>> Hi Manajit,
>> 
>> Thank you for the clarification.
>> 
>> Actually I worry about the case when a window is blocked by another window 
>> or a modal dialog. We call orderAboveSiblings() for the blocker window in 
>> several places, (e.g. inside setVisible(), checkBlockingAndOrder(), etc.) 
>> and I guess some of these call might fail with the new implementation of 
>> orderAboveSiblings(), (i.e. the modal dialog might be placed behind the 
>> blocked window).
>> 
>> For example, let’s say we have the window which is blocked by the modal 
>> dialog. What will be the position of the dialog (above or behind the blocked 
>> window) once the window is activated? Please note: there are several ways to 
>> activate the window: mouse click, toFront() call, etc. Can you verify that 
>> these scenarios still work properly on the build with your fix, please?
>> 
>> Also it would be good to execute related AWT/Swing jtreg tests to ensure 
>> that nothing is broken.
>> 
>> Thanks,
>> Dmitry 
>> 
>>> On 4 Sep 2018, at 18:54, Manajit Halder >> <mailto:manajit.hal...@oracle.com>> wrote:
>>> 
>>> Hi Dmitry,
>>> 
>>> Thanks for your reply. Please see my reply inline.
>>> 
>>> Thanks,
>>> Manajit
>>> 
>>> On 01/09/18 12:02 AM, Dmitry Markov wrote:
>>>> Hi Manajit,
>>>> 
>>>> The current implementation assumes that orderAboveSiblings() places the 
>>>> window in front of other windows located at the same level. The proposed 
>>>> fix introduces new behaviour: if the window does not have an owner and 
>>>> child windows it won’t be ordered, (i.e. in certain conditions the window 
>>>> may be obscured by other windows even after orderAboveSibling() 
>>>> execution). Most likely such approach will break existed functionality - 
>>>> some windows will be incorrectly placed behind other windows.
>>> If I am not wrong windows (other than Window.Type.POPUP) are already 
>>> ordered in setVisible method at line no 632 while creation. While debugging 
>>> I observed that orderFront is called twice when the window is displayed for 
>>> the first time (in method setVisible and in method orderAboveSiblings). Now 
>>> when Key press COMMAND + ` is pressed the current window receives 
>>> windowDidBecomeMain notification and which calls orderFront and also 
>>> COMMAND + ` tries to order the window above. Two time ordering the window 
>>> when COMMAND + ` is pressed is causing problem in case of multiple windows.
>>>> 
>>>> I am sorry, but the relationship between the original problem described in 
>>>> the bug, (i.e. cycling through windows issue) and implementation of
>>>>  orderAboveSiblings() is NOT clear. Can you explain this?
>>> This issue is a regression of JDK-8169589 introduced in JDK release 
>>> 8u131. 8169589 introduced new window ordering model and which has 
>>> introduced the cycling through window issue.
>>>> 
>>>> Thanks,
>>>> Dmitry  
>>>> 
>>>>> On 31 Aug 2018, at 07:55, Manajit Halder >>>> <mailto:manajit.hal...@oracle.com>> wrote:
>>>>> 
>>>>> Hi All,
>>>>> 
>>>>> Please review the fix for JDK12.
>>>>> 
>>>>> 
>>>>> 
>>>>> Bug: 
>>>>>  https://bugs.openjdk.java.net/browse/JDK-8206392 
>>>>> <https://bugs.openjdk.java.net/browse/JDK-8206392>
>>>>>  <https://bugs.openjdk.java.net/browse/JDK-8206392>
>>>>> Webrev: 
>>>>> 
>>>>> http://cr.openjdk.java.net/~mhalder/8206392/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/%7Emhalder/8206392/webrev.00/>
>>>>> Fix: 
>>>>> 
>>>>> Window ordering is not required if the window doesn't own any other 
>>>>> windows.
>>>>> 
>>>>> Regards,
>>>>> Manajit
>>>> 
>>> 
>> 
> 

Re: [12] Review request for JDK-8206392: [macosx] Cycling through windows (JFrames) does not work with keyboard shortcut

[Dmitry Markov]

Hi Manajit,

Thank you for the clarification.

Actually I worry about the case when a window is blocked by another window or a 
modal dialog. We call orderAboveSiblings() for the blocker window in several 
places, (e.g. inside setVisible(), checkBlockingAndOrder(), etc.) and I guess 
some of these call might fail with the new implementation of 
orderAboveSiblings(), (i.e. the modal dialog might be placed behind the blocked 
window).

For example, let’s say we have the window which is blocked by the modal dialog. 
What will be the position of the dialog (above or behind the blocked window) 
once the window is activated? Please note: there are several ways to activate 
the window: mouse click, toFront() call, etc. Can you verify that these 
scenarios still work properly on the build with your fix, please?

Also it would be good to execute related AWT/Swing jtreg tests to ensure that 
nothing is broken.

Thanks,
Dmitry 

> On 4 Sep 2018, at 18:54, Manajit Halder  wrote:
> 
> Hi Dmitry,
> 
> Thanks for your reply. Please see my reply inline.
> 
> Thanks,
> Manajit
> 
> On 01/09/18 12:02 AM, Dmitry Markov wrote:
>> Hi Manajit,
>> 
>> The current implementation assumes that orderAboveSiblings() places the 
>> window in front of other windows located at the same level. The proposed fix 
>> introduces new behaviour: if the window does not have an owner and child 
>> windows it won’t be ordered, (i.e. in certain conditions the window may be 
>> obscured by other windows even after orderAboveSibling() execution). Most 
>> likely such approach will break existed functionality - some windows will be 
>> incorrectly placed behind other windows.
> If I am not wrong windows (other than Window.Type.POPUP) are already 
> ordered in setVisible method at line no 632 while creation. While debugging I 
> observed that orderFront is called twice when the window is displayed for the 
> first time (in method setVisible and in method orderAboveSiblings). Now when 
> Key press COMMAND + ` is pressed the current window receives 
> windowDidBecomeMain notification and which calls orderFront and also COMMAND 
> + ` tries to order the window above. Two time ordering the window when 
> COMMAND + ` is pressed is causing problem in case of multiple windows.
>> 
>> I am sorry, but the relationship between the original problem described in 
>> the bug, (i.e. cycling through windows issue) and implementation of 
>> orderAboveSiblings() is NOT clear. Can you explain this?
> This issue is a regression of JDK-8169589 introduced in JDK release 
> 8u131. 8169589 introduced new window ordering model and which has introduced 
> the cycling through window issue.
>> 
>> Thanks,
>> Dmitry  
>> 
>>> On 31 Aug 2018, at 07:55, Manajit Halder >> <mailto:manajit.hal...@oracle.com>> wrote:
>>> 
>>> Hi All,
>>> 
>>> Please review the fix for JDK12.
>>> 
>>> 
>>> 
>>> Bug: 
>>>  https://bugs.openjdk.java.net/browse/JDK-8206392 
>>> <https://bugs.openjdk.java.net/browse/JDK-8206392>
>>>  <https://bugs.openjdk.java.net/browse/JDK-8206392>
>>> Webrev: 
>>> 
>>> http://cr.openjdk.java.net/~mhalder/8206392/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Emhalder/8206392/webrev.00/>
>>> Fix: 
>>> 
>>> Window ordering is not required if the window doesn't own any other 
>>> windows.
>>> 
>>> Regards,
>>> Manajit
>> 
> 

Re: [11] RFR 8205479: OS X: requestFocus() does not work properly for embedded frame

[Dmitry Markov]

Hi Alexey,

Thank you for the review. Unfortunately it is impossible to create the 
automated standalone test for this issue.

Thanks,
Dmitry 

> On 15 Aug 2018, at 19:46, Alexey Ivanov  wrote:
> 
> Hi Dmitry,
> 
> The fix looks good.
> 
> Can an automated test be written for this issue?
> 
> Regards,
> Alexey
> 
> On 06/08/2018 15:46, Dmitry Markov wrote:
>> Thank you, Sergey!
>> 
>> Looking for the second +1 from someone else.
>> 
>> Thanks,
>> Dmitry
>> 
>>> On 5 Aug 2018, at 01:15, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com>> wrote:
>>> 
>>> Looks fine.
>>> 
>>> On 01/07/2018 02:26, Dmitry Markov wrote:
>>>> Hi Sergey,
>>>> The changes in CPlatformEmbeddedFrame are intended for handling the case 
>>>> when the embedder window contains several embedded frames and focus is 
>>>> transferred programmatically between the frames. In particular if 
>>>> requestFocus() is invoked for some component located at inactive frame it 
>>>> is necessary to activate the frame via 
>>>> CPlatformEmbeddedFrame.requestWindowFocus() to make such focus transition 
>>>> possible.
>>>> I am sorry, I do not have information about whether the situation 
>>>> described above is applicable for SWT or not. Anyway it is out of scope 
>>>> for this review.
>>>> Thanks,
>>>> Dmitry
>>>>> On 1 Jul 2018, at 00:21, Sergey Bylokhov >>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>>>> 
>>>>> Hi, Dmitry
>>>>> Can you please clarify why the changes in CPlatformEmbeddedFrame are 
>>>>> necessary? Why the same code does not exists in 
>>>>> CViewPlatformEmbeddedFrame?
>>>>> 
>>>>> On 27/06/2018 10:25, Dmitry Markov wrote:
>>>>>> Hi Sergey,
>>>>>> You are right, it is better to use synthesizeWindowActivation(). Please 
>>>>>> find the updated webrew 
>>>>>> here:http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/ 
>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.01/> 
>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.01/ 
>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.01/>>
>>>>>> Changes:
>>>>>>  - Overrode synthesizeWindowActivation() for CEmbeddedFrame. It calls 
>>>>>> handleWindowFocusEvent() which actually activates or deactivates 
>>>>>> embedded frame.
>>>>>>  - Added updateGlobalFocusedWindow() to CEmbeddedFrame. This method 
>>>>>> should be called when the focus is transferred to embedded frame 
>>>>>> programmatically since handleWindowFocusEvent() skips activation for 
>>>>>> non-focused embedded frames.
>>>>>> Thanks,
>>>>>> Dmitry
>>>>>>> On 27 Jun 2018, at 01:20, Sergey Bylokhov >>>>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>>>>>> <mailto:sergey.bylok...@oracle.com>><mailto:sergey.bylok...@oracle.com 
>>>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>>>>>> 
>>>>>>> Hi, Dmitry.
>>>>>>> Can you please confirm that we should not implement 
>>>>>>> synthesizeWindowActivation() to achieve this behavior?
>>>>>>> I guess we should do the same as in CViewEmbeddedFrame which is used by 
>>>>>>> SWT.
>>>>>>> 
>>>>>>> On 25/06/2018 09:11, Dmitry Markov wrote:
>>>>>>>> Hello,
>>>>>>>> Could you review a fix for jdk11, please?
>>>>>>>>  bug: https://bugs.openjdk.java.net/browse/JDK-8205479 
>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8205479>
>>>>>>>>  webrev: http://cr.openjdk.java.net/~dmarkov/8205479/webrev.00/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.00/> 
>>>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.00/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.00/>>
>>>>>>>> Problem description:
>>>>>>>> On Mac OSX when focus is transferred to some component located at 
>>>>>>>> embedded frame, CPlatformEmbeddedFrame.requestWindowFocus() is called 
>>>>>>>> to activate owning frame. However that method does nothing, (i.e. no 
>>>>>>>> activation happens). As a result the focus cannot be transferred to 
>>>>>>>> the component because its owner is not active.
>>>>>>>> Fix:
>>>>>>>> CPlatformEmbeddedFrame.requestWindowFocus() should activate the 
>>>>>>>> embedded frame, (i.e. invoke notifyActivation() for the corresponding 
>>>>>>>> peer).
>>>>>>>> Thanks,
>>>>>>>> Dmitry
>>>>>>> 
>>>>>>> 
>>>>>>> --
>>>>>>> Best regards, Sergey.
>>>>> 
>>>>> 
>>>>> --
>>>>> Best regards, Sergey.
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
>> 
> 

Re: [11] RFR 8205479: OS X: requestFocus() does not work properly for embedded frame

[Dmitry Markov]

Thank you, Sergey!

Looking for the second +1 from someone else.

Thanks,
Dmitry

> On 5 Aug 2018, at 01:15, Sergey Bylokhov  wrote:
> 
> Looks fine.
> 
> On 01/07/2018 02:26, Dmitry Markov wrote:
>> Hi Sergey,
>> The changes in CPlatformEmbeddedFrame are intended for handling the case 
>> when the embedder window contains several embedded frames and focus is 
>> transferred programmatically between the frames. In particular if 
>> requestFocus() is invoked for some component located at inactive frame it is 
>> necessary to activate the frame via 
>> CPlatformEmbeddedFrame.requestWindowFocus() to make such focus transition 
>> possible.
>> I am sorry, I do not have information about whether the situation described 
>> above is applicable for SWT or not. Anyway it is out of scope for this 
>> review.
>> Thanks,
>> Dmitry
>>> On 1 Jul 2018, at 00:21, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>> 
>>> Hi, Dmitry
>>> Can you please clarify why the changes in CPlatformEmbeddedFrame are 
>>> necessary? Why the same code does not exists in CViewPlatformEmbeddedFrame?
>>> 
>>> On 27/06/2018 10:25, Dmitry Markov wrote:
>>>> Hi Sergey,
>>>> You are right, it is better to use synthesizeWindowActivation(). Please 
>>>> find the updated webrew 
>>>> here:http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/ 
>>>> <http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/> 
>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.01/ 
>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.01/>>
>>>> Changes:
>>>>  - Overrode synthesizeWindowActivation() for CEmbeddedFrame. It calls 
>>>> handleWindowFocusEvent() which actually activates or deactivates embedded 
>>>> frame.
>>>>  - Added updateGlobalFocusedWindow() to CEmbeddedFrame. This method should 
>>>> be called when the focus is transferred to embedded frame programmatically 
>>>> since handleWindowFocusEvent() skips activation for non-focused embedded 
>>>> frames.
>>>> Thanks,
>>>> Dmitry
>>>>> On 27 Jun 2018, at 01:20, Sergey Bylokhov >>>> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>>>> <mailto:sergey.bylok...@oracle.com>><mailto:sergey.bylok...@oracle.com 
>>>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>>>> 
>>>>> Hi, Dmitry.
>>>>> Can you please confirm that we should not implement 
>>>>> synthesizeWindowActivation() to achieve this behavior?
>>>>> I guess we should do the same as in CViewEmbeddedFrame which is used by 
>>>>> SWT.
>>>>> 
>>>>> On 25/06/2018 09:11, Dmitry Markov wrote:
>>>>>> Hello,
>>>>>> Could you review a fix for jdk11, please?
>>>>>>  bug: https://bugs.openjdk.java.net/browse/JDK-8205479 
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8205479>
>>>>>>  webrev: http://cr.openjdk.java.net/~dmarkov/8205479/webrev.00/ 
>>>>>> <http://cr.openjdk.java.net/~dmarkov/8205479/webrev.00/> 
>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.00/ 
>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.00/>>
>>>>>> Problem description:
>>>>>> On Mac OSX when focus is transferred to some component located at 
>>>>>> embedded frame, CPlatformEmbeddedFrame.requestWindowFocus() is called to 
>>>>>> activate owning frame. However that method does nothing, (i.e. no 
>>>>>> activation happens). As a result the focus cannot be transferred to the 
>>>>>> component because its owner is not active.
>>>>>> Fix:
>>>>>> CPlatformEmbeddedFrame.requestWindowFocus() should activate the embedded 
>>>>>> frame, (i.e. invoke notifyActivation() for the corresponding peer).
>>>>>> Thanks,
>>>>>> Dmitry
>>>>> 
>>>>> 
>>>>> --
>>>>> Best regards, Sergey.
>>> 
>>> 
>>> --
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [11] RFR 8130655: OS X: keyboard input in textfield is not possible if the window contained textfield is owned by EmbeddedFrame

[Dmitry Markov]

Hi Phil,

You are right, these changes are mainly intended for JDK 8u due to applet 
support presence. However they might be helpful for other EmbeddedFrame users. 
That’s why I would like to integrate the changes into jdk-client too.

I am not going to integrate the fix into JDK 11 GA, I will push it to JDK 12 
instead.

Thanks,
Dmitry 

> On 5 Aug 2018, at 02:37, Philip Race  wrote:
> 
> This is a P3. It is now too late for 11 even if it was fine when the thread 
> began.
> Fixes for 11 must be P2 or higher and you will need to personally
> run all jtreg + JCK tests as a minimum level of testing.
> 
> And CPlatformWindow fixes have not been very successful lately !
> 
> So you probably need to wait until the current TCK issue there is resolved 
> before
> you can begin.
> 
> I'm also curious if this even matters for 11 which has no applet support - 
> although
> I do recognise EmbeddedFrame is used in other scenarios too.
> 
> But if this is just to enable a backport, then perhaps rushing it into 11 GA 
> is not needed.
> 
> -phil.
> 
> On 8/4/18, 5:13 PM, Sergey Bylokhov wrote:
>> Looks fine.
>> 
>> On 30/06/2018 08:12, Dmitry Markov wrote:
>>> Hi Sergey,
>>> 
>>>> On 27 Jun 2018, at 01:15, Sergey Bylokhov >>> <mailto:sergey.bylok...@oracle.com>> wrote:
>>>> 
>>>> Hi, Dmitry.
>>>> Can you please provide some more details why this bug is reproducible only 
>>>> on mac and not on the other platforms(I meant the bug related to the popup 
>>>> menu not SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits)
>>>> 
>>> 
>>> The implementation of EmbeddedFrame on Mac OSX is lightweight and it is 
>>> heavyweight on Window/Linux. In other words on Windows/Linux an embedded 
>>> frame is able to receive events from the platform by itself and transfer 
>>> them to its child windows if any. However it does not work for Mac OSX: 
>>> CEmbeddedFrame receives events only from the embedder, (e.g. browser) which 
>>> is not aware of any windows owned by the embedded frame. So on Mac OSX if 
>>> the embedded frame owns a simple window (which is unfocusable by default) 
>>> there is no way to provide the simple window with keyboard input. At the 
>>> same time on other platforms the simple window receives key events in the 
>>> same scenario.
>>> 
>>> The test applet attached to the bug demonstrates the problem, (i.e. simple 
>>> window owned by embedded frame is unable to receive keyboard input) on Mac 
>>> and its absence on other platform. Currently I have only this test which 
>>> works on all platforms without any modifications.
>>> 
>>>> I have checked our current behavior on win/lin/mac in all cases the simple 
>>>> "Window" cannot get a focus, without any difference which parent is 
>>>> used(null/frame/window).
>>>> 
>>>> So for example if the window1 is owned by another window2, then both of 
>>>> them are "unfocusable", but after the current fix, both will be 
>>>> "focusable" if the window2 will be owned by the embedded frame.
>>> 
>>> I agree it was not good idea to make simple window focusable even if it was 
>>> owned by embedded frame. I think it will be better to allow simple window 
>>> receive keyboard input if it is owned by embedded frame. In other words 
>>> simple window will stay unfocusable, (i.e. SHOUL_BECOME_MAIN and 
>>> SHOULD_BECOME_KEY bits are unset) but it will be able to receive key events 
>>> when necessary. Please find the implementation of this approach here: 
>>> http://cr.openjdk.java.net/~dmarkov/8130655/webrev.01/ 
>>> <http://cr.openjdk.java.net/%7Edmarkov/8130655/webrev.01/>
>>> 
>>> Changes:
>>>  - Added methods CPlatformWindow and AWTWindow to check whether the window 
>>> is simple and owned by embedded frame
>>>  - Modified canBecomeKeyWindow() in AWTWindow to take into account window 
>>> type and owner
>>> 
>>> Could you review the new version, please?
>>> 
>>> Thanks,
>>> Dmitry
>>>> 
>>>> On 25/06/2018 03:35, Dmitry Markov wrote:
>>>>> Hello,
>>>>> Could you review a fix for jdk11, please?
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8130655
>>>>> webrev: http://cr.openjdk.java.net/~dmarkov/8130655/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8130655/webrev.00/>
>>>>> Problem description:
>>>>> On OS X platform a window does not receive keyboard input if it is owned 
>>>>> by embedded frame. According to the current implementation “simple 
>>>>> window” (not frame or dialog) is NOT natively focusable, (i.e. 
>>>>> SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits are not set for its native 
>>>>> peer); embedded frame receives events from the embedder, (e.g. browser, 
>>>>> etc.) but does not translate them to the its own child windows. So if 
>>>>> “simple window” is owned by embedded frame it is impossible for the 
>>>>> window to receive any key events.
>>>>> Fix:
>>>>> Set SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits for simple window which 
>>>>> is owned by embedded frame.
>>>>> Thanks,
>>>>> Dmitry
>>>> 
>>>> 
>>>> -- 
>>>> Best regards, Sergey.
>>> 
>> 
>> 

Re: [11] RFR 8130655: OS X: keyboard input in textfield is not possible if the window contained textfield is owned by EmbeddedFrame

[Dmitry Markov]

Thank you, Sergey!

Looking for the second +1 from someone else.

Thanks,
Dmitry

> On 5 Aug 2018, at 01:13, Sergey Bylokhov  wrote:
> 
> Looks fine.
> 
> On 30/06/2018 08:12, Dmitry Markov wrote:
>> Hi Sergey,
>>> On 27 Jun 2018, at 01:15, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com>> wrote:
>>> 
>>> Hi, Dmitry.
>>> Can you please provide some more details why this bug is reproducible only 
>>> on mac and not on the other platforms(I meant the bug related to the popup 
>>> menu not SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits)
>>> 
>> The implementation of EmbeddedFrame on Mac OSX is lightweight and it is 
>> heavyweight on Window/Linux. In other words on Windows/Linux an embedded 
>> frame is able to receive events from the platform by itself and transfer 
>> them to its child windows if any. However it does not work for Mac OSX: 
>> CEmbeddedFrame receives events only from the embedder, (e.g. browser) which 
>> is not aware of any windows owned by the embedded frame. So on Mac OSX if 
>> the embedded frame owns a simple window (which is unfocusable by default) 
>> there is no way to provide the simple window with keyboard input. At the 
>> same time on other platforms the simple window receives key events in the 
>> same scenario.
>> The test applet attached to the bug demonstrates the problem, (i.e. simple 
>> window owned by embedded frame is unable to receive keyboard input) on Mac 
>> and its absence on other platform. Currently I have only this test which 
>> works on all platforms without any modifications.
>>> I have checked our current behavior on win/lin/mac in all cases the simple 
>>> "Window" cannot get a focus, without any difference which parent is 
>>> used(null/frame/window).
>>> 
>>> So for example if the window1 is owned by another window2, then both of 
>>> them are "unfocusable", but after the current fix, both will be "focusable" 
>>> if the window2 will be owned by the embedded frame.
>> I agree it was not good idea to make simple window focusable even if it was 
>> owned by embedded frame. I think it will be better to allow simple window 
>> receive keyboard input if it is owned by embedded frame. In other words 
>> simple window will stay unfocusable, (i.e. SHOUL_BECOME_MAIN and 
>> SHOULD_BECOME_KEY bits are unset) but it will be able to receive key events 
>> when necessary. Please find the implementation of this approach here: 
>> http://cr.openjdk.java.net/~dmarkov/8130655/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Edmarkov/8130655/webrev.01/>
>> Changes:
>>  - Added methods CPlatformWindow and AWTWindow to check whether the window 
>> is simple and owned by embedded frame
>>  - Modified canBecomeKeyWindow() in AWTWindow to take into account window 
>> type and owner
>> Could you review the new version, please?
>> Thanks,
>> Dmitry
>>> 
>>> On 25/06/2018 03:35, Dmitry Markov wrote:
>>>> Hello,
>>>> Could you review a fix for jdk11, please?
>>>>  bug: https://bugs.openjdk.java.net/browse/JDK-8130655
>>>>  webrev: http://cr.openjdk.java.net/~dmarkov/8130655/webrev.00/ 
>>>> <http://cr.openjdk.java.net/%7Edmarkov/8130655/webrev.00/>
>>>> Problem description:
>>>> On OS X platform a window does not receive keyboard input if it is owned 
>>>> by embedded frame. According to the current implementation “simple window” 
>>>> (not frame or dialog) is NOT natively focusable, (i.e. SHOULD_BECOME_KEY 
>>>> and SHOULD_BECOME_MAIN bits are not set for its native peer); embedded 
>>>> frame receives events from the embedder, (e.g. browser, etc.) but does not 
>>>> translate them to the its own child windows. So if “simple window” is 
>>>> owned by embedded frame it is impossible for the window to receive any key 
>>>> events.
>>>> Fix:
>>>> Set SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits for simple window which 
>>>> is owned by embedded frame.
>>>> Thanks,
>>>> Dmitry
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [11] RFR 8205479: OS X: requestFocus() does not work properly for embedded frame

[Dmitry Markov]

Hi Sergey,

The changes in CPlatformEmbeddedFrame are intended for handling the case when 
the embedder window contains several embedded frames and focus is transferred 
programmatically between the frames. In particular if requestFocus() is invoked 
for some component located at inactive frame it is necessary to activate the 
frame via CPlatformEmbeddedFrame.requestWindowFocus() to make such focus 
transition possible.

I am sorry, I do not have information about whether the situation described 
above is applicable for SWT or not. Anyway it is out of scope for this review.

Thanks,
Dmitry

> On 1 Jul 2018, at 00:21, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry
> Can you please clarify why the changes in CPlatformEmbeddedFrame are 
> necessary? Why the same code does not exists in CViewPlatformEmbeddedFrame?
> 
> On 27/06/2018 10:25, Dmitry Markov wrote:
>> Hi Sergey,
>> You are right, it is better to use synthesizeWindowActivation(). Please find 
>> the updated webrew here: 
>> http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/ 
>> <http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/>
>> Changes:
>>  - Overrode synthesizeWindowActivation() for CEmbeddedFrame. It calls 
>> handleWindowFocusEvent() which actually activates or deactivates embedded 
>> frame.
>>  - Added updateGlobalFocusedWindow() to CEmbeddedFrame. This method should 
>> be called when the focus is transferred to embedded frame programmatically 
>> since handleWindowFocusEvent() skips activation for non-focused embedded 
>> frames.
>> Thanks,
>> Dmitry
>>> On 27 Jun 2018, at 01:20, Sergey Bylokhov >> <mailto:sergey.bylok...@oracle.com> <mailto:sergey.bylok...@oracle.com 
>>> <mailto:sergey.bylok...@oracle.com>>> wrote:
>>> 
>>> Hi, Dmitry.
>>> Can you please confirm that we should not implement 
>>> synthesizeWindowActivation() to achieve this behavior?
>>> I guess we should do the same as in CViewEmbeddedFrame which is used by SWT.
>>> 
>>> On 25/06/2018 09:11, Dmitry Markov wrote:
>>>> Hello,
>>>> Could you review a fix for jdk11, please?
>>>>  bug: https://bugs.openjdk.java.net/browse/JDK-8205479
>>>>  webrev: http://cr.openjdk.java.net/~dmarkov/8205479/webrev.00/
>>>> Problem description:
>>>> On Mac OSX when focus is transferred to some component located at embedded 
>>>> frame, CPlatformEmbeddedFrame.requestWindowFocus() is called to activate 
>>>> owning frame. However that method does nothing, (i.e. no activation 
>>>> happens). As a result the focus cannot be transferred to the component 
>>>> because its owner is not active.
>>>> Fix:
>>>> CPlatformEmbeddedFrame.requestWindowFocus() should activate the embedded 
>>>> frame, (i.e. invoke notifyActivation() for the corresponding peer).
>>>> Thanks,
>>>> Dmitry
>>> 
>>> 
>>> -- 
>>> Best regards, Sergey.
> 
> 
> -- 
> Best regards, Sergey.

Re: [11] RFR 8130655: OS X: keyboard input in textfield is not possible if the window contained textfield is owned by EmbeddedFrame

[Dmitry Markov]

Hi Sergey,

> On 27 Jun 2018, at 01:15, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> Can you please provide some more details why this bug is reproducible only on 
> mac and not on the other platforms(I meant the bug related to the popup menu 
> not SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits)
> 

The implementation of EmbeddedFrame on Mac OSX is lightweight and it is 
heavyweight on Window/Linux. In other words on Windows/Linux an embedded frame 
is able to receive events from the platform by itself and transfer them to its 
child windows if any. However it does not work for Mac OSX: CEmbeddedFrame 
receives events only from the embedder, (e.g. browser) which is not aware of 
any windows owned by the embedded frame. So on Mac OSX if the embedded frame 
owns a simple window (which is unfocusable by default) there is no way to 
provide the simple window with keyboard input. At the same time on other 
platforms the simple window receives key events in the same scenario.

The test applet attached to the bug demonstrates the problem, (i.e. simple 
window owned by embedded frame is unable to receive keyboard input) on Mac and 
its absence on other platform. Currently I have only this test which works on 
all platforms without any modifications.

> I have checked our current behavior on win/lin/mac in all cases the simple 
> "Window" cannot get a focus, without any difference which parent is 
> used(null/frame/window).
> 
> So for example if the window1 is owned by another window2, then both of them 
> are "unfocusable", but after the current fix, both will be "focusable" if the 
> window2 will be owned by the embedded frame.

I agree it was not good idea to make simple window focusable even if it was 
owned by embedded frame. I think it will be better to allow simple window 
receive keyboard input if it is owned by embedded frame. In other words simple 
window will stay unfocusable, (i.e. SHOUL_BECOME_MAIN and SHOULD_BECOME_KEY 
bits are unset) but it will be able to receive key events when necessary. 
Please find the implementation of this approach here: 
http://cr.openjdk.java.net/~dmarkov/8130655/webrev.01/ 
<http://cr.openjdk.java.net/~dmarkov/8130655/webrev.01/>

Changes:
 - Added methods CPlatformWindow and AWTWindow to check whether the window is 
simple and owned by embedded frame
 - Modified canBecomeKeyWindow() in AWTWindow to take into account window type 
and owner

Could you review the new version, please?

Thanks,
Dmitry
> 
> On 25/06/2018 03:35, Dmitry Markov wrote:
>> Hello,
>> Could you review a fix for jdk11, please?
>>  bug: https://bugs.openjdk.java.net/browse/JDK-8130655
>>  webrev: http://cr.openjdk.java.net/~dmarkov/8130655/webrev.00/
>> Problem description:
>> On OS X platform a window does not receive keyboard input if it is owned by 
>> embedded frame. According to the current implementation “simple window” (not 
>> frame or dialog) is NOT natively focusable, (i.e. SHOULD_BECOME_KEY and 
>> SHOULD_BECOME_MAIN bits are not set for its native peer); embedded frame 
>> receives events from the embedder, (e.g. browser, etc.) but does not 
>> translate them to the its own child windows. So if “simple window” is owned 
>> by embedded frame it is impossible for the window to receive any key events.
>> Fix:
>> Set SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits for simple window which is 
>> owned by embedded frame.
>> Thanks,
>> Dmitry
> 
> 
> -- 
> Best regards, Sergey.

Re: [11] RFR 8205479: OS X: requestFocus() does not work properly for embedded frame

[Dmitry Markov]

Hi Sergey,

You are right, it is better to use synthesizeWindowActivation(). Please find 
the updated webrew here: http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/ 
<http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/>

Changes:
 - Overrode synthesizeWindowActivation() for CEmbeddedFrame. It calls 
handleWindowFocusEvent() which actually activates or deactivates embedded frame.
 - Added updateGlobalFocusedWindow() to CEmbeddedFrame. This method should be 
called when the focus is transferred to embedded frame programmatically since 
handleWindowFocusEvent() skips activation for non-focused embedded frames.

Thanks,
Dmitry
> On 27 Jun 2018, at 01:20, Sergey Bylokhov  wrote:
> 
> Hi, Dmitry.
> Can you please confirm that we should not implement 
> synthesizeWindowActivation() to achieve this behavior?
> I guess we should do the same as in CViewEmbeddedFrame which is used by SWT.
> 
> On 25/06/2018 09:11, Dmitry Markov wrote:
>> Hello,
>> Could you review a fix for jdk11, please?
>>  bug: https://bugs.openjdk.java.net/browse/JDK-8205479
>>  webrev: http://cr.openjdk.java.net/~dmarkov/8205479/webrev.00/
>> Problem description:
>> On Mac OSX when focus is transferred to some component located at embedded 
>> frame, CPlatformEmbeddedFrame.requestWindowFocus() is called to activate 
>> owning frame. However that method does nothing, (i.e. no activation 
>> happens). As a result the focus cannot be transferred to the component 
>> because its owner is not active.
>> Fix:
>> CPlatformEmbeddedFrame.requestWindowFocus() should activate the embedded 
>> frame, (i.e. invoke notifyActivation() for the corresponding peer).
>> Thanks,
>> Dmitry
> 
> 
> -- 
> Best regards, Sergey.

[11] RFR 8205479: OS X: requestFocus() does not work properly for embedded frame

[Dmitry Markov]

Hello,

Could you review a fix for jdk11, please?

 bug: https://bugs.openjdk.java.net/browse/JDK-8205479
 webrev: http://cr.openjdk.java.net/~dmarkov/8205479/webrev.00/

Problem description:  
On Mac OSX when focus is transferred to some component located at embedded 
frame, CPlatformEmbeddedFrame.requestWindowFocus() is called to activate owning 
frame. However that method does nothing, (i.e. no activation happens). As a 
result the focus cannot be transferred to the component because its owner is 
not active.   

Fix: 
CPlatformEmbeddedFrame.requestWindowFocus() should activate the embedded frame, 
(i.e. invoke notifyActivation() for the corresponding peer). 

Thanks,
Dmitry 

[11] RFR 8130655: OS X: keyboard input in textfield is not possible if the window contained textfield is owned by EmbeddedFrame

[Dmitry Markov]

Hello,

Could you review a fix for jdk11, please?

 bug: https://bugs.openjdk.java.net/browse/JDK-8130655
 webrev: http://cr.openjdk.java.net/~dmarkov/8130655/webrev.00/

Problem description:
On OS X platform a window does not receive keyboard input if it is owned by 
embedded frame. According to the current implementation “simple window” (not 
frame or dialog) is NOT natively focusable, (i.e. SHOULD_BECOME_KEY and 
SHOULD_BECOME_MAIN bits are not set for its native peer); embedded frame 
receives events from the embedder, (e.g. browser, etc.) but does not translate 
them to the its own child windows. So if “simple window” is owned by embedded 
frame it is impossible for the window to receive any key events. 

Fix:
Set SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN bits for simple window which is 
owned by embedded frame.

Thanks,
Dmitry 

[11] RFR 8200353: Shift or Capslock not working in Textfield after accentued keystrokes

[Dmitry Markov]

Hello,

Could you review a fix for jdk11, please?

 bug: https://bugs.openjdk.java.net/browse/JDK-8200353
 webrev: http://cr.openjdk.java.net/~dmarkov/8200353/webrev.00/

The new mechanism of dead keys detection and processing was introduced by 
JDK-8139189 [1]. According to that changes dead key input is activated by 
WM_KEYDOWN and deactivated by WM_CHAR messages. However the function 
WindowsKeyToJavaChar() (which actually sets the flag deadKeyActive to true) is 
also called from _NativeHandleEvent() and in this case target component doesn't 
receive WM_CHAR message, (i.e. dead key input remains active).
At the same time _NativeHandleEvent() sends character back to the native window 
using WM_AWT_FORWARD_CHAR message. So it is necessary to disable dead key 
input, (i.e. set deadKeyActive to false) when WM_AWT_FORWARD_CHAR is received.

I ran related AWT/Swing regression tests and did not observe any new failures.
 
Thanks,
Dmitry 

[1] - https://bugs.openjdk.java.net/browse/JDK-8139189