Re: RFR: 8301219: JavaFX crash when closing with the escape key [v3]
On Mon, 18 Dec 2023 19:15:03 GMT, Martin Fox wrote: >> While processing a key down event the Glass GTK code sends out PRESSED and >> TYPED KeyEvents back to back. If the stage is closed during the PRESSED >> event the code will end up referencing freed memory while sending out the >> TYPED event. This can lead to intermittent crashes. >> >> In GlassApplication.cpp the EventCounterHelper object ensures the >> WindowContext isn't deleted while processing an event. Currently the helper >> object is being created *after* IME handling instead of before. If the IME >> is enabled it's possible for the WindowContext to be deleted in the middle >> of executing a number of keyboard-related events. >> >> The fix is simple; instantiate the EventCounterHelper object earlier. There >> isn't always a WindowContext so I tweaked the EventCounterHelper to do >> nothing if the context is null. >> >> To make the crash more reproducible I altered the WindowContext such that >> when it's deleted the freed memory is filled with 0xCC. This made the crash >> more reproducible and allowed me to test the fix. I did the same with >> GlassView since that's the only other Glass GTK class that's instantiated >> with `new` and discarded with `delete`. > > Martin Fox has updated the pull request incrementally with one additional > commit since the last revision: > > Consistent use of FILL in mem debug code. Looks good. - Marked as reviewed by tsayao (Committer). PR Review: https://git.openjdk.org/jfx/pull/1307#pullrequestreview-1795582351
Re: RFR: 8320965: Scrolling on a touch enabled display fails on Wayland [v3]
On Mon, 18 Dec 2023 11:19:18 GMT, Jose Pereda wrote: >> This PR replaces the deprecated `gdk_pointer_grab` with `gdk_seat_grab`, and >> `gdk_pointer_ungrab ` with `gdk_seat_ungrab`, using runtime checks and >> wrapped functions for GTK 3.20+ (so systems without it still run with GTK >> 3.8+), and fixes the dragging issue on Wayland. > > Jose Pereda has updated the pull request incrementally with one additional > commit since the last revision: > > Add compile-time checks to GdkSeat Looks good. - Marked as reviewed by tsayao (Committer). PR Review: https://git.openjdk.org/jfx/pull/1305#pullrequestreview-1795582193
Re: RFR: 8305418: [Linux] Replace obsolete XIM as Input Method Editor
On Wed, 6 Sep 2023 16:41:24 GMT, Martin Fox wrote: >> This replaces obsolete XIM and uses gtk api for IME. >> Gtk uses [ibus](https://github.com/ibus/ibus) >> >> Gtk3+ uses relative positioning (as Wayland does), so I've added a Relative >> positioning on `InputMethodRequest`. >> >> [Screencast from 17-09-2023 >> 21:59:04.webm](https://github.com/openjdk/jfx/assets/30704286/6c398e39-55a3-4420-86a2-beff07b549d3) > > I ran into similar failures when I added a method to a core class but did not > add it to the Stub version of the same class. In my case I added a call to > Stage.java but didn't add it to SubStage.java. > > I'll be away from my computer for about a week and will take a closer look at > this when I get back. I did notice that when I press a dead key the caret > ends up at the beginning of the preview string when it should be at the end. > There's also an issue with the way keys on the numeric keypad are being > encoded. The fix is minor (I think) and I'll send details when I get back. > Beyond that I don't understand the IME machinery so here's hoping that > someone who does can spare some cycles to review this. There's a bug when closing the window with a key, as pointed by @beldenfox on #1307. - PR Comment: https://git.openjdk.org/jfx/pull/1080#issuecomment-1868527856
Re: RFR: 8301219: JavaFX crash when closing with the escape key [v3]
On Mon, 18 Dec 2023 19:15:03 GMT, Martin Fox wrote: >> While processing a key down event the Glass GTK code sends out PRESSED and >> TYPED KeyEvents back to back. If the stage is closed during the PRESSED >> event the code will end up referencing freed memory while sending out the >> TYPED event. This can lead to intermittent crashes. >> >> In GlassApplication.cpp the EventCounterHelper object ensures the >> WindowContext isn't deleted while processing an event. Currently the helper >> object is being created *after* IME handling instead of before. If the IME >> is enabled it's possible for the WindowContext to be deleted in the middle >> of executing a number of keyboard-related events. >> >> The fix is simple; instantiate the EventCounterHelper object earlier. There >> isn't always a WindowContext so I tweaked the EventCounterHelper to do >> nothing if the context is null. >> >> To make the crash more reproducible I altered the WindowContext such that >> when it's deleted the freed memory is filled with 0xCC. This made the crash >> more reproducible and allowed me to test the fix. I did the same with >> GlassView since that's the only other Glass GTK class that's instantiated >> with `new` and discarded with `delete`. > > Martin Fox has updated the pull request incrementally with one additional > commit since the last revision: > > Consistent use of FILL in mem debug code. The change looks correct to me - the change makes sense, but I can't reproduce the crash on my systems. I can confirm that it's reaching the `delete ctx` correctly. I didn't know about the `0xCC` debug trick. - PR Comment: https://git.openjdk.org/jfx/pull/1307#issuecomment-1868527442
