Re: RFR: 8260528: Clean glass-gtk sizing and positioning code [v47]
> 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]
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.
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]
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)
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)
.. 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.
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)
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)
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]
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]
> 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]
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]
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]
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]
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)
> > 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
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 >