Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread John Hendrikx
On Thu, 15 Feb 2024 17:29:10 GMT, Andy Goryachev  wrote:

>> Move caret animation handling due to keyboard input to the Skin, by 
>> registering a listener on the caretPosition property.  This will restart 
>> animation only when the caret position changes instead of every key press.
>> 
>> This also simplifies internal behaviors of TextArea, TextField, and 
>> PasswordField in light of the future InputMap RFE 
>> [JDK-8314968](https://bugs.openjdk.org/browse/JDK-8314968)
>
> Andy Goryachev has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains four additional 
> commits since the last revision:
> 
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8322748.blink
>  - Merge remote-tracking branch 'origin/master' into 8322748.blink
>  - 8322748: Caret blinking in JavaFX should only stop when caret moves

Marked as reviewed by jhendrikx (Committer).

-

PR Review: https://git.openjdk.org/jfx/pull/1368#pullrequestreview-1885127795


Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread John Hendrikx
On Thu, 15 Feb 2024 17:26:39 GMT, Andy Goryachev  wrote:

> > I think if we're busy here, it would be worthwhile to also remove the other 
> > calls to the skin to change the caret animating.
> 
> I would rather not: it would require a much larger effort. Avoiding the caret 
> flickering when its position does not change is an incremental improvement 
> from which we all benefit.

Alright, the original issue did envision this though, specifically to start get 
rid of some of the blockers for a Behavior API.  The issue isn't fully 
addressed then, so I guess I'll file another one.

-

PR Comment: https://git.openjdk.org/jfx/pull/1368#issuecomment-1948368629


Re: [jfx17u] RFR: 8221261: Deadlock on macOS in JFXPanel app when handling IME calls

2024-02-16 Thread Kevin Rushforth
On Thu, 8 Feb 2024 09:11:13 GMT, Johannes Bechberger  
wrote:

> Clean backport of 8221261: Deadlock on macOS in JFXPanel app when handling 
> IME calls

The backport looks fine to me.

@johanvos will need to review / approve it for inclusion into jfx17u as he 
maintains this code line.

-

PR Comment: https://git.openjdk.org/jfx17u/pull/178#issuecomment-1948371652


[jfx22u] RFR: 8323880: Caret rendered at wrong position in case of a click event on RTL text

2024-02-16 Thread Jay Bhaskar
A clean backport to jfx22u. The fix is for rtl text issue, where the caret 
position is at the wrong position. I have tested the fix it is working with the 
fix and failing without the fix.

-

Commit messages:
 - Backport 1fb56e333bc65860cc1abeebd1cbb01cd8b8e5f3

Changes: https://git.openjdk.org/jfx22u/pull/12/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx22u&pr=12&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8323880
  Stats: 12 lines in 1 file changed: 12 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jfx22u/pull/12.diff
  Fetch: git fetch https://git.openjdk.org/jfx22u.git pull/12/head:pull/12

PR: https://git.openjdk.org/jfx22u/pull/12


[jfx22u] Integrated: 8323880: Caret rendered at wrong position in case of a click event on RTL text

2024-02-16 Thread Jay Bhaskar
On Fri, 16 Feb 2024 14:00:57 GMT, Jay Bhaskar  wrote:

> A clean backport to jfx22u. The fix is for rtl text issue, where the caret 
> position is at the wrong position. I have tested the fix it is working with 
> the fix and failing without the fix.

This pull request has now been integrated.

Changeset: 9fac61d7
Author:Jay Bhaskar 
URL:   
https://git.openjdk.org/jfx22u/commit/9fac61d7afd5bfb6095edc50572a84026f9c419f
Stats: 12 lines in 1 file changed: 12 ins; 0 del; 0 mod

8323880: Caret rendered at wrong position in case of a click event on RTL text

Backport-of: 1fb56e333bc65860cc1abeebd1cbb01cd8b8e5f3

-

PR: https://git.openjdk.org/jfx22u/pull/12


Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread Andy Goryachev
On Thu, 15 Feb 2024 17:29:10 GMT, Andy Goryachev  wrote:

>> Move caret animation handling due to keyboard input to the Skin, by 
>> registering a listener on the caretPosition property.  This will restart 
>> animation only when the caret position changes instead of every key press.
>> 
>> This also simplifies internal behaviors of TextArea, TextField, and 
>> PasswordField in light of the future InputMap RFE 
>> [JDK-8314968](https://bugs.openjdk.org/browse/JDK-8314968)
>
> Andy Goryachev has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains four additional 
> commits since the last revision:
> 
>  - review comments
>  - Merge remote-tracking branch 'origin/master' into 8322748.blink
>  - Merge remote-tracking branch 'origin/master' into 8322748.blink
>  - 8322748: Caret blinking in JavaFX should only stop when caret moves

@aghaisas or @karthikpandelu could you please take a look at this PR?

-

PR Comment: https://git.openjdk.org/jfx/pull/1368#issuecomment-1948731415


Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread Andy Goryachev
On Fri, 16 Feb 2024 13:15:14 GMT, John Hendrikx  wrote:

> so I guess I'll file another one.

Thank you, please do.

As a side note, before we can start re-writing the controls' behaviors, we 
should:

a) identify all the behavioral aspects of each control and
b) develop a set of tests to exercise each one, along the lines of 
[JDK-8314906](https://bugs.openjdk.org/browse/JDK-8314906)

without these steps, we are guaranteed to create regression.

-

PR Comment: https://git.openjdk.org/jfx/pull/1368#issuecomment-1948730006


Re: [External] : Re: Platform preferences theme detection

2024-02-16 Thread Kevin Rushforth

Good to hear. Thanks for confirming.

-- Kevin

On 2/15/2024 4:46 AM, Christopher Schnick wrote:


Just wanted to let you know that after setting 
apple.awt.application.appearance=system everything works as expected 
now. Thanks for all the work on this great feature. I plan to release 
the next version of our application with platform preferences 
integration soon to production.


On 13/02/2024 20:11, Michael Strauß wrote:

The reason why the values are wrong might be that in PlatformSupport.m
L105, we query [NSApp effectiveApperance], but this always seems to
return the light appearance if JavaFX doesn't own the NSApplication.

On Mon, Feb 12, 2024 at 6:26 PM Kevin Rushforth
  wrote:

Actually, it's worse than not detecting changes, it's simply not getting the 
right values at all. If I run the program when the system appearance is already 
in Dark mode, it doesn't get the correct values at start up.

-- Kevin


Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread John Hendrikx
On Fri, 16 Feb 2024 16:00:07 GMT, Andy Goryachev  wrote:

> As a side note, before we can start re-writing the controls' behaviors, we 
> should:
> 
> a) identify all the behavioral aspects of each control and b) develop a set 
> of tests to exercise each one, along the lines of 
> [JDK-8314906](https://bugs.openjdk.org/browse/JDK-8314906)
> 
> without these steps, we are guaranteed to create regression.

I think those tests should be unit tests, not via Robot, to make this a 
feasible endeavor. This is the reason why I asked about a mocking framework to 
be included in FX test utilities. We should look into making it much easier to 
set up a Behavior or Control or Skin, without having to fire up a complete FX.  
Things like animations and timers should be mocked out, so there can be precise 
control during the tests.

-

PR Comment: https://git.openjdk.org/jfx/pull/1368#issuecomment-1949186138


Re: RFR: 8322748: Caret blinking in JavaFX should only stop when caret moves [v2]

2024-02-16 Thread Andy Goryachev
On Fri, 16 Feb 2024 19:23:00 GMT, John Hendrikx  wrote:

> I think those tests should be unit tests, not via Robot

Preferably, yes.  If something can be tested via headless unit tests, it should 
be done so, you are absolutely right.

Some tests, however, require a fully rendered skin - for example, PgUp in a 
TextArea.  I suspect only 10-20% of all the tests will require a headful 
environment.

-

PR Comment: https://git.openjdk.org/jfx/pull/1368#issuecomment-1949198317


Re: [jfx17u] RFR: 8221261: Deadlock on macOS in JFXPanel app when handling IME calls

2024-02-16 Thread Johan Vos
On Thu, 8 Feb 2024 09:11:13 GMT, Johannes Bechberger  
wrote:

> Clean backport of 8221261: Deadlock on macOS in JFXPanel app when handling 
> IME calls

Marked as reviewed by jvos (Reviewer).

-

PR Review: https://git.openjdk.org/jfx17u/pull/178#pullrequestreview-1885952738


Re: UI scaling issues when physical and logical screen resolutions differ on Linux

2024-02-16 Thread Martin Fox
Hi Christopher,

This may be a side-effect of using KDE. To determine the UI scale the JavaFX 
code consults the “scaling-factor” setting in the “org.gnome.desktop.interface” 
schema. You can check this on the command line:

gsettings get org.gnome.desktop.interface scaling-factor

This should be 0 so JavaFX can compute the scale itself. If it’s greater than 0 
that’s the value JavaFX will use for the UI scale.

It appears that a KDE install can set this value to 1. In my case I started 
with the ARM version of Ubuntu server and then installed KDE (kubuntu-desktop) 
and afterward the scaling-factor was 1. This doesn’t happen when installing the 
standard GNOME desktop.

Martin

> On Feb 13, 2024, at 2:13 AM, Christopher Schnick  wrote:
> 
> Hello,
> 
> several users of our JavaFX applications have reported that the UI scale is 
> too small when the physical and logical screen resolutions differ on Linux. 
> For example in this case
> 
> 
> 
> there is an implicit scaling factor of 150% included as the monitor is a 4k 
> display but is using a lowered resolution of 2560x1440. This is then further 
> stretched as the OS resolution is 1920x1080, but the main problem is that the 
> 150% factor is somehow not getting picked up and JavaFX is treating this as a 
> 4k display, thus making everything too small. For now these users can use 
> -Dglass.gtk.uiScale=1.5 but that is not a nice solution to this problem.
> 
> Best
> Christopher Schnick
> 



[jfx21u] RFR: 8325154: resizeColumnToFitContent is slower than it needs to be

2024-02-16 Thread Jose Pereda
Hi all,

This pull request contains a clean backport of commit 
[e2f42c5b](https://github.com/openjdk/jfx/commit/e2f42c5b71ef061c614540508fbac3fb610b1ae3)
 from the [openjdk/jfx](https://git.openjdk.org/jfx) repository.

The commit being backported was authored by Robert Lichtenberger on 14 Feb 2024 
and was reviewed by Andy Goryachev and Marius Hanl.

Thanks!

-

Commit messages:
 - Backport e2f42c5b71ef061c614540508fbac3fb610b1ae3

Changes: https://git.openjdk.org/jfx21u/pull/42/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx21u&pr=42&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8325154
  Stats: 8 lines in 1 file changed: 4 ins; 4 del; 0 mod
  Patch: https://git.openjdk.org/jfx21u/pull/42.diff
  Fetch: git fetch https://git.openjdk.org/jfx21u.git pull/42/head:pull/42

PR: https://git.openjdk.org/jfx21u/pull/42


Proposal: Bump minimum JDK version for JavaFX 23 to JDK 21

2024-02-16 Thread Kevin Rushforth

All,

Even though we build JavaFX binaries with JDK 21 as the boot JDK, the 
latest version of JavaFX still runs with JDK 17, although it isn't 
tested with older JDK versions. In order for JavaFX to be able to use 
newer JDK features, such as code snippets (in API docs), record 
patterns, pattern matching for switch statements, and so forth, we need 
to increase the minimum version of the JDK that can run the latest 
JavaFX. Additionally, there is an ongoing cost to keeping JavaFX 
buildable and runnable on older versions of Java, and very little reason 
to continue to do so.


A question was raised [1] as to whether we should go even further and, 
once JDK 22 is released, jump straight to JDK 22 as a  minimum. While we 
could do that, I feel that there isn't sufficient justification for this 
at this time, although we could reconsider for next release.


To this end, I propose to bump the minimum version of the JDK needed to 
run JavaFX 23 to JDK 21. I filed JDK-8321603 [2] to track this and 
prepared PR  #1370 [3] (I've moved the PR back to Draft, pending this 
discussion). This will not affect update releases of earlier versions of 
JavaFX (e.g., JavaFX 21.0.NN or JavaFX 17.0.NN), which will continue to 
run with the same minimum JDK that they run on today.


As a reminder, we only assure that JavaFX NN will run with JDK NN-1 or 
later, although in practice, we don't bump the minimum required JDK 
version without a good reason. For example, while JavaFX 22 is built 
using JDK 21 as the boot JDK, it produces class files that will run with 
JDK 17, using "--release 17". The proposed change discussed here would 
update that in JavaFX 23 to "--release 21".


NOTE: this will not be an invitation to do wholesale refactoring of 
existing classes or methods to use newer language features (e.g., a PR 
that refactors existing switch statements and switch expressions into 
pattern-matching switch expressions would not be welcome). Rather, this 
can be seen as enabling judicious use of new features in new code, much 
as we did when we started allowing the use of "var", records, and 
pattern-matching instanceof.


Comments are welcome.

-- Kevin

[1] https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044081.html
[2] https://bugs.openjdk.org/browse/JDK-8321603
[3] https://github.com/openjdk/jfx/pull/1370



RFR: 8325591: [Mac] DRAG_DONE reports null transferMode when destination is external

2024-02-16 Thread Martin Fox
At the end of a drag operation the Mac Glass code sends out a DRAG_DONE event 
using the operation mask tracked in the GlassDragSource to determine the final 
transfer mode. That mask is only updated when a window in the JavaFX app is the 
drop destination. If the drag moves to an external destination the mask is set 
to NONE. If the drag terminates in the external destination that NONE forms the 
basis of the transfer mode sent via the DRAG_DONE event.

At the very end of the drag the OS calls the NSDraggingSource 
(GlassDraggingSource) with the final drag operation. This PR issues the 
DRAG_DONE from that callback so it can get the final transfer mode correct for 
both internal and external destinations.

-

Commit messages:
 - Using OS to determine DRAG_DONE operation

Changes: https://git.openjdk.org/jfx/pull/1371/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=1371&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8325591
  Stats: 29 lines in 5 files changed: 20 ins; 1 del; 8 mod
  Patch: https://git.openjdk.org/jfx/pull/1371.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/1371/head:pull/1371

PR: https://git.openjdk.org/jfx/pull/1371


[jfx22u] RFR: Merge jfx:jfx22

2024-02-16 Thread Kevin Rushforth
Clean merge of `jfx:jfx22` to `jfx22u:master`

-

Commit messages:
 - Merge jfx:master
 - 8325873: Update JDK_DOCS property to point to JDK 21 docs

The merge commit only contains trivial merges, so no merge-specific webrevs 
have been generated.

Changes: https://git.openjdk.org/jfx22u/pull/13/files
  Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jfx22u/pull/13.diff
  Fetch: git fetch https://git.openjdk.org/jfx22u.git pull/13/head:pull/13

PR: https://git.openjdk.org/jfx22u/pull/13


[jfx22u] Integrated: Merge jfx:jfx22

2024-02-16 Thread Kevin Rushforth
On Fri, 16 Feb 2024 22:47:52 GMT, Kevin Rushforth  wrote:

> Clean merge of `jfx:jfx22` to `jfx22u:master`

This pull request has now been integrated.

Changeset: 9d0592f5
Author:Kevin Rushforth 
URL:   
https://git.openjdk.org/jfx22u/commit/9d0592f5a44cad789e92e6bff218bc9606272c1f
Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod

Merge

-

PR: https://git.openjdk.org/jfx22u/pull/13