> Macroscopic issue is that initially, the toggle is not sync'ed to the 
> selection state. Root reason is an missing else
> block when updating toggle selection state (see report for details).
> Fixed by introducing the else block and removing all follow-up errors that 
> tried to amend the consequences of the
> incorrect selection state
> - removed listener to selected item
> - removed toggle selection update in showing listener
> 
> The former also fixed the memory leak when replacing the selectionModel plus 
> an unreported NPE when the selectionModel
> is null initially.
> Added tests that failed before the fix and passed after. As there had been no 
> tests around toggle state, so added some
> to verify that the change doesn't break. Enhanced shim/skin to allow access 
> to popup for testing. Removed the
> informally ignored test part for memory leak.

Jeanette Winzenburg has updated the pull request incrementally with one 
additional commit since the last revision:

  ChoiceBox/-Skin/-SelectionTest
  
  - added code comments to clarify test/implemenation intention

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

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/177/files
  - new: https://git.openjdk.java.net/jfx/pull/177/files/0d3ce123..01aa7901

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/177/webrev.02
 - incr: https://webrevs.openjdk.java.net/jfx/177/webrev.01-02

  Stats: 5 lines in 2 files changed: 4 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/177.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/177/head:pull/177

PR: https://git.openjdk.java.net/jfx/pull/177

Reply via email to