JDK-8305712: [MacOS] Deprecated Cocoa-NSEvent names #13435

honkar-jdk · 2023-04-11T20:28:29Z

There are a few deprecated Cocoa NSEvent names currently in use in JDK.

https://github.com/openjdk/jdk/blob/jdk-21+17/src/java.desktop/macosx/classes/sun/lwawt/macosx/CocoaConstants.java
https://developer.apple.com/documentation/appkit/nsmousemoved

Since these event names are marked for deprecation it is better to switch to the newer names.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8305712: [MacOS] Deprecated Cocoa-NSEvent names

Reviewers

Dmitry Markov (@dmarkov20 - Reviewer)
Phil Race (@prrace - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13435/head:pull/13435
$ git checkout pull/13435

Update a local copy of the PR:
$ git checkout pull/13435
$ git pull https://git.openjdk.org/jdk.git pull/13435/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13435

View PR using the GUI difftool:
$ git pr show -t 13435

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13435.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-04-11T20:30:11Z

👋 Welcome back honkar! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-04-11T20:32:03Z

@honkar-jdk The following label will be automatically applied to this pull request:

client

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-04-11T20:35:27Z

Webrevs

prrace · 2023-04-11T20:39:56Z

This has been tested in the CI build system, right ? And all tests pass too, right ?

prrace · 2023-04-11T20:41:16Z

src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m

@@ -999,7 +999,7 @@ - (void)windowDidExitFullScreen:(NSNotification *)notification {
 }

 - (void)sendEvent:(NSEvent *)event {
-        if ([event type] == NSLeftMouseDown || [event type] == NSRightMouseDown || [event type] == NSOtherMouseDown) {


since you are touching this, maybe we can format it for better legibility !

if ([event type] == NSEventTypeLeftMouseDown || [event type] == NSEventTypeRightMouseDown || [event type] == NSEventTypeOtherMouseDown)) {

prrace · 2023-04-11T20:44:38Z

src/java.desktop/macosx/native/libosxui/ScreenMenu.m

@@ -173,16 +173,16 @@ - (void)handleJavaMouseEvent:(NSEvent *)event
    jint javaKind = 0;

    switch (kind) {
-        case NSLeftMouseUp: case NSRightMouseUp: case NSOtherMouseUp:


ugh. I've never seen multiple case: clauses on one line.
break these up too.

honkar-jdk · 2023-04-11T20:51:44Z

This has been tested in the CI build system, right ? And all tests pass too, right ?

@prrace That is correct Phil, CI testing passes and additionally, I checked by running mouse related tests locally - java/awt/Mouse (since all the NSEvents correspond to mouse events) and they work the same in both cases - before and after the changes. Are there any other set of tests to be cross-checked again to ensure the changes don't break anything?

honkar-jdk · 2023-04-11T20:55:16Z

I missed updating the copyright year, will be updating with the rest of the changes in the next commit.

honkar-jdk · 2023-04-11T23:04:16Z

test/jdk/java/awt/Mouse/MacTouchPressEventModifiers/MouseMacTouchPressEventModifiers.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
 * Copyright (c) 2022, JetBrains s.r.o.. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.


Not sure if we need to have Oracle and JetBrains copyright here? The test was originally contributed by JetBrains team.

IIRC we need both Oracle and JetBrains copyrights here.

You need to have two years in Oracle copyright: 1st - the year of the initial load; 2nd - the year of the last modification. In other words something like that:
Copyright (c) 2022, 2023,

dmarkov20 · 2023-04-12T06:46:45Z

test/jdk/java/awt/Mouse/MacTouchPressEventModifiers/MouseMacTouchPressEventModifiers.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
 * Copyright (c) 2022, JetBrains s.r.o.. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.


IIRC we need both Oracle and JetBrains copyrights here.

You need to have two years in Oracle copyright: 1st - the year of the initial load; 2nd - the year of the last modification. In other words something like that:
Copyright (c) 2022, 2023,

openjdk · 2023-04-12T22:39:33Z

@honkar-jdk This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8305712: [MacOS] Deprecated Cocoa-NSEvent names

Reviewed-by: dmarkov, prr

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 182 new commits pushed to the master branch:

3f36dd8: 8305529: DefaultProxySelector.select(URI) in certain cases returns a List with null element
425ef06: 8303923: ZipOutStream::putEntry should include an apiNote to indicate that the STORED compression method should be used when writing directory entries
2bbbff2: 8305858: Resolve multiple definition of 'handleSocketError' when statically linking with JDK native libraries
bc15163: 8304834: Fix wrapper insertion in TestScaffold.parseArgs(String args[])
19380d7: 8305324: C2: Wrong execution of vectorizing Interger.reverseBytes
87017b5: 8295859: Update Manual Test Groups
99a9dbc: 8305783: x86_64: Optimize AbsI and AbsL
d8af7a6: 8304725: AsyncGetCallTrace can cause SIGBUS on M1
b9bdbe9: 8305524: AArch64: Fix arraycopy issue on SVE caused by matching rule vmask_gen_sub
82e8b03: 8305203: Simplify trimming operation in Region::Ideal
... and 172 more: https://git.openjdk.org/jdk/compare/50a995f03a0cc5c342929a0f48c43fd04fdf0b0d...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

honkar-jdk · 2023-04-13T15:25:39Z

/integrate

openjdk · 2023-04-13T15:27:03Z

Going to push as commit d7dc474.
Since your change was applied there have been 188 commits pushed to the master branch:

e0620b8: 8296420: javac has long lines in its command-line help
6b9b7d1: 8304350: Font.getStringBounds calculates wrong width for TextAttribute.TRACKING other than 0.0
2060c8e: 8305688: jdk build --with-memory-size=1024 broken by JDK-8305100
646b666: 8305900: Use loopback IP addresses in security policy files of httpclient tests
fb9a29d: 8305414: gtest/NMTGtests.java is failing various sub-tests
e846a1d: 8304450: [vectorapi] Refactor VectorShuffle implementation
3f36dd8: 8305529: DefaultProxySelector.select(URI) in certain cases returns a List with null element
425ef06: 8303923: ZipOutStream::putEntry should include an apiNote to indicate that the STORED compression method should be used when writing directory entries
2bbbff2: 8305858: Resolve multiple definition of 'handleSocketError' when statically linking with JDK native libraries
bc15163: 8304834: Fix wrapper insertion in TestScaffold.parseArgs(String args[])
... and 178 more: https://git.openjdk.org/jdk/compare/50a995f03a0cc5c342929a0f48c43fd04fdf0b0d...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-04-13T15:27:22Z

@honkar-jdk Pushed as commit d7dc474.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

honkar-jdk added 2 commits April 10, 2023 17:18

renamed deprecated Cocoa NSEvents

f003aed

prefix added to commented event types

291ae4b

openjdk bot added the rfr Pull request is ready for review label Apr 11, 2023

openjdk bot added the client client-libs-dev@openjdk.org label Apr 11, 2023

prrace reviewed Apr 11, 2023

View reviewed changes

copyright year & formatting update

e9b183e

honkar-jdk commented Apr 11, 2023

View reviewed changes

dmarkov20 suggested changes Apr 12, 2023

View reviewed changes

copyright year updated

e091a02

prrace approved these changes Apr 12, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Apr 12, 2023

dmarkov20 approved these changes Apr 13, 2023

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Apr 13, 2023

openjdk bot closed this Apr 13, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 13, 2023

honkar-jdk mentioned this pull request Oct 11, 2023

8317970: Bump target macosx-x64 version to 11.00.00 #16155

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8305712: [MacOS] Deprecated Cocoa-NSEvent names #13435

JDK-8305712: [MacOS] Deprecated Cocoa-NSEvent names #13435

honkar-jdk commented Apr 11, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Apr 11, 2023

openjdk bot commented Apr 11, 2023

mlbridge bot commented Apr 11, 2023 •

edited

Loading

prrace commented Apr 11, 2023

prrace Apr 11, 2023

prrace Apr 11, 2023

honkar-jdk commented Apr 11, 2023

honkar-jdk commented Apr 11, 2023

honkar-jdk Apr 11, 2023 •

edited

Loading

dmarkov20 Apr 12, 2023

dmarkov20 Apr 12, 2023

openjdk bot commented Apr 12, 2023 •

edited

Loading

honkar-jdk commented Apr 13, 2023

openjdk bot commented Apr 13, 2023

openjdk bot commented Apr 13, 2023

JDK-8305712: [MacOS] Deprecated Cocoa-NSEvent names #13435

JDK-8305712: [MacOS] Deprecated Cocoa-NSEvent names #13435

Conversation

honkar-jdk commented Apr 11, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Apr 11, 2023

openjdk bot commented Apr 11, 2023

mlbridge bot commented Apr 11, 2023 • edited Loading

Webrevs

prrace commented Apr 11, 2023

prrace Apr 11, 2023

Choose a reason for hiding this comment

prrace Apr 11, 2023

Choose a reason for hiding this comment

honkar-jdk commented Apr 11, 2023

honkar-jdk commented Apr 11, 2023

honkar-jdk Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

dmarkov20 Apr 12, 2023

Choose a reason for hiding this comment

dmarkov20 Apr 12, 2023

Choose a reason for hiding this comment

openjdk bot commented Apr 12, 2023 • edited Loading

honkar-jdk commented Apr 13, 2023

openjdk bot commented Apr 13, 2023

openjdk bot commented Apr 13, 2023

honkar-jdk commented Apr 11, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Apr 11, 2023 •

edited

Loading

honkar-jdk Apr 11, 2023 •

edited

Loading

openjdk bot commented Apr 12, 2023 •

edited

Loading