8267374: macOS: Option+Up/Down Arrow don't traverse to beginning/end of line in JTextArea

[prsadhuk]

Unlike in native "Notes" editor where Option+Up/Down traverses to start/end of each line respectively,
Java does not honour these key combination in editor.

Added the key combination support by moving the caret to start/end of line and then doing caret up/down a line depending on if we are pressing UP or DOWN key.

---

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
• Change requires a CSR request to be approved

Issues

• JDK-8267374: macOS: Option+Up/Down Arrow don't traverse to beginning/end of line in JTextArea
• JDK-8291468: macOS: Option+Up/Down Arrow don't traverse to beginning/end of line in JTextArea (CSR)

Reviewers

• Phil Race (@prrace - Reviewer) ⚠️ Review applies to 392b415a

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 9230

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

Using diff file

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

[bridgekeeper]

👋 Welcome back psadhukhan! 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]

@prsadhuk 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]

Webrevs

• 08: Full - Incremental (78df4a71)
• 07: Full - Incremental (392b415a)
• 06: Full - Incremental (3fa91217)
• 05: Full - Incremental (a586dd05)
• 04: Full - Incremental (71fa4673)
• 03: Full - Incremental (8149271b)
• 02: Full - Incremental (0268daed)
• 01: Full - Incremental (2b523c29)
• 00: Full (95d2ff04)

[mrserb]

Is it possible that the submitter tested the right option key? I think it stopped working somewhere after jdk8 as described in the JBS.

[prsadhuk]

Is it possible that the submitter tested the right option key? I think it stopped working somewhere after jdk8 as described in the JBS.

Right option key does not work at all...Left option key worked for left-right but not for up-down..this PR addresses that left option key up/down issue..

[mrserb]

Right option key does not work at all...Left option key worked for left-right but not for up-down..this PR addresses that left option key up/down issue..

But the right key worked in jdk8, no?

[prsadhuk]

Right option key does not work at all...Left option key worked for left-right but not for up-down..this PR addresses that left option key up/down issue..

But the right key worked in jdk8, no?

Right Option Key worked for left-right, not for up-down in jdk8..in fact it stopped working from jdk9-b120 even for left-right it seems..
BTW, what is the keycode for Right Option Key? Is it VK_ALT_GRAPH?

[prsadhuk]

Right option key does not work at all...Left option key worked for left-right but not for up-down..this PR addresses that left option key up/down issue..

But the right key worked in jdk8, no?

Right Option Key worked for left-right, not for up-down in jdk8..in fact it stopped working from jdk9-b120 even for left-right it seems.. BTW, what is the keycode for Right Option Key? Is it VK_ALT_GRAPH?

JDK-8151136 regressed Right Option Key working for making test/java/awt/event/MouseEvent/AltGraphModifierTest/AltGraphModifierTest.java work which only tests for Linux as per manual instruction so no need to test it for mac

[mrserb]

JDK-8151136 was about the handling of the right alt key? It had the instructions for windows and linux, and that fix added the case for macOS. I do not think it should be reverted. Probably the shortcuts should take care about that right-option key.

[prsadhuk]

JDK-8151136 was about the handling of the right alt key? It had the instructions for windows and linux, and that fix added the case for macOS. I do not think it should be reverted. Probably the shortcuts should take care about that right-option key.

But if you take a look now, that test only do the test for linux and all other instructions for other platforms are removed. I couldn't find the JBS issue which did the removal but I guess it was done for a cause.

Also, I couldn't find any shortcuts for right-option key...It seems, both left and right option key has same keycode from native NSAlternateKeyMask

If you still insist on reverting, I will request to let this PR to tackle only Left Option for now. I will file another issue to take a look at Right Option Key.

[prsadhuk]

JDK-8151136 was about the handling of the right alt key?

Yes. As per the comments on the bug

Cause of the problem:
On OS X AltGraph functionality was not implemented. 
Fix:
Right Option key is defined to behave as AltGraph key on OS X. Left Option key only act as Alt key. 

Now, our key bindings are all Alt Key based.
https://github.com/openjdk/jdk/blob/master/src/java.desktop/macosx/classes/com/apple/laf/AquaKeyBindings.java#L89
so it used to work for Right Option key till this fix was integrated...It still works for Left Option key as it is mapped to Alt..
Now it return VK_ALT_GRAPH and we do not have keybindings for that.. I do not see anywhere in any L&F ALT_GRAPH is used

so either we need to have VK_ALT as used before the fix (which is why I reverted) or somewhat use ALT_GRAPH binding.

Do you know how to have "ALT GRAPH" key binding?

[mlbridge]

Mailing list message from Alan Snyder on client-libs-dev:

What is a ?PC keyboard?? Do you mean a keyboard attached to a machine running Windows?

On Jul 12, 2022, at 12:51 PM, Sergey Bylokhov <serb at openjdk.org> wrote:

So I do not see a reason why the "right option" - "alt gr" key on the PC keyboard cannot be mapped to the ALT_GRAPH, same as "left option" - "alt" is mapped to the ALT.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/client-libs-dev/attachments/20220712/c9b70407/attachment.htm>

[prrace]

/csr

New API here, it seems

[openjdk]

@prrace has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@prsadhuk please create a CSR request for issue JDK-8267374 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

[prsadhuk]

Modified and CSR added https://bugs.openjdk.org/browse/JDK-8291468

[openjdk]

@prsadhuk 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:

8267374: macOS: Option+Up/Down Arrow don't traverse to beginning/end of line in JTextArea

Reviewed-by: 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 817 new commits pushed to the master branch:

• 1ed03d8: 8292226: Prepare make for better Link Time Optimization support
• 79597f1: 8272702: Resolving URI relative path with no / may lead to incorrect toString
• 7b5f9ed: 8288966: Better handle very spiky promotion in G1
• 07c7977: 8290249: Vectorize signum on AArch64
• a3ec0bb: 8253413: [REDO] [REDO] G1 incorrectly limiting young gen size when using the reserve can result in repeated full gcs
• 27b0f77: 8292318: Memory corruption in remove_dumptime_info
• 9a65524: 8290300: Use standard String-joining tools where applicable
• f9004fe: 8292561: Make "ReplayCompiles" a diagnostic product switch
• 2fbb936: 8292691: Move CompilerConfig::is_xxx() inline functions out of compilerDefinitions.hpp
• 3601e30: 8290909: MemoryPoolMBean/isUsageThresholdExceeded tests failed with "isUsageThresholdExceeded() returned false, and is still false, while threshold = MMMMMMM and used peak = NNNNNNN"
• ... and 807 more: https://git.openjdk.org/jdk/compare/fe807217a79753f84c00432e7451c17baa6645c5...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.

[prsadhuk]

/integrate

[openjdk]

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

• 8df671c: 8293355: JDK-8293167 included bad copyright header
• 5bed9f7: 8293290: RISC-V: Explicitly pass a third temp register to MacroAssembler::store_heap_oop
• 48b3ab0: 8293167: Memory leak in JfrThreadSampler if stackdepth is larger than default (64)
• 4067321: 8291586: Broken links in JVMTI specification
• 32f4dc8: 8293295: Add type check asserts to java_lang_ref_Reference accessors
• e945619: 8293088: Fix compilation with the new Visual Studio preprocessor
• 730ced9: 8292660: C2: blocks made unreachable by NeverBranch-to-Goto conversion are removed incorrectly
• 3464019: 8292899: CustomTzIDCheckDST.java testcase failed on AIX platform
• e92b9e4: 8293325: Minor improvements to macos catch_mach_exception_raise() error handling
• 767262e: 8292201: serviceability/sa/ClhsdbThreadContext.java fails with "'Thread "Common-Cleaner"' missing from stdout/stderr"
• ... and 978 more: https://git.openjdk.org/jdk/compare/fe807217a79753f84c00432e7451c17baa6645c5...master

Your commit was automatically rebased without conflicts.

[openjdk]

@prsadhuk Pushed as commit 955baa3.

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