On Thu, 12 Sep 2024 11:09:49 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> Further additions to the MonkeyTester application:
>> 
>> - tree/table view column property sheets ✔
>> - new pages: button, checkbox, hyperlink, menu bar, radio button, toggle 
>> button, canvas, split menu button, dnd ✔
>> - properties... context menu ✔
>> - context menu: table columns, toolbar items ✔
>> - control.context menu option
>> - property monitor tool ✔
>> - accessibility logging ✔
>> 
>> - editable combobox for double values with pre-set list (e.g. Region: 
>> min/max width etc.)
>> - Page -> Snapped Split Panes menu option for the control container
>> 
>> This PR simply syncs up with the dev repository
>> https://github.com/andy-goryachev-oracle/MonkeyTest
>
> Providing a few comments observed when running MonkeyTester app.
> 
> 1. Z transformations Scale Z / Translate Z can be avoided for 2D controls / 
> shapes
> 2. TreeTableView height is fixed. It always shows 1 row. Changes in 
> TreeTableView->Root does not have any effect. Height of TTV could vary with 
> the number of rows.
> 3. Button: Button does not receive ENTER key press event when set as default 
> button.<br>
> 3.1 Check default button.<br>
> 3.2 Move focus on button by clicking on it
> <br>3.3 Press ENTER key<br>
> => button does not show pressed effect. Press space button, button shows 
> pressed effect.
> 4. ChoiceBox/ColorPicker/MenuBar  background cannot be set back to the 
> default. Changing back the background to <null> has no effect
> 5. No pages were getting rendered after following NPE was observed with 
> RadioButton sample:<br>Attaching the user preferences file with which issue 
> can be reproduced 
> [ui-settings.properties.zip](https://github.com/user-attachments/files/16978042/ui-settings.properties.zip)<br>
>  > Start MonkeyTester app with the provided user preferences file and 
> navigate to RadioButton page<br>=> NPE should be observed on terminal and 
> page does not render any controls. Navigating to other pages also does not 
> render any controls
> 
> 
> Exception in thread "JavaFX Application Thread" 
> java.lang.NullPointerException: Cannot invoke "javafx.geometry.Pos.getHpos()" 
> because the return value of "javafx.scene.control.RadioButton.getAlignment()" 
> is null
>       at 
> javafx.controls@24-internal/javafx.scene.control.skin.RadioButtonSkin.layoutChildren(RadioButtonSkin.java:136)
>       at 
> javafx.controls@24-internal/javafx.scene.control.Control.layoutChildren(Control.java:612)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1208)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at 
> javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent.java:1215)
>       at javafx.graphics@24-internal/javafx.scene.Parent.layout(Parent....

@arapte thank you for testing!

> 1. Z transformations Scale Z / Translate Z can be avoided for 2D controls / 
> shapes

I'd rather keep them, might be useful for testing (i.e. to ensure there is no 
change when setting these properties in 2D)

> * TreeTableView height is fixed. It always shows 1 row. Changes in 
> TreeTableView->Root does not have any effect. Height of TTV could vary with 
> the number of rows.

You probably need to make sure the "Root:" has a number of items, and click the 
root node to expand.

I did notice one thing though: setting the Background and the Fixed Cell Size 
33.4 results in a weird rendering (probably because of the fractional cell 
size).  Not sure if it's a bug though:

![Screenshot 2024-09-13 at 12 11 
55](https://github.com/user-attachments/assets/67483347-dfba-4371-98ca-5f6441f4bb21)


> * Button: Button does not receive ENTER key press event when set as default 
> button.

My understanding is that a default button creates an accelerator at the Scene 
level, so the button rendering is unaffected when ENTER is pressed.

Try Window -> Open Modal Window and hit ENTER.  No change.

> * ChoiceBox/ColorPicker/MenuBar  background cannot be set back to the 
> default. Changing back the background to  has no effect

I am not sure why this is happening - possibly the interaction with the skin.  
Setting background back to null does not resurrect the skin's visuals, neither 
setting a new skin with Skin -> Set New Skin.  Only re-creating the control by 
switching to a different page and back does the trick.

This could be a P5 product bug, possibly.

> * No pages were getting rendered after following NPE was observed with 
> RadioButton sample

So this is either a product bug or a case of incomplete documentation.  I've 
raised this issue before (see https://bugs.openjdk.org/browse/JDK-8334883), but 
basically some properties allow for null values and some do not.  The 
documentation is unclear.

In this particular case, we might have encountered a product bug: even though 
the code in Labeled:190 shows some attempt to deal with null property instance, 
it fails when the property _value_ is null:


    public final Pos getAlignment() { return alignment == null ? 
Pos.CENTER_LEFT : alignment.get(); }


so we need to either:
- throw NPE when setting a null value
- fix the getter to handle null value as some valid value
- consider documenting whether nulls are allowed in various javafx properties 

In any case, these are product issues and not the MT issue.  In fact, the MT 
serves its purpose of uncovering product issues, so thank you!

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1560#issuecomment-2350016125

Reply via email to