8340982: [win] Dead key followed by Space generates two characters instead of one #1584

beldenfox · 2024-09-27T14:29:17Z

The standard across all platforms is:

A dead key followed by a composable character generates the composed character. For example, a circumflex dead key followed by an 'e' should generate 'ê'.
A dead key followed by a character that can't compose with it generates a spacing character followed by the non-composable character. On Windows US International a circumflex dead key followed by a 'q' generates '^q'. The spacing character corresponding to the dead key varies based on the OS and layout.
An exception is SPACE. On all platforms a dead key followed by SPACE should generate just the spacing version of the dead key but not a space character. Users rely on this shortcut to quickly access the character 'hidden' by the dead key.

The Windows glass code didn't implement the Space exception. This PR fixes that.

On Windows the US US International layout. Shift+6 is the dead key for a circumflex diacritic if you want to test out the combinations mentioned above.

For some reason Windows 11 hides this setting well. To install a US International layout:

Go to Settings > Time & Language > Language & Region.
In the entry for English click on the three dots to the far right and select 'Language Options'.
Scroll down until you see 'Installed keyboards' and select 'Add a keyboard'.
From the list select "United States - International".
To actually use the layout look to the right of the Task Bar and you should see a button for choosing the layout (it will contain the word "ENG").

Progress

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

Issue

JDK-8340982: [win] Dead key followed by Space generates two characters instead of one (Bug - P4)

Reviewers

Andy Goryachev (@andy-goryachev-oracle - Reviewer)
John Hendrikx (@hjohn - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1584

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1584.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2024-09-27T14:29:45Z

👋 Welcome back mfox! 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 · 2024-09-27T14:31:08Z

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

8340982: [win] Dead key followed by Space generates two characters instead of one

Reviewed-by: angorya, jhendrikx

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 1 new commit pushed to the master branch:

7870a22: 8297072: Provide gradle option to test a previously built SDK

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

mlbridge · 2024-09-27T14:34:16Z

Webrevs

00: Full (1ee9d318)

andy-goryachev-oracle · 2024-09-27T20:05:17Z

Thank you for logging the ticket!
Do you think it is possible to write a unit test?

beldenfox · 2024-09-27T21:06:42Z

I haven't figured out how to write unit tests for dead keys or IME events. The Windows US English layout doesn't use dead keys. There's no way for me to force the use of a different keyboard layout or verify that the right one is installed before running the test. The best I can do is a manual test like the existing KeyboardTest app in the tests/manual/event folder.

andy-goryachev-oracle · 2024-09-27T21:41:16Z

tested on win11, the behavior matches Notepad.exe with dead keys ^ (shift-6), ~, backtick (shift-tilda), quotes, ' (tick, same key as quotes),
producing the following characters:
â ã à ä á

but using the sequence of ' produces ć instead of ç.

where can I get a complete list of dead keys for the US International keyboard, and will it be any different for other keyboards (is there any impact on this PR for different keyboards)?

edit: the issue above is present in the master branch, so it is a different issue.
the handling of space after the dead keys listed above seems to be correct with this PR.

andy-goryachev-oracle · 2024-09-27T21:42:38Z

Could someone with a German keyboard take a look at this as well?

/reviewers 2

openjdk · 2024-09-27T21:42:54Z

@andy-goryachev-oracle
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

andy-goryachev-oracle · 2024-09-27T22:54:59Z

Using DEAD_ACUTE key the sequence printed by the Keyboard Event Viewer in the Monkey Tester -
master branch:

KeyEvent{type=KEY_PRESSED, character=<\u00>, text=', code=DEAD_ACUTE}
KeyEvent{type=KEY_PRESSED, character=<\u00>, text= , code=SPACE}
KeyEvent{type=KEY_TYPED, character=', text=, code=UNDEFINED}
KeyEvent{type=KEY_TYPED, character=<\u20>, text=, code=UNDEFINED}
KeyEvent{type=KEY_RELEASED, character=<\u00>, text= , code=SPACE}

with this PR:

KeyEvent{type=KEY_PRESSED, character=<\u00>, text=', code=DEAD_ACUTE}
KeyEvent{type=KEY_PRESSED, character=<\u00>, text= , code=SPACE}
KeyEvent{type=KEY_TYPED, character=', text=, code=UNDEFINED}
KeyEvent{type=KEY_RELEASED, character=<\u00>, text= , code=SPACE}

Do we need to create a new ticket for DEAD_ACUTE + c ?
Will it be related to JDK-8183521 ?

hjohn

Thanks, this looks like a good fix.

beldenfox · 2024-09-28T12:59:34Z

Do we need to create a new ticket for DEAD_ACUTE + c ? Will it be related to JDK-8183521 ?

I could not find an official Microsoft page listing the dead key sequences for this keyboard. I did find a few university sites that contain write-ups (like this one at Washington State). It shows the DEAD_ACUTE + c combination as generating a ç. I also get ç in other Windows apps so it doesn't appear to be a bug.

JDK-8183521 is in the same area of the code (good catch!) but whatever is going wrong occurs earlier. I decided to beat my head against that a bit more and will update the bug report with what I find.

beldenfox · 2024-09-28T12:59:46Z

/integrate

openjdk · 2024-09-28T13:00:14Z

Going to push as commit 5428f26.
Since your change was applied there has been 1 commit pushed to the master branch:

7870a22: 8297072: Provide gradle option to test a previously built SDK

Your commit was automatically rebased without conflicts.

openjdk · 2024-09-28T13:00:22Z

@beldenfox Pushed as commit 5428f26.

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

beldenfox · 2024-09-28T16:39:25Z

@andy-goryachev-oracle Sorry, I misread your earlier message.

You're right, in JavaFX DEAD_ACUTE + c is producing the wrong result with the U.S. International layout. We should see ç. I will enter a bug.

andy-goryachev-oracle · 2024-09-30T14:49:08Z

Thank you! Is the DEAD_ACUTE + c problem limited to the US International keyboard, or it also happens with other (e.g. German, French?) keyboards?

mlbridge · 2024-09-30T14:57:34Z

Mailing list message from Johan Corveleyn on openjfx-dev:

On Sat, Sep 28, 2024 at 3:02?PM Martin Fox <mfox at openjdk.org> wrote:

Great! Thanks for the quick fix.

Is there any chance this could be backported to JavaFX 8 and 11 (and
perhaps 21)?
We're currently still on Java 8, but are working to upgrade to 11 in
the next half year or so, and possibly on to 21 after that.

I realize this is not such an important issue (it's been there for
ages). Still, if it's not too much effort, it would help us to get a
hold on that fix (in an official release) in the near future.

Thanks,
--
Johan

mlbridge · 2024-09-30T15:25:34Z

Mailing list message from Kevin Rushforth on openjfx-dev:

Gluon maintains JavaFX 17 and 21, so Johan can answer that.

There is no maintainer for the JavaFX 8 or 11 code lines in OpenJDK.

-- Kevin

On 9/30/2024 7:55 AM, Johan Corveleyn wrote:

On Sat, Sep 28, 2024 at 3:02?PM Martin Fox <mfox at openjdk.org> wrote:
Great! Thanks for the quick fix.

Is there any chance this could be backported to JavaFX 8 and 11 (and
perhaps 21)?
We're currently still on Java 8, but are working to upgrade to 11 in
the next half year or so, and possibly on to 21 after that.

I realize this is not such an important issue (it's been there for
ages). Still, if it's not too much effort, it would help us to get a
hold on that fix (in an official release) in the near future.

Thanks,

beldenfox · 2024-09-30T17:05:51Z

Thank you! Is the DEAD_ACUTE + c problem limited to the US International keyboard, or it also happens with other (e.g. German, French?) keyboards?

I have entered JDK-8341256 with some background information. I'm not aware of other layouts that would have this particular problem. Layouts associated with languages that use ç (like French) have a dedicated key for it. On other keyboards you just can't type ç. The way US International works is sort of unusual since most layouts handle their dead key combinations in a uniform way. Still, given the way Glass works there's no doubt other layouts which will throw it off.

mlbridge · 2024-10-01T20:32:35Z

Mailing list message from Johan Corveleyn on openjfx-dev:

On Mon, Sep 30, 2024 at 5:23?PM Kevin Rushforth
<kevin.rushforth at oracle.com> wrote:

Gluon maintains JavaFX 17 and 21, so Johan can answer that.

There is no maintainer for the JavaFX 8 or 11 code lines in OpenJDK.

Ah yes, for 8 we use the Oracle JDK which includes its JavaFX build.
So for backport to Oracle Java 8 I guess we'd need to ask Oracle.

Having this fix backported into OpenJFX 17 and 21 would be great though.

--
Johan

mlbridge · 2024-10-07T15:03:34Z

Mailing list message from Johan Corveleyn on openjfx-dev:

On Tue, Oct 1, 2024 at 10:30?PM Johan Corveleyn <jcorvel at gmail.com> wrote:

On Mon, Sep 30, 2024 at 5:23?PM Kevin Rushforth
<kevin.rushforth at oracle.com> wrote:

Gluon maintains JavaFX 17 and 21, so Johan can answer that.

There is no maintainer for the JavaFX 8 or 11 code lines in OpenJDK.

Ah yes, for 8 we use the Oracle JDK which includes its JavaFX build.
So for backport to Oracle Java 8 I guess we'd need to ask Oracle.

Having this fix backported into OpenJFX 17 and 21 would be great though.

Coming back to this: any chance this fix could be backported to
OpenJFX 17 and 21?

--
Johan

mlbridge · 2024-11-05T10:42:28Z

Mailing list message from Johan Corveleyn on openjfx-dev:

On Mon, Oct 7, 2024 at 5:01?PM Johan Corveleyn <jcorvel at gmail.com> wrote:

On Tue, Oct 1, 2024 at 10:30?PM Johan Corveleyn <jcorvel at gmail.com> wrote:

On Mon, Sep 30, 2024 at 5:23?PM Kevin Rushforth
<kevin.rushforth at oracle.com> wrote:

Gluon maintains JavaFX 17 and 21, so Johan can answer that.

There is no maintainer for the JavaFX 8 or 11 code lines in OpenJDK.

Ah yes, for 8 we use the Oracle JDK which includes its JavaFX build.
So for backport to Oracle Java 8 I guess we'd need to ask Oracle.

Having this fix backported into OpenJFX 17 and 21 would be great though.

Coming back to this: any chance this fix could be backported to
OpenJFX 17 and 21?

One last try: anyone able to backport this deadkey fix to 17 and 21?
Or even take it into consideration for inclusion in OpenJFX 11 or JDK
8?

--
Johan

mlbridge · 2024-11-05T11:00:30Z

Mailing list message from Johan Vos on openjfx-dev:

Hi Johan,

Sorry for not replying earlier.
Since this is a real small fix, I think it makes sense to backport it to
17/21.
I'm a bit hesitant because of JEP 14 [1] and the current discussions on the
Tip&Tail approach [2] , where it is explicitly discouraged to backport
anything apart from vulnerabilities and critical errors. Since this is a P4
bug, I don't think it qualifies -- hence my doubt.

This is a situation that I believe could be discussed in jdk-dev -- not for
this issue in particular, but rather the principle: what is the
recommendation with backport requests for P4 bugs that are "small" and
"guaranteed to have no regression"?

I don't think it's good to have the discussion at 2 places, but to
summarize some of the key reasons on why not backporting
non-criticial things:
* we do not want to break existing work in LTS releases (software that
relies on some undocumented internal JavaFX behavior might go wrong if the
behavior is changed)
* we need to make sure the CPU fixes can "easily" be backported.
* time spent in tail-backporting can not be spent in tip-development. And
unfortunately, I learned the hard way that backporting is much more
time-consuming (and error-prone) than I hoped for.

Having said that, I definitely don't want to reject this upfront -- just
want to clarify the complexity and I very much welcome other input.

- Johan

[1] https://openjdk.org/jeps/14
[2] https://mail.openjdk.org/pipermail/jdk-dev/2024-October/009433.html

Op di 5 nov 2024 om 11:40 schreef Johan Corveleyn <jcorvel at gmail.com>:

On Mon, Oct 7, 2024 at 5:01?PM Johan Corveleyn <jcorvel at gmail.com> wrote:

On Tue, Oct 1, 2024 at 10:30?PM Johan Corveleyn <jcorvel at gmail.com>
wrote:

On Mon, Sep 30, 2024 at 5:23?PM Kevin Rushforth
<kevin.rushforth at oracle.com> wrote:

Gluon maintains JavaFX 17 and 21, so Johan can answer that.

There is no maintainer for the JavaFX 8 or 11 code lines in OpenJDK.

Ah yes, for 8 we use the Oracle JDK which includes its JavaFX build.
So for backport to Oracle Java 8 I guess we'd need to ask Oracle.

Having this fix backported into OpenJFX 17 and 21 would be great
though.

Coming back to this: any chance this fix could be backported to
OpenJFX 17 and 21?

One last try: anyone able to backport this deadkey fix to 17 and 21?
Or even take it into consideration for inclusion in OpenJFX 11 or JDK
8?

--
Johan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20241105/6444e454/attachment.htm>

mlbridge · 2024-11-06T11:00:28Z

Mailing list message from Johan Corveleyn on openjfx-dev:

Hi Johan,

Thanks for explaining. I've taken a look at that JEP 14 document, and
I can certainly understand the reasoning behind it.

And indeed, this is a P4 bug, not particularly critical. Though the
fix is small and low-risk, it is still a change and it inevitably
takes resources to backport (always more than expected, I know :-)).
Depending on how strict the "backport as little as possible" rule is
interpreted, this may or may not be taken along.

I don't have anything particular to throw into the balance, so I'll
just see where this goes. In any case, thanks for considering /
discussing it, and for having taken the time for your thoughful
response.

--
Johan C

On Tue, Nov 5, 2024 at 11:59?AM Johan Vos <johan at lodgon.com> wrote:

Hi Johan,

Sorry for not replying earlier.
Since this is a real small fix, I think it makes sense to backport it to 17/21.
I'm a bit hesitant because of JEP 14 [1] and the current discussions on the Tip&Tail approach [2] , where it is explicitly discouraged to backport anything apart from vulnerabilities and critical errors. Since this is a P4 bug, I don't think it qualifies -- hence my doubt.

This is a situation that I believe could be discussed in jdk-dev -- not for this issue in particular, but rather the principle: what is the recommendation with backport requests for P4 bugs that are "small" and "guaranteed to have no regression"?

I don't think it's good to have the discussion at 2 places, but to summarize some of the key reasons on why not backporting non-criticial things:
* we do not want to break existing work in LTS releases (software that relies on some undocumented internal JavaFX behavior might go wrong if the behavior is changed)
* we need to make sure the CPU fixes can "easily" be backported.
* time spent in tail-backporting can not be spent in tip-development. And unfortunately, I learned the hard way that backporting is much more time-consuming (and error-prone) than I hoped for.

Having said that, I definitely don't want to reject this upfront -- just want to clarify the complexity and I very much welcome other input.

- Johan

[1] https://openjdk.org/jeps/14
[2] https://mail.openjdk.org/pipermail/jdk-dev/2024-October/009433.html

Op di 5 nov 2024 om 11:40 schreef Johan Corveleyn <jcorvel at gmail.com>:

On Mon, Oct 7, 2024 at 5:01?PM Johan Corveleyn <jcorvel at gmail.com> wrote:

On Tue, Oct 1, 2024 at 10:30?PM Johan Corveleyn <jcorvel at gmail.com> wrote:

On Mon, Sep 30, 2024 at 5:23?PM Kevin Rushforth
<kevin.rushforth at oracle.com> wrote:

Gluon maintains JavaFX 17 and 21, so Johan can answer that.

There is no maintainer for the JavaFX 8 or 11 code lines in OpenJDK.

Ah yes, for 8 we use the Oracle JDK which includes its JavaFX build.
So for backport to Oracle Java 8 I guess we'd need to ask Oracle.

Having this fix backported into OpenJFX 17 and 21 would be great though.

Coming back to this: any chance this fix could be backported to
OpenJFX 17 and 21?

One last try: anyone able to backport this deadkey fix to 17 and 21?
Or even take it into consideration for inclusion in OpenJFX 11 or JDK
8?

--
Johan

A dead key followed by Space generates a single character, not two

1ee9d31

openjdk bot added the rfr Ready for review label Sep 27, 2024

andy-goryachev-oracle approved these changes Sep 27, 2024

View reviewed changes

hjohn approved these changes Sep 28, 2024

View reviewed changes

openjdk bot added the ready Ready to be integrated label Sep 28, 2024

openjdk bot added the integrated Pull request has been integrated label Sep 28, 2024

openjdk bot closed this Sep 28, 2024

openjdk bot removed ready Ready to be integrated rfr Ready for review labels Sep 28, 2024

beldenfox deleted the deadkeyspace branch September 30, 2024 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8340982: [win] Dead key followed by Space generates two characters instead of one #1584

8340982: [win] Dead key followed by Space generates two characters instead of one #1584

beldenfox commented Sep 27, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Sep 27, 2024

openjdk bot commented Sep 27, 2024 •

edited

Loading

mlbridge bot commented Sep 27, 2024

andy-goryachev-oracle commented Sep 27, 2024

beldenfox commented Sep 27, 2024

andy-goryachev-oracle commented Sep 27, 2024 •

edited

Loading

andy-goryachev-oracle commented Sep 27, 2024

openjdk bot commented Sep 27, 2024

andy-goryachev-oracle commented Sep 27, 2024 •

edited

Loading

hjohn left a comment

beldenfox commented Sep 28, 2024

beldenfox commented Sep 28, 2024

openjdk bot commented Sep 28, 2024

openjdk bot commented Sep 28, 2024

beldenfox commented Sep 28, 2024

andy-goryachev-oracle commented Sep 30, 2024

mlbridge bot commented Sep 30, 2024

mlbridge bot commented Sep 30, 2024

beldenfox commented Sep 30, 2024

mlbridge bot commented Oct 1, 2024

mlbridge bot commented Oct 7, 2024

mlbridge bot commented Nov 5, 2024

mlbridge bot commented Nov 5, 2024

mlbridge bot commented Nov 6, 2024

8340982: [win] Dead key followed by Space generates two characters instead of one #1584

8340982: [win] Dead key followed by Space generates two characters instead of one #1584

Conversation

beldenfox commented Sep 27, 2024 • edited by openjdk bot Loading

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Sep 27, 2024

openjdk bot commented Sep 27, 2024 • edited Loading

mlbridge bot commented Sep 27, 2024

Webrevs

andy-goryachev-oracle commented Sep 27, 2024

beldenfox commented Sep 27, 2024

andy-goryachev-oracle commented Sep 27, 2024 • edited Loading

andy-goryachev-oracle commented Sep 27, 2024

openjdk bot commented Sep 27, 2024

andy-goryachev-oracle commented Sep 27, 2024 • edited Loading

hjohn left a comment

Choose a reason for hiding this comment

beldenfox commented Sep 28, 2024

beldenfox commented Sep 28, 2024

openjdk bot commented Sep 28, 2024

openjdk bot commented Sep 28, 2024

beldenfox commented Sep 28, 2024

andy-goryachev-oracle commented Sep 30, 2024

mlbridge bot commented Sep 30, 2024

mlbridge bot commented Sep 30, 2024

beldenfox commented Sep 30, 2024

mlbridge bot commented Oct 1, 2024

mlbridge bot commented Oct 7, 2024

mlbridge bot commented Nov 5, 2024

mlbridge bot commented Nov 5, 2024

mlbridge bot commented Nov 6, 2024

beldenfox commented Sep 27, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Sep 27, 2024 •

edited

Loading

andy-goryachev-oracle commented Sep 27, 2024 •

edited

Loading

andy-goryachev-oracle commented Sep 27, 2024 •

edited

Loading