Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v3]
On Wed, 15 Dec 2021 11:43:36 GMT, John Hendrikx wrote:
>> This is an implementation of the proposal in
>> https://bugs.openjdk.java.net/browse/JDK-8274771 that me and Nir Lisker
>> (@nlisker) have been working on. It's a complete implementation including
>> good test coverage.
>>
>> This was based on https://github.com/openjdk/jfx/pull/434 but with a smaller
>> API footprint. Compared to the PoC this is lacking public API for
>> subscriptions, and does not include `orElseGet` or the `conditionOn`
>> conditional mapping.
>>
>> **Flexible Mappings**
>> Map the contents of a property any way you like with `map`, or map nested
>> properties with `flatMap`.
>>
>> **Lazy**
>> The bindings created are lazy, which means they are always _invalid_ when
>> not themselves observed. This allows for easier garbage collection (once the
>> last observer is removed, a chain of bindings will stop observing their
>> parents) and less listener management when dealing with nested properties.
>> Furthermore, this allows inclusion of such bindings in classes such as
>> `Node` without listeners being created when the binding itself is not used
>> (this would allow for the inclusion of a `treeShowingProperty` in `Node`
>> without creating excessive listeners, see this fix I did in an earlier PR:
>> https://github.com/openjdk/jfx/pull/185)
>>
>> **Null Safe**
>> The `map` and `flatMap` methods are skipped, similar to `java.util.Optional`
>> when the value they would be mapping is `null`. This makes mapping nested
>> properties with `flatMap` trivial as the `null` case does not need to be
>> taken into account in a chain like this:
>> `node.sceneProperty().flatMap(Scene::windowProperty).flatMap(Window::showingProperty)`.
>> Instead a default can be provided with `orElse`.
>>
>> Some examples:
>>
>> void mapProperty() {
>> // Standard JavaFX:
>> label.textProperty().bind(Bindings.createStringBinding(() ->
>> text.getValueSafe().toUpperCase(), text));
>>
>> // Fluent: much more compact, no need to handle null
>> label.textProperty().bind(text.map(String::toUpperCase));
>> }
>>
>> void calculateCharactersLeft() {
>> // Standard JavaFX:
>>
>> label.textProperty().bind(text.length().negate().add(100).asString().concat("
>> characters left"));
>>
>> // Fluent: slightly more compact and more clear (no negate needed)
>> label.textProperty().bind(text.orElse("").map(v -> 100 - v.length() +
>> " characters left"));
>> }
>>
>> void mapNestedValue() {
>> // Standard JavaFX:
>> label.textProperty().bind(Bindings.createStringBinding(
>> () -> employee.get() == null ? ""
>> : employee.get().getCompany() == null ? ""
>> : employee.get().getCompany().getName(),
>> employee
>> ));
>>
>> // Fluent: no need to handle nulls everywhere
>> label.textProperty().bind(
>> employee.map(Employee::getCompany)
>> .map(Company::getName)
>> .orElse("")
>> );
>> }
>>
>> void mapNestedProperty() {
>> // Standard JavaFX:
>> label.textProperty().bind(
>> Bindings.when(Bindings.selectBoolean(label.sceneProperty(),
>> "window", "showing"))
>> .then("Visible")
>> .otherwise("Not Visible")
>> );
>>
>> // Fluent: type safe
>> label.textProperty().bind(label.sceneProperty()
>> .flatMap(Scene::windowProperty)
>> .flatMap(Window::showingProperty)
>> .orElse(false)
>> .map(showing -> showing ? "Visible" : "Not Visible")
>> );
>> }
>>
>> Note that this is based on ideas in ReactFX and my own experiments in
>> https://github.com/hjohn/hs.jfx.eventstream. I've come to the conclusion
>> that this is much better directly integrated into JavaFX, and I'm hoping
>> this proof of concept will be able to move such an effort forward.
>
> John Hendrikx has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Upgrade tests to JUnit 5
I will start my review this week.
-
PR: https://git.openjdk.java.net/jfx/pull/675
Re: RFR: 8242544: CMD+ENTER key event crashes the application when invoked on dialog
On Wed, 29 Dec 2021 00:44:44 GMT, Andreas Heger wrote: > This also solves [JDK-8205915 [macOS] Accelerator assigned to button in > dialog fires menuItem in owning > stage](https://bugs.openjdk.java.net/browse/JDK-8205915l). > > I must admit that I don't have 100% insight about what actually caused the > problems and how the event flow completely works. > > In MacOS, key events with a command or control modifier are handled via the > performKeyEquivalent method, first. If no view or window handles such a > shortcut key event (indicated by a return value NO in performKeyEquivalent), > the key event is sent a second time via the sendEvent method. Before this > fix, the method **sendJavaKeyEvent** in **GlassViewDelegate.m** seemed to be > called twice: first by performKeyEquivalent and then by sendEvent, since no > responder returned YES in performKeyEquivalent. For not handling the same > event twice in JFX, there seemed to be a workaround in **sendJavaKeyEvent** > in **GlassViewDelegate.m**. It only handled the first call. The second call > checked if the given event was exactly the same like in the last call. In > this case it simply returned and did not forward the event. This means that > all key events with a CMD or CTRL modifier were handled in JFX in the > performKeyEquivalent event loop, even though they actually weren't used by > the MacOS keyEquivale nt functionality and all responders returned NO in performKeyEquivalent. So MacOS sent the event again to all responders in the sendEvent loop. I assume that the window has been destroyed already, when MacOS tried to send the second event to it, because the closing was handled in the first call. Sending an event to a no longer existing window probably caused the crash by an unhandled exception. > > It seems that this problem existed in the past and there was a workaround in > **GlassWindow.m**. In the method **performKeyEquivalent**, it was checked if > the window was closed and if so, it returned YES. However, nowadays the > window closing check did not work... self->gWindow->isClosed returned NO when > I debugged it, although the window should be closed by the key event. > Returning YES in case of a closed window is not a clean solution, anyway, > because YES should mean that the shortcut key equivalent is handled. Another > point is that Apple writes that overwriting performKeyEquivalent in an > NSWindow subclass is discouraged. So, I completely deleted the method from > **GlassWindow.m**, since it only returned the value of the super function, > except for the non working closing check. > > Since the default version of performKeyEquivalent in NSWindow always returns > NO, only deleting the method from **GlassWindow.m** did not fix the crash. So > I tried to solve the problem that a shortcut key event is handled in the > performKeyEquivalent loop. The call of **sendJavaKeyEvent** in > **GlassViewDelegate.m** which is caused by a performKeyEquivalent should not > be handled, actually. So, I simply removed this call which is done in > **GlassView3D.m** **performKeyEquivalent** method. By removing the call, > there is also no longer any need to check if the same key event was passed to > **sendJavaKeyEvent** in **GlassViewDelegate.m**, so this check is also > removed. > > I'm curious about your comments and reviews and I wonder if this is a clean > solution. All my tests so far seemed to be fine. I can reproduce this with a small Mac app that does nothing but set the NSWindow's contentView to nil while handling `processKeyEvent:`. I'm not handling the event twice or destroying the NSWindow. The crash happens immediately after returning from my implementation of `processKeyEvent:` and back into Apple's code. There's something about Apple's implementation of this routine that's sensitive to having the contentView nulled out. I don't see the same crash if I set the contentView to nil inside a `keyDown:` event. I would love to get rid of our `performKeyEquivalent:` methods but we can't. JavaFX assumes that the focused Node will get the first opportunity to process a KeyEvent. For example, if a TextArea has the focus it gets the first opportunity to process `Cmd-A` to select all text. A KeyEvent should only bubble up to the top level menus if the focused node doesn't handle it. When using the system menubar on Mac this means we need to implement `processKeyEquivalent:` to ensure that we can hand the key event to the focused JavaFX node before it gets handled by the main NSMenu. Unfortunately there's currently no way to determine that the event was consumed by the node so the Mac glass code hands it over to the main menu anyway. This leads to bugs where one event triggers multiple actions like [JDK-8087863](https://bugs.openjdk.java.net/browse/JDK-8087863) and [JDK-8088897](https://bugs.openjdk.java.net/browse/JDK-8088897) and the ones listed over in javafxports. Most of the changes for f
RFR: 8279328: CssParser uses default charset instead of UTF-8
`CssParser.parse(URL)` is specified to assume UTF-8 file encoding, but (implicitly) uses the default charset to read the file, potentially resulting in incorrect interpretation of the file content. This can be fixed by explicitly specifying the UTF-8 charset for `InputStreamReader`. - Commit messages: - Use UTF-8 charset to parse stylesheets - Failing test Changes: https://git.openjdk.java.net/jfx/pull/705/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx&pr=705&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279328 Stats: 31 lines in 2 files changed: 30 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/705.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/705/head:pull/705 PR: https://git.openjdk.java.net/jfx/pull/705
Re: RFR: 8242544: CMD+ENTER key event crashes the application when invoked on dialog
On Wed, 29 Dec 2021 00:44:44 GMT, Andreas Heger wrote: > This also solves [JDK-8205915 [macOS] Accelerator assigned to button in > dialog fires menuItem in owning > stage](https://bugs.openjdk.java.net/browse/JDK-8205915l). > > I must admit that I don't have 100% insight about what actually caused the > problems and how the event flow completely works. > > In MacOS, key events with a command or control modifier are handled via the > performKeyEquivalent method, first. If no view or window handles such a > shortcut key event (indicated by a return value NO in performKeyEquivalent), > the key event is sent a second time via the sendEvent method. Before this > fix, the method **sendJavaKeyEvent** in **GlassViewDelegate.m** seemed to be > called twice: first by performKeyEquivalent and then by sendEvent, since no > responder returned YES in performKeyEquivalent. For not handling the same > event twice in JFX, there seemed to be a workaround in **sendJavaKeyEvent** > in **GlassViewDelegate.m**. It only handled the first call. The second call > checked if the given event was exactly the same like in the last call. In > this case it simply returned and did not forward the event. This means that > all key events with a CMD or CTRL modifier were handled in JFX in the > performKeyEquivalent event loop, even though they actually weren't used by > the MacOS keyEquivale nt functionality and all responders returned NO in performKeyEquivalent. So MacOS sent the event again to all responders in the sendEvent loop. I assume that the window has been destroyed already, when MacOS tried to send the second event to it, because the closing was handled in the first call. Sending an event to a no longer existing window probably caused the crash by an unhandled exception. > > It seems that this problem existed in the past and there was a workaround in > **GlassWindow.m**. In the method **performKeyEquivalent**, it was checked if > the window was closed and if so, it returned YES. However, nowadays the > window closing check did not work... self->gWindow->isClosed returned NO when > I debugged it, although the window should be closed by the key event. > Returning YES in case of a closed window is not a clean solution, anyway, > because YES should mean that the shortcut key equivalent is handled. Another > point is that Apple writes that overwriting performKeyEquivalent in an > NSWindow subclass is discouraged. So, I completely deleted the method from > **GlassWindow.m**, since it only returned the value of the super function, > except for the non working closing check. > > Since the default version of performKeyEquivalent in NSWindow always returns > NO, only deleting the method from **GlassWindow.m** did not fix the crash. So > I tried to solve the problem that a shortcut key event is handled in the > performKeyEquivalent loop. The call of **sendJavaKeyEvent** in > **GlassViewDelegate.m** which is caused by a performKeyEquivalent should not > be handled, actually. So, I simply removed this call which is done in > **GlassView3D.m** **performKeyEquivalent** method. By removing the call, > there is also no longer any need to check if the same key event was passed to > **sendJavaKeyEvent** in **GlassViewDelegate.m**, so this check is also > removed. > > I'm curious about your comments and reviews and I wonder if this is a clean > solution. All my tests so far seemed to be fine. This fix seems to also solve [https://github.com/javafxports/openjdk-jfx/issues/370](https://github.com/javafxports/openjdk-jfx/issues/370). - PR: https://git.openjdk.java.net/jfx/pull/704
