Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel [v2]
On Mon, 21 Feb 2022 12:35:34 GMT, eduardsdv wrote: >> If the InputMethod's node is not in the scene, the default text location >> point is returned. > > eduardsdv has updated the pull request incrementally with one additional > commit since the last revision: > >8281953: Format TextInputControlSkinTest Marked as reviewed by aghaisas (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/735
Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel [v2]
On Mon, 21 Feb 2022 11:30:17 GMT, Ajit Ghaisas wrote: >> eduardsdv has updated the pull request incrementally with one additional >> commit since the last revision: >> >>8281953: Format TextInputControlSkinTest > > modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TextInputControlSkinTest.java > line 105: > >> 103: // and that the default point is returned. >> 104: Point2D point = >> textField.getInputMethodRequests().getTextLocation(0); >> 105: assertEquals(new Point2D(0,0), point); > > Very minor : Please add a space between `0,0` I added the space and checked the fix again with the sample program. The fix works, the NullPointer does not occur anymore. - PR: https://git.openjdk.java.net/jfx/pull/735
Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel [v2]
> If the InputMethod's node is not in the scene, the default text location > point is returned. eduardsdv has updated the pull request incrementally with one additional commit since the last revision: 8281953: Format TextInputControlSkinTest - Changes: - all: https://git.openjdk.java.net/jfx/pull/735/files - new: https://git.openjdk.java.net/jfx/pull/735/files/c9c66b47..755444da Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx=735=01 - incr: https://webrevs.openjdk.java.net/?repo=jfx=735=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/735.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/735/head:pull/735 PR: https://git.openjdk.java.net/jfx/pull/735
Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel
On Thu, 17 Feb 2022 12:57:27 GMT, eduardsdv wrote: > If the InputMethod's node is not in the scene, the default text location > point is returned. The fix looks good. I thought about returning `null` if either the scene or the window is null. As we need to fix this corner case, whether returning `null` or the default point (as proposed in this PR) does not matter. Both approaches will work. - Have you verified the sample program attached to the JBS runs successfully with your fix? - I have a very minor formatting comment on the test. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TextInputControlSkinTest.java line 105: > 103: // and that the default point is returned. > 104: Point2D point = > textField.getInputMethodRequests().getTextLocation(0); > 105: assertEquals(new Point2D(0,0), point); Very minor : Please add a space between `0,0` - PR: https://git.openjdk.java.net/jfx/pull/735
Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel
On Mon, 21 Feb 2022 07:27:46 GMT, eduardsdv wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextInputControlSkin.java
>> line 340:
>>
>>> 338: if (window == null) {
>>> 339: return new Point2D(0, 0);
>>> 340: }
>>
>> As long as each scene needs to be located in a window, the following will
>> simplify the code:
>> Suggestion:
>>
>> if (scene == null) {
>> return new Point2D(0, 0);
>> }
>> Window window = scene.getWindow();
>>
>> Is it possible to have a scene without an assigned window?
>
> Since the NullPointer occurs here because the synchronization between JavaFx
> and AWT threads is not really possible in this case, we cannot ensure that
> this method is only called when the scene is assigned to a window.
> I would check here for null both the scene and the window.
What a pity. Then ignore this comment.
-
PR: https://git.openjdk.java.net/jfx/pull/735
Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel
On Sat, 19 Feb 2022 17:40:54 GMT, delvh wrote:
>> If the InputMethod's node is not in the scene, the default text location
>> point is returned.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextInputControlSkin.java
> line 340:
>
>> 338: if (window == null) {
>> 339: return new Point2D(0, 0);
>> 340: }
>
> As long as each scene needs to be located in a window, the following will
> simplify the code:
> Suggestion:
>
> if (scene == null) {
> return new Point2D(0, 0);
> }
> Window window = scene.getWindow();
>
> Is it possible to have a scene without an assigned window?
Since the NullPointer occurs here because the synchronization between JavaFx
and AWT threads is not really possible in this case, we cannot ensure that this
method is only called when the scene is assigned to a window.
I would check here for null both the scene and the window.
-
PR: https://git.openjdk.java.net/jfx/pull/735
Re: RFR: 8281953: NullPointer in InputMethod components in JFXPanel
On Thu, 17 Feb 2022 12:57:27 GMT, eduardsdv wrote:
> If the InputMethod's node is not in the scene, the default text location
> point is returned.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextInputControlSkin.java
line 340:
> 338: if (window == null) {
> 339: return new Point2D(0, 0);
> 340: }
As long as each scene needs to be located in a window, the following will
simplify the code:
Suggestion:
if (scene == null) {
return new Point2D(0, 0);
}
Window window = scene.getWindow();
Is it possible to have a scene without an assigned window?
-
PR: https://git.openjdk.java.net/jfx/pull/735
RFR: 8281953: NullPointer in InputMethod components in JFXPanel
If the InputMethod's node is not in the scene, the default text location point is returned. - Commit messages: - 8281953: NullPointer in InputMethod components in JFXPanel Changes: https://git.openjdk.java.net/jfx/pull/735/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx=735=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281953 Stats: 17 lines in 2 files changed: 16 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jfx/pull/735.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/735/head:pull/735 PR: https://git.openjdk.java.net/jfx/pull/735
