On Sat, 16 May 2026 15:01:17 GMT, Christopher Schnick <[email protected]> 
wrote:

>> This is a follow-up to the combobox converter PR. It seems like the 
>> updateDisplayNode method in the Skin does not properly handle null cases for 
>> when null is an item in the combobox. It probably incorrectly assumes that 
>> any null values is mapped to null by the stringconverter. However, if you 
>> create a custom converter, a null value can have a non-null string value.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Christopher Schnick has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Move setItem call

The test passes with the fix and fails without.  I do have a question however:

Since a reproducer is missing from the JBS ticket, what is expected of this 
change, visually?  To illustrate, first grab the latest monkey tester where I 
modified the "Quoted" converter to handle null as `◇` symbol.

With a null value selected from the pulldown, I am expecting to see ◇ in the 
text field, right?  But instead the text field is empty.  Yet when I enter an 
empty text by clearing the text field and pressing ENTER, I do get `""` as 
expected.

I don't see any changes in behavior between the master and this PR.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ComboBoxListViewSkin.java
 line 375:

> 373:             buttonCell.pseudoClassStateChanged(PSEUDO_CLASS_EMPTY,    
> empty);
> 374:             buttonCell.pseudoClassStateChanged(PSEUDO_CLASS_FILLED,   
> !empty);
> 375:             buttonCell.pseudoClassStateChanged(PSEUDO_CLASS_SELECTED, 
> true);

this is not exactly an equivalent change, since we are updating pseudostyles 
(esp. PSEUDO_CLASS_SELECTED, true).  is it important?  probably not: we'll end 
up with pseudostyles set on something not connected to the scene graph, 
probably not a big deal.

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

PR Review: https://git.openjdk.org/jfx/pull/2169#pullrequestreview-4323438522
PR Review Comment: https://git.openjdk.org/jfx/pull/2169#discussion_r3269831591

Reply via email to