Re: RFR: 8260528: Clean glass-gtk sizing and positioning code [v47]

2023-02-12 Thread Thiago Milczarek Sayao
> This cleans size and positioning code, reducing special cases, code 
> complexity and size.
> 
> Changes:
> 
> - cached extents: 28, 1, 1, 1 are old defaults - modern gnome uses different 
> sizes. It does not assume any size because it varies - it does cache because 
> it's unlikely to vary on the same system - but if it does occur, it will only 
> waste a resize event.
> - window geometry, min/max size are centralized in 
> `update_window_constraints`;
> - Frame extents (the window decoration size used for "total window size"):
> - frame extents are received in `process_property_notify`;
> - removed quirks in java code;
> - When received, call `set_bounds` again to adjust the size (to account 
> decorations later received);
> - Removed `activate_window` because it's the same as focusing the window. 
> `gtk_window_present` will deiconify and focus it.
> - `ensure_window_size` was a quirk - removed;
> - `requested_bounds` removed - not used anymore;
> - `window_configure` incorporated in `set_bounds` with `gtk_window_move` and 
> `gtk_window_resize`;
> - `process_net_wm_property` is a work-around for Unity only (added a check if 
> Unity - but it can probably be removed at some point)
> - `restack` split in `to_front()` and `to_back()` to conform to managed code;
> - Set `gtk_window_set_focus_on_map` to FALSE because if TRUE the Window 
> Manager changes the window ordering in the "focus stealing" mechanism - this 
> makes possible to remove the quirk on `request_focus()`;
> - Note:  `geometry_get_*` and `geometry_set_*` moved location but unchanged.

Thiago Milczarek Sayao has updated the pull request incrementally with one 
additional commit since the last revision:

  Fix deiconify bug

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/915/files
  - new: https://git.openjdk.org/jfx/pull/915/files/ccd84d72..4360d952

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=915=46
 - incr: https://webrevs.openjdk.org/?repo=jfx=915=45-46

  Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jfx/pull/915.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/915/head:pull/915

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


Re: RFR: 8260528: Clean glass-gtk sizing and positioning code [v46]

2023-02-12 Thread Thiago Milczarek Sayao
On Mon, 23 Jan 2023 03:51:33 GMT, Thiago Milczarek Sayao  
wrote:

>> This cleans size and positioning code, reducing special cases, code 
>> complexity and size.
>> 
>> Changes:
>> 
>> - cached extents: 28, 1, 1, 1 are old defaults - modern gnome uses different 
>> sizes. It does not assume any size because it varies - it does cache because 
>> it's unlikely to vary on the same system - but if it does occur, it will 
>> only waste a resize event.
>> - window geometry, min/max size are centralized in 
>> `update_window_constraints`;
>> - Frame extents (the window decoration size used for "total window size"):
>> - frame extents are received in `process_property_notify`;
>> - removed quirks in java code;
>> - When received, call `set_bounds` again to adjust the size (to account 
>> decorations later received);
>> - Removed `activate_window` because it's the same as focusing the window. 
>> `gtk_window_present` will deiconify and focus it.
>> - `ensure_window_size` was a quirk - removed;
>> - `requested_bounds` removed - not used anymore;
>> - `window_configure` incorporated in `set_bounds` with `gtk_window_move` and 
>> `gtk_window_resize`;
>> - `process_net_wm_property` is a work-around for Unity only (added a check 
>> if Unity - but it can probably be removed at some point)
>> - `restack` split in `to_front()` and `to_back()` to conform to managed code;
>> - Set `gtk_window_set_focus_on_map` to FALSE because if TRUE the Window 
>> Manager changes the window ordering in the "focus stealing" mechanism - this 
>> makes possible to remove the quirk on `request_focus()`;
>> - Note:  `geometry_get_*` and `geometry_set_*` moved location but unchanged.
>
> Thiago Milczarek Sayao has updated the pull request incrementally with four 
> additional commits since the last revision:
> 
>  - Fix JDK-8089923
>  - Fix JDK-8089923
>  - Fix JDK-8089923
>  - Fix deiconify regression

Working on it - I think it's possible to fix the DeIconify bug too.

-

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


WebKit Crashes JVM when removing nodes from DOM on wrong thread.

2023-02-12 Thread Scott Palmer
I'm seeing a hard crash in native code that brings down the JVM when I
accidentally called removeChild on an element from a WebView Document while
not on the Platform thread.  While I know it's my error, bringing down the
JVM instead of throwing an exception seems wrong.

Should this be considered a bug or not?

Scott

With JavaFX 17:
Thread 50 Crashed:: Java: ForkJoinPool-1-worker-5
0   libjfxwebkit.dylib   0x14fa2ac33 WTFCrashWithInfo(int,
char const*, char const*, int) + 19
1   libjfxwebkit.dylib   0x14ea5b60d
WebCore::TimerBase::setNextFireTime(WTF::MonotonicTime) + 541
2   libjfxwebkit.dylib   0x14ee0a513
WebCore::RenderTreeBuilder::detachFromRenderElement(WebCore::RenderElement&,
WebCore::RenderObject&, WebCore::RenderTreeBuilder::WillBeDestroyed) + 179
3   libjfxwebkit.dylib   0x14ee09fa2
WebCore::RenderTreeBuilder::Block::detach(WebCore::RenderBlock&,
WebCore::RenderObject&,
WebCore::RenderTreeBuilder::CanCollapseAnonymousBlock) + 562
4   libjfxwebkit.dylib   0x14ee085ef
WebCore::RenderTreeBuilder::detach(WebCore::RenderElement&,
WebCore::RenderObject&,
WebCore::RenderTreeBuilder::CanCollapseAnonymousBlock) + 543
5   libjfxwebkit.dylib   0x14ee082ba
WebCore::RenderTreeBuilder::destroy(WebCore::RenderObject&) + 58
6   libjfxwebkit.dylib   0x14ee0bd57
WebCore::RenderTreeBuilder::destroyAndCleanUpAnonymousWrappers(WebCore::RenderObject&)
+ 263
7   libjfxwebkit.dylib   0x14ee19aae
WebCore::RenderTreeUpdater::tearDownRenderers(WebCore::Element&,
WebCore::RenderTreeUpdater::TeardownType,
WebCore::RenderTreeBuilder&)::$_7::operator()(unsigned int) const + 734
8   libjfxwebkit.dylib   0x14ee18c13
WebCore::RenderTreeUpdater::tearDownRenderers(WebCore::Element&,
WebCore::RenderTreeUpdater::TeardownType, WebCore::RenderTreeBuilder&) +
1171
9   libjfxwebkit.dylib   0x14ee196d1
WebCore::RenderTreeUpdater::tearDownRenderers(WebCore::Element&) + 65
10  libjfxwebkit.dylib   0x14e46db9c
WebCore::ContainerNode::removeBetween(WebCore::Node*, WebCore::Node*,
WebCore::Node&) + 108
11  libjfxwebkit.dylib   0x14e46ad44
WebCore::ContainerNode::removeChild(WebCore::Node&) + 324
12  libjfxwebkit.dylib   0x14e50e24b
WebCore::Node::removeChild(WebCore::Node&) + 43
13  libjfxwebkit.dylib   0x14d98deeb
Java_com_sun_webkit_dom_NodeImpl_removeChildImpl + 107
14  ??? 0x1203e753a ???
15  ??? 0x1203e335c ???
16  ??? 0x1203e36a2 ???
17  ??? 0x1203e342b ???
18  ??? 0x1203e342b ???
19  ??? 0x1203e388f ???
20  ??? 0x1203e342b ???
21  ??? 0x1203e3317 ???
22  ??? 0x1203e3317 ???
23  ??? 0x1203e342b ???
24  ??? 0x1203e3317 ???
25  ??? 0x1203e342b ???
26  ??? 0x1203dacc9 ???
27  libjvm.dylib 0x110790af6
JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*,
JavaThread*) + 710
28  libjvm.dylib 0x11078fb47
JavaCalls::call_virtual(JavaValue*, Klass*, Symbol*, Symbol*,
JavaCallArguments*, JavaThread*) + 327
29  libjvm.dylib 0x11078fc13
JavaCalls::call_virtual(JavaValue*, Handle, Klass*, Symbol*, Symbol*,
JavaThread*) + 99
30  libjvm.dylib 0x11083ab94
thread_entry(JavaThread*, JavaThread*) + 180
31  libjvm.dylib 0x110d164af
JavaThread::thread_main_inner() + 335
32  libjvm.dylib 0x110d1481f Thread::call_run() +
207
33  libjvm.dylib 0x110b1f898
thread_native_entry(Thread*) + 328
34  libsystem_pthread.dylib  0x7ff8062b4259 _pthread_start + 125
35  libsystem_pthread.dylib  0x7ff8062afc7b thread_start + 15


With JavaFX 19.0.2.1
Thread 48 Crashed:: Java: ForkJoinPool-1-worker-2
0   libjfxwebkit.dylib   0x14f2eb9f3 0x14d0d8000 + 35731955
1   libjfxwebkit.dylib   0x14e3744a6 0x14d0d8000 + 19514534
2   libjfxwebkit.dylib   0x14e747d49 0x14d0d8000 + 23526729
3   libjfxwebkit.dylib   0x14e747798 0x14d0d8000 + 23525272
4   libjfxwebkit.dylib   0x14e745b7f 0x14d0d8000 + 23518079
5   libjfxwebkit.dylib   0x14e745837 0x14d0d8000 + 23517239
6   libjfxwebkit.dylib   0x14e749766 0x14d0d8000 + 23533414
7   libjfxwebkit.dylib   0x14e757dc1 0x14d0d8000 + 23592385
8   libjfxwebkit.dylib   0x14e757033 0x14d0d8000 + 23588915
9   libjfxwebkit.dylib   0x14e757aa1 0x14d0d8000 + 

Re: RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages [v5]

2023-02-12 Thread John Hendrikx
On Sun, 12 Feb 2023 21:49:11 GMT, John Hendrikx  wrote:

>> Packages fixed:
>> - com.sun.javafx.binding
>> - com.sun.javafx.collections
>> - javafx.beans
>> - javafx.beans.binding
>> - javafx.collections
>> - javafx.collections.transformation
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix review comments

@arapte do you want to take a look still?

-

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


Re: Alt key sticks even after app loses focus (JDK-8090647)

2023-02-12 Thread John Hendrikx
Well, it is not a permanent solution, just a way to test to see if it 
actually solves your problem.


--John

On 12/02/2023 23:23, Pedro Duque Vieira wrote:
.. all in all of course, I'd prefer a solution that properly fixes 
this bug and not have to rely on hacks that are prone to break in the 
future or code that needs to circumvent encapsulation or the module 
system. So either one is unlikely to be a fix I'm happy with.


Thanks for considering submitting a PR to properly fix this issue!

Kind regards,



On Sun, Feb 12, 2023 at 10:15 PM Pedro Duque Vieira 
 wrote:


John,

Thank you very much for submitting a work around. Unfortunately,
since there were other higher priority bugs and features and we're
very close to a release, I wasn't allocated time to work on this
much more than sending a message to this mailing list to warn
of the existence of this bug.
Not sure when I can get back to this. I'll submit your suggestion
to the dev team and if someone is allocated time to do it, I'll
get back here and let you know if it works.

There's also this suggestion that I forgot to mention (I haven't
been able to test it though):

https://stackoverflow.com/questions/65765656/release-mnemonic-when-application-loses-focus

Thanks again, kind regards,


On Sun, Feb 12, 2023 at 10:06 PM John Hendrikx
 wrote:

I'm not sure if you are able to test this yourself, but I made
a fix for this problem.

You could potentially test it by copying the class
`javafx.scene.Scene` in your project (without changing the
package -- and if modules will allow it, I don't use them
personally) and then using this piece of code:

    private void setWindowFocused(boolean value) {
    windowFocused = value;

    Node node = getFocusOwner();
    if (node != null) {
    node.setFocusQuietly(windowFocused,
focusOwner.focusVisible);
    node.notifyFocusListeners();
    }

    if (windowFocused && accessible != null) {
accessible.sendNotification(AccessibleAttribute.FOCUS_NODE);
    }

    if (!windowFocused) {

getInternalEventDispatcher().getKeyboardShortcutsHandler().setMnemonicsDisplayEnabled(false);
    }
    }

The last three lines are what I added. A quick test on Windows
here shows that the mnemonics get disabled as soon as the
window loses focus, and when returning, they're not responding
as you'd expect.

I'll submit a PR as well.

--John

On 12/02/2023 15:52, Pedro Duque Vieira wrote:


The behavior on Windows is all over the place for different
applications.? I tested a few I've got running:


Notepad, Notepad++, Eclipse:


- Alt-down: Shows mnemonics on menu bar
- Alt-up: Highlights file menu on alt release
- Alt-tab: Shows mnemonics but doesn't highlight menu
when it loses
focus; when returning, mnemonics still highlighted, but
doesn't act on
them as menu not selected


-> Looks buggy


Thunderbird / Opera / Firefox:


- Alt-down: nothing
- Alt-up: shows menu bar (it is hidden normally)
- Alt-tab: works as expected, no highlighting


-> Looks well behaved


Explorer / Excel / Wordpad:


- Alt-down: nothing
- Alt-up: shows mnemonics
- Alt-tab: works as expected, no highlighting


-> Looks well behaved


Visual Studio Code:


- Alt-down: Shows mnemonics on menu bar
- Alt-up: Highlights file menu on alt release
- Alt-tab: Shows mnemonics, but hides them once it loses
focus; on
return doesn't show mnemonics


-> Looks well behaved


Chrome / IntelliJ:


-> Looks well behaved, doesn't react to alt presses in
any way


None of the applications tested reacted on a mnemonic key
after
regaining focus however, even though they may have them still
highlighted (which I think is a bug).


In my opinion, the behavior of notepad/notepad++/eclipse
is incorrect
(they need to hide the mnemonics on focus lost, like
Visual Studio Code
does, but they don't).


There seems to be two correct ways of handling mnemonics
in applications
that use them, either:


a) shows mnemonics immediately on alt-down, but hide them
on focus lost
(if the alt-down becomes an alt-tab, or probably any
other alt combination)


b) only show mnemonics on a naked alt-up



Re: Alt key sticks even after app loses focus (JDK-8090647)

2023-02-12 Thread Pedro Duque Vieira
.. all in all of course, I'd prefer a solution that properly fixes this bug
and not have to rely on hacks that are prone to break in the future or code
that needs to circumvent encapsulation or the module system. So either one
is unlikely to be a fix I'm happy with.

Thanks for considering submitting a PR to properly fix this issue!

Kind regards,



On Sun, Feb 12, 2023 at 10:15 PM Pedro Duque Vieira <
pedro.duquevie...@gmail.com> wrote:

> John,
>
> Thank you very much for submitting a work around. Unfortunately, since
> there were other higher priority bugs and features and we're very close to
> a release, I wasn't allocated time to work on this much more than sending a
> message to this mailing list to warn of the existence of this bug.
> Not sure when I can get back to this. I'll submit your suggestion to the
> dev team and if someone is allocated time to do it, I'll get back here and
> let you know if it works.
>
> There's also this suggestion that I forgot to mention (I haven't been able
> to test it though):
> https://stackoverflow.com/questions/65765656/release-mnemonic-when-application-loses-focus
>
> Thanks again, kind regards,
>
>
> On Sun, Feb 12, 2023 at 10:06 PM John Hendrikx 
> wrote:
>
>> I'm not sure if you are able to test this yourself, but I made a fix for
>> this problem.
>>
>> You could potentially test it by copying the class `javafx.scene.Scene`
>> in your project (without changing the package -- and if modules will allow
>> it, I don't use them personally) and then using this piece of code:
>>
>> private void setWindowFocused(boolean value) {
>> windowFocused = value;
>>
>> Node node = getFocusOwner();
>> if (node != null) {
>> node.setFocusQuietly(windowFocused, focusOwner.focusVisible);
>> node.notifyFocusListeners();
>> }
>>
>> if (windowFocused && accessible != null) {
>> accessible.sendNotification(AccessibleAttribute.FOCUS_NODE);
>> }
>>
>> if (!windowFocused) {
>>
>> getInternalEventDispatcher().getKeyboardShortcutsHandler().setMnemonicsDisplayEnabled(false);
>> }
>> }
>>
>> The last three lines are what I added. A quick test on Windows here shows
>> that the mnemonics get disabled as soon as the window loses focus, and when
>> returning, they're not responding as you'd expect.
>>
>> I'll submit a PR as well.
>>
>> --John
>> On 12/02/2023 15:52, Pedro Duque Vieira wrote:
>>
>> The behavior on Windows is all over the place for different
>>> applications.? I tested a few I've got running:
>>
>>
>> Notepad, Notepad++, Eclipse:
>>
>>
>> - Alt-down: Shows mnemonics on menu bar
>>> - Alt-up: Highlights file menu on alt release
>>> - Alt-tab: Shows mnemonics but doesn't highlight menu when it loses
>>> focus; when returning, mnemonics still highlighted, but doesn't act on
>>> them as menu not selected
>>
>>
>> -> Looks buggy
>>
>>
>> Thunderbird / Opera / Firefox:
>>
>>
>> - Alt-down: nothing
>>> - Alt-up: shows menu bar (it is hidden normally)
>>> - Alt-tab: works as expected, no highlighting
>>
>>
>> -> Looks well behaved
>>
>>
>> Explorer / Excel / Wordpad:
>>
>>
>> - Alt-down: nothing
>>> - Alt-up: shows mnemonics
>>> - Alt-tab: works as expected, no highlighting
>>
>>
>> -> Looks well behaved
>>
>>
>> Visual Studio Code:
>>
>>
>> - Alt-down: Shows mnemonics on menu bar
>>> - Alt-up: Highlights file menu on alt release
>>> - Alt-tab: Shows mnemonics, but hides them once it loses focus; on
>>> return doesn't show mnemonics
>>
>>
>> -> Looks well behaved
>>
>>
>> Chrome / IntelliJ:
>>
>>
>> -> Looks well behaved, doesn't react to alt presses in any way
>>
>>
>> None of the applications tested reacted on a mnemonic key after
>>> regaining focus however, even though they may have them still
>>> highlighted (which I think is a bug).
>>
>>
>> In my opinion, the behavior of notepad/notepad++/eclipse is incorrect
>>> (they need to hide the mnemonics on focus lost, like Visual Studio Code
>>> does, but they don't).
>>
>>
>> There seems to be two correct ways of handling mnemonics in applications
>>> that use them, either:
>>
>>
>> a) shows mnemonics immediately on alt-down, but hide them on focus lost
>>> (if the alt-down becomes an alt-tab, or probably any other alt
>>> combination)
>>
>>
>> b) only show mnemonics on a naked alt-up
>>
>>
>> Ticket JDK-8090647 mentions a spec that has been updated, but I can't
>>> find it.? It also mentions that the behavior for JavaFX should be what I
>>> described in a), so I think this is a bug that can simply be fixed.
>>
>>
>> --John
>>
>>
>>
>> Yap, there's quite different behaviors across apps. If you test on
>> windows 11 you'll get yet another set of different behaviors.
>>
>> But all in all, if you: alt+tab (without release) and alt+tab again so
>> your app regains focus, you'll have the mnemonic still activated, which, as
>> you say, sounds like a bug no matter the difference in behaviors across
>> apps.
>>
>> I agree with your 

RFR: JDK-8090647: Mnemonics : on windows we should cancel the underscore latch when an app loses focus.

2023-02-12 Thread John Hendrikx
This fix hides mnemonics when the Scene's Window loses focus.

Before integration, we need to make sure this is correct behavior for Mac and 
Linux as well. It is correct for Windows.

How to test:

- Create a control with a mnemonic
- Alt-tab to another window (mnemonics appear when alt is pressed)

Before:
- Window shows visible mnemonic still and reacts on them when returning

After:
- Window hides mnemonics when focus is lost and doesn't react on them when 
returning

-

Commit messages:
 - Mnemonics should be hidden when Scene's window loses focus

Changes: https://git.openjdk.org/jfx/pull/1030/files
 Webrev: https://webrevs.openjdk.org/?repo=jfx=1030=00
  Issue: https://bugs.openjdk.org/browse/JDK-8090647
  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.org/jfx/pull/1030.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/1030/head:pull/1030

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


Re: Alt key sticks even after app loses focus (JDK-8090647)

2023-02-12 Thread Pedro Duque Vieira
John,

Thank you very much for submitting a work around. Unfortunately, since
there were other higher priority bugs and features and we're very close to
a release, I wasn't allocated time to work on this much more than sending a
message to this mailing list to warn of the existence of this bug.
Not sure when I can get back to this. I'll submit your suggestion to the
dev team and if someone is allocated time to do it, I'll get back here and
let you know if it works.

There's also this suggestion that I forgot to mention (I haven't been able
to test it though):
https://stackoverflow.com/questions/65765656/release-mnemonic-when-application-loses-focus

Thanks again, kind regards,


On Sun, Feb 12, 2023 at 10:06 PM John Hendrikx 
wrote:

> I'm not sure if you are able to test this yourself, but I made a fix for
> this problem.
>
> You could potentially test it by copying the class `javafx.scene.Scene` in
> your project (without changing the package -- and if modules will allow it,
> I don't use them personally) and then using this piece of code:
>
> private void setWindowFocused(boolean value) {
> windowFocused = value;
>
> Node node = getFocusOwner();
> if (node != null) {
> node.setFocusQuietly(windowFocused, focusOwner.focusVisible);
> node.notifyFocusListeners();
> }
>
> if (windowFocused && accessible != null) {
> accessible.sendNotification(AccessibleAttribute.FOCUS_NODE);
> }
>
> if (!windowFocused) {
>
> getInternalEventDispatcher().getKeyboardShortcutsHandler().setMnemonicsDisplayEnabled(false);
> }
> }
>
> The last three lines are what I added. A quick test on Windows here shows
> that the mnemonics get disabled as soon as the window loses focus, and when
> returning, they're not responding as you'd expect.
>
> I'll submit a PR as well.
>
> --John
> On 12/02/2023 15:52, Pedro Duque Vieira wrote:
>
> The behavior on Windows is all over the place for different
>> applications.? I tested a few I've got running:
>
>
> Notepad, Notepad++, Eclipse:
>
>
> - Alt-down: Shows mnemonics on menu bar
>> - Alt-up: Highlights file menu on alt release
>> - Alt-tab: Shows mnemonics but doesn't highlight menu when it loses
>> focus; when returning, mnemonics still highlighted, but doesn't act on
>> them as menu not selected
>
>
> -> Looks buggy
>
>
> Thunderbird / Opera / Firefox:
>
>
> - Alt-down: nothing
>> - Alt-up: shows menu bar (it is hidden normally)
>> - Alt-tab: works as expected, no highlighting
>
>
> -> Looks well behaved
>
>
> Explorer / Excel / Wordpad:
>
>
> - Alt-down: nothing
>> - Alt-up: shows mnemonics
>> - Alt-tab: works as expected, no highlighting
>
>
> -> Looks well behaved
>
>
> Visual Studio Code:
>
>
> - Alt-down: Shows mnemonics on menu bar
>> - Alt-up: Highlights file menu on alt release
>> - Alt-tab: Shows mnemonics, but hides them once it loses focus; on
>> return doesn't show mnemonics
>
>
> -> Looks well behaved
>
>
> Chrome / IntelliJ:
>
>
> -> Looks well behaved, doesn't react to alt presses in any way
>
>
> None of the applications tested reacted on a mnemonic key after
>> regaining focus however, even though they may have them still
>> highlighted (which I think is a bug).
>
>
> In my opinion, the behavior of notepad/notepad++/eclipse is incorrect
>> (they need to hide the mnemonics on focus lost, like Visual Studio Code
>> does, but they don't).
>
>
> There seems to be two correct ways of handling mnemonics in applications
>> that use them, either:
>
>
> a) shows mnemonics immediately on alt-down, but hide them on focus lost
>> (if the alt-down becomes an alt-tab, or probably any other alt
>> combination)
>
>
> b) only show mnemonics on a naked alt-up
>
>
> Ticket JDK-8090647 mentions a spec that has been updated, but I can't
>> find it.? It also mentions that the behavior for JavaFX should be what I
>> described in a), so I think this is a bug that can simply be fixed.
>
>
> --John
>
>
>
> Yap, there's quite different behaviors across apps. If you test on windows
> 11 you'll get yet another set of different behaviors.
>
> But all in all, if you: alt+tab (without release) and alt+tab again so
> your app regains focus, you'll have the mnemonic still activated, which, as
> you say, sounds like a bug no matter the difference in behaviors across
> apps.
>
> I agree with your suggestion: "a".
>
> My client was quite disappointed and started to rant about javafx. It
> didn't help that we were hit by a couple other bugs (perhaps bad
> luck).  The fact that the bug was filed on 2013 (10 years later) and is
> still happening can be quite problematic.
> Perhaps it would make sense to review all bugs filed, giving highest
> priority to the bugs that were submitted longer ago?
>
> Thanks!
>
> --
> Pedro Duque Vieira - https://www.pixelduke.com
>
>

-- 
Pedro Duque Vieira - https://www.pixelduke.com


Re: Alt key sticks even after app loses focus (JDK-8090647)

2023-02-12 Thread John Hendrikx
I'm not sure if you are able to test this yourself, but I made a fix for 
this problem.


You could potentially test it by copying the class `javafx.scene.Scene` 
in your project (without changing the package -- and if modules will 
allow it, I don't use them personally) and then using this piece of code:


    private void setWindowFocused(boolean value) {
    windowFocused = value;

    Node node = getFocusOwner();
    if (node != null) {
    node.setFocusQuietly(windowFocused, focusOwner.focusVisible);
    node.notifyFocusListeners();
    }

    if (windowFocused && accessible != null) {
accessible.sendNotification(AccessibleAttribute.FOCUS_NODE);
    }

    if (!windowFocused) {
getInternalEventDispatcher().getKeyboardShortcutsHandler().setMnemonicsDisplayEnabled(false);
    }
    }

The last three lines are what I added. A quick test on Windows here 
shows that the mnemonics get disabled as soon as the window loses focus, 
and when returning, they're not responding as you'd expect.


I'll submit a PR as well.

--John

On 12/02/2023 15:52, Pedro Duque Vieira wrote:


The behavior on Windows is all over the place for different
applications.? I tested a few I've got running:


Notepad, Notepad++, Eclipse:


- Alt-down: Shows mnemonics on menu bar
- Alt-up: Highlights file menu on alt release
- Alt-tab: Shows mnemonics but doesn't highlight menu when it loses
focus; when returning, mnemonics still highlighted, but doesn't act on
them as menu not selected


-> Looks buggy


Thunderbird / Opera / Firefox:


- Alt-down: nothing
- Alt-up: shows menu bar (it is hidden normally)
- Alt-tab: works as expected, no highlighting


-> Looks well behaved


Explorer / Excel / Wordpad:


- Alt-down: nothing
- Alt-up: shows mnemonics
- Alt-tab: works as expected, no highlighting


-> Looks well behaved


Visual Studio Code:


- Alt-down: Shows mnemonics on menu bar
- Alt-up: Highlights file menu on alt release
- Alt-tab: Shows mnemonics, but hides them once it loses focus; on
return doesn't show mnemonics


-> Looks well behaved


Chrome / IntelliJ:


-> Looks well behaved, doesn't react to alt presses in any way


None of the applications tested reacted on a mnemonic key after
regaining focus however, even though they may have them still
highlighted (which I think is a bug).


In my opinion, the behavior of notepad/notepad++/eclipse is incorrect
(they need to hide the mnemonics on focus lost, like Visual Studio
Code
does, but they don't).


There seems to be two correct ways of handling mnemonics in
applications
that use them, either:


a) shows mnemonics immediately on alt-down, but hide them on focus
lost
(if the alt-down becomes an alt-tab, or probably any other alt
combination)


b) only show mnemonics on a naked alt-up


Ticket JDK-8090647 mentions a spec that has been updated, but I can't
find it.? It also mentions that the behavior for JavaFX should be
what I
described in a), so I think this is a bug that can simply be fixed.


--John



Yap, there's quite different behaviors across apps. If you test on 
windows 11 you'll get yet another set of different behaviors.


But all in all, if you: alt+tab (without release) and alt+tab again so 
your app regains focus, you'll have the mnemonic still activated, 
which, as you say, sounds like a bug no matter the difference in 
behaviors across apps.


I agree with your suggestion: "a".

My client was quite disappointed and started to rant about javafx. It 
didn't help that we were hit by a couple other bugs (perhaps bad 
luck).  The fact that the bug was filed on 2013 (10 years later) and 
is still happening can be quite problematic.
Perhaps it would make sense to review all bugs filed, giving highest 
priority to the bugs that were submitted longer ago?


Thanks!

--
Pedro Duque Vieira - https://www.pixelduke.com

Re: RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages [v5]

2023-02-12 Thread Nir Lisker
On Sun, 12 Feb 2023 21:49:11 GMT, John Hendrikx  wrote:

>> Packages fixed:
>> - com.sun.javafx.binding
>> - com.sun.javafx.collections
>> - javafx.beans
>> - javafx.beans.binding
>> - javafx.collections
>> - javafx.collections.transformation
>
> John Hendrikx has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fix review comments

Marked as reviewed by nlisker (Reviewer).

-

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


Re: RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages [v5]

2023-02-12 Thread John Hendrikx
> Packages fixed:
> - com.sun.javafx.binding
> - com.sun.javafx.collections
> - javafx.beans
> - javafx.beans.binding
> - javafx.collections
> - javafx.collections.transformation

John Hendrikx has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix review comments

-

Changes:
  - all: https://git.openjdk.org/jfx/pull/972/files
  - new: https://git.openjdk.org/jfx/pull/972/files/8814a990..40abc6e6

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jfx=972=04
 - incr: https://webrevs.openjdk.org/?repo=jfx=972=03-04

  Stats: 52 lines in 2 files changed: 17 ins; 24 del; 11 mod
  Patch: https://git.openjdk.org/jfx/pull/972.diff
  Fetch: git fetch https://git.openjdk.org/jfx pull/972/head:pull/972

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


Re: RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages [v5]

2023-02-12 Thread John Hendrikx
On Sun, 12 Feb 2023 18:35:40 GMT, Nir Lisker  wrote:

>> Can make these consistent if the approach is agreed upon.
>
> So let's make the list and the set use an instance singleton pattern, like 
> the empty list does.

Fixed

-

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


Re: RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages [v4]

2023-02-12 Thread John Hendrikx
On Sun, 12 Feb 2023 16:31:35 GMT, Nir Lisker  wrote:

>> John Hendrikx 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 seven additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' of https://git.openjdk.org/jfx into 
>> feature/fix-raw-type-warnings-in-base-2
>>  - Undo changes in SortHelper
>>  - Simplify MappingChange by using Function
>>  - Clean up expression classes repeated null checks
>>  - Use instanceof with pattern matching in a few spots
>>  - Use static method for no-op map
>>  - Fix raw type warnings in base
>>
>>Packages fixed:
>>- com.sun.javafx.binding
>>- com.sun.javafx.collections
>>- javafx.beans
>>- javafx.beans.binding
>>- javafx.collections
>>- javafx.collections.transformation
>
> modules/javafx.base/src/main/java/javafx/collections/transformation/FilteredList.java
>  line 133:
> 
>> 131: return getPredicate();
>> 132: }
>> 133: return (Predicate) ALWAYS_TRUE;
> 
> Do we actually need the constant `ALWAYS_TRUE`? I think that just inlining 
> `return e -> true;` is fine and we can remove the field, the cast, and the 
> warning suppression. What do you think?

I guess this is fine, as for example `Function#identity()` also just returns `t 
-> t` instead of using a static final.

-

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


Re: RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages [v4]

2023-02-12 Thread Nir Lisker
On Sun, 1 Jan 2023 15:25:01 GMT, John Hendrikx  wrote:

>> modules/javafx.base/src/main/java/javafx/collections/FXCollections.java line 
>> 1640:
>> 
>>> 1638: @Override
>>> 1639: public Iterator iterator() {
>>> 1640: return new Iterator<>() {
>> 
>> Here the empty `Set` creates a listener on invocation, unlike in the list 
>> case. Might want to keep a single pattern. I prefer the one with a singleton 
>> iterator because the empty set itself is a singleton. Same comment about 
>> considering "inlining" it.
>
> Can make these consistent if the approach is agreed upon.

So let's make the list and the set use an instance singleton pattern, like the 
empty list does.

-

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


Re: RFR: JDK-8298528: Clean up raw type warnings in base in bindings and collections packages [v4]

2023-02-12 Thread Nir Lisker
On Wed, 8 Feb 2023 15:05:46 GMT, John Hendrikx  wrote:

>> Packages fixed:
>> - com.sun.javafx.binding
>> - com.sun.javafx.collections
>> - javafx.beans
>> - javafx.beans.binding
>> - javafx.collections
>> - javafx.collections.transformation
>
> John Hendrikx 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 seven additional 
> commits since the last revision:
> 
>  - Merge branch 'master' of https://git.openjdk.org/jfx into 
> feature/fix-raw-type-warnings-in-base-2
>  - Undo changes in SortHelper
>  - Simplify MappingChange by using Function
>  - Clean up expression classes repeated null checks
>  - Use instanceof with pattern matching in a few spots
>  - Use static method for no-op map
>  - Fix raw type warnings in base
>
>Packages fixed:
>- com.sun.javafx.binding
>- com.sun.javafx.collections
>- javafx.beans
>- javafx.beans.binding
>- javafx.collections
>- javafx.collections.transformation

modules/javafx.base/src/main/java/javafx/collections/FXCollections.java line 
235:

> 233: }
> 234: 
> 235: private static ObservableMap EMPTY_OBSERVABLE_MAP = new 
> EmptyObservableMap<>();

This can be a `final` field. Same for the list and set ones. Not sure why it 
wasn't declared as such from the start.

modules/javafx.base/src/main/java/javafx/collections/transformation/FilteredList.java
 line 133:

> 131: return getPredicate();
> 132: }
> 133: return (Predicate) ALWAYS_TRUE;

Do we actually need the constant `ALWAYS_TRUE`? I think that just inlining 
`return e -> true;` is fine and we can remove the field, the cast, and the 
warning suppression. What do you think?

-

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


Re: Alt key sticks even after app loses focus (JDK-8090647)

2023-02-12 Thread Pedro Duque Vieira
>
> The behavior on Windows is all over the place for different
> applications.? I tested a few I've got running:


Notepad, Notepad++, Eclipse:


- Alt-down: Shows mnemonics on menu bar
> - Alt-up: Highlights file menu on alt release
> - Alt-tab: Shows mnemonics but doesn't highlight menu when it loses
> focus; when returning, mnemonics still highlighted, but doesn't act on
> them as menu not selected


-> Looks buggy


Thunderbird / Opera / Firefox:


- Alt-down: nothing
> - Alt-up: shows menu bar (it is hidden normally)
> - Alt-tab: works as expected, no highlighting


-> Looks well behaved


Explorer / Excel / Wordpad:


- Alt-down: nothing
> - Alt-up: shows mnemonics
> - Alt-tab: works as expected, no highlighting


-> Looks well behaved


Visual Studio Code:


- Alt-down: Shows mnemonics on menu bar
> - Alt-up: Highlights file menu on alt release
> - Alt-tab: Shows mnemonics, but hides them once it loses focus; on
> return doesn't show mnemonics


-> Looks well behaved


Chrome / IntelliJ:


-> Looks well behaved, doesn't react to alt presses in any way


None of the applications tested reacted on a mnemonic key after
> regaining focus however, even though they may have them still
> highlighted (which I think is a bug).


In my opinion, the behavior of notepad/notepad++/eclipse is incorrect
> (they need to hide the mnemonics on focus lost, like Visual Studio Code
> does, but they don't).


There seems to be two correct ways of handling mnemonics in applications
> that use them, either:


a) shows mnemonics immediately on alt-down, but hide them on focus lost
> (if the alt-down becomes an alt-tab, or probably any other alt combination)


b) only show mnemonics on a naked alt-up


Ticket JDK-8090647 mentions a spec that has been updated, but I can't
> find it.? It also mentions that the behavior for JavaFX should be what I
> described in a), so I think this is a bug that can simply be fixed.


--John



Yap, there's quite different behaviors across apps. If you test on windows
11 you'll get yet another set of different behaviors.

But all in all, if you: alt+tab (without release) and alt+tab again so your
app regains focus, you'll have the mnemonic still activated, which, as you
say, sounds like a bug no matter the difference in behaviors across apps.

I agree with your suggestion: "a".

My client was quite disappointed and started to rant about javafx. It
didn't help that we were hit by a couple other bugs (perhaps bad
luck).  The fact that the bug was filed on 2013 (10 years later) and is
still happening can be quite problematic.
Perhaps it would make sense to review all bugs filed, giving highest
priority to the bugs that were submitted longer ago?

Thanks!

-- 
Pedro Duque Vieira - https://www.pixelduke.com


Re: Enhance javafx Canvas API

2023-02-12 Thread Laurent Bourgès
Hi,

For jfree.FXGraphics2D, I implemented the proposed approach in the new
GCHandler:
https://github.com/jfree/fxgraphics2d/blob/1994b9cb3a9bff93bf95100d398756d7f9f626c2/src/main/java/org/jfree/fx/GCStateHandler.java

It boosts fxG2D performance & correctness.

Ideally, such methods getSavePoint(), restoreToCount() should be added to
GraphicsContext API...

Cheers,
Laurent

Le ven. 3 févr. 2023, 23:31, Laurent Bourgès  a
écrit :

> Hi,
>
> While working with skia & jfree's skijaGraphics2D wrapper, I observed skia
> offers nice methods in SkCanvas missing in javafx Canvas/GraphicsContext to
> deal with save/restore state:
>
> - int getSaveCount ()
>   Returns the number of saved states, each containing: SkMatrix and clip.
>
> - void restoreToCount (int saveCount)
>   Restores state to SkMatrix and clip values when save(), returned
> saveCount.
>
> As javafx canvas also maintains s stack of saved states, such methods are
> trivial to implement to allow resetting completely the canvas state by
> restoreToCount(0).
>
> Moreover, clip(shape) + save/restore are really tricky to allows ensure
> clip is reset to previous state ...
>
> Would you agree to add such methods to have more predictive behaviour than
> counting all save() calls to ensure to call restore() the same number of
> times to reset the canvas to its initial state ?
>
> See SkijaGraphics2d implementation that saves / restore skCanvas clip
> using such index property:
>
> https://github.com/jfree/skijagraphics2d/blob/main/src/main/java/org/jfree/skija/SkijaGraphics2D.java
>
> Cheers,
> Laurent
>