RFR: 8188026: TextFieldXXCell: NPE on calling startEdit [v5]

Marius Hanl mhanl at openjdk.java.net
Sun Sep 19 11:52:55 UTC 2021


> This PR sets an unified logic to every **startEdit()** method of all Cell implementations.
> So startEdit() is always doing the same now:
> 
> `super.startEdit();`
> `if (!isEditing()) {
> return;
> }` 
> 
> This will prevent a NPE while also being cleaner (no more double checks)
> The commits are splitted into 4 base commits:
> - First commit changes the startEdit() for TableCell implementations (8 files)
> - Second commit changes the startEdit() for TreeTableCell implementations (7 files)
> - Third commit changes the startEdit() for ListCell implementations (7 files)
> - Fourth commit changes the startEdit() for TreeCell implementations (7 files)
> 
> While writing tests, I found out that the CheckBoxListCell and the CheckBoxTreeCell don't disable their CheckBox, which is wrong.
> In CheckBoxTableCell and CheckBoxTreeTableCell the CheckBox is disabled, but they don't check their dependencies e.g. TableColumn for null, leading to a NPE if not set.
> 
> I wrote a follow-up and assigned it to myself: https://bugs.openjdk.java.net/browse/JDK-8270042
> The aim of this should be, that all 4 CheckBox...Cells have a proper CheckBox disablement while also being nullsafe.
> 
> ~Note 1: I removed the tests in TableCellTest added in https://github.com/openjdk/jfx/pull/529 in favor of the new parameterized TableCellStartEditTest~
> Note 2: This also fixes https://bugs.openjdk.java.net/browse/JDK-8268295

Marius Hanl has updated the pull request incrementally with one additional commit since the last revision:

  Addressed review comments
  
  - cell is supplied in setup() method
  - treeItem = null guard is done before calling startEdit()
  - Added checks for the cell graphic

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

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/569/files
  - new: https://git.openjdk.java.net/jfx/pull/569/files/fa89fccd..bf9190b0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=569&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=569&range=03-04

  Stats: 1186 lines in 11 files changed: 602 ins; 574 del; 10 mod
  Patch: https://git.openjdk.java.net/jfx/pull/569.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/569/head:pull/569

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


More information about the openjfx-dev mailing list