Skip to content

Commit

Permalink
8251941: ListCell: visual artifact when items contain null values
Browse files Browse the repository at this point in the history
Reviewed-by: aghaisas
  • Loading branch information
Jeanette Winzenburg committed Aug 28, 2020
1 parent 23ad8f4 commit c86bd35
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -483,7 +483,7 @@ private void updateItem(int oldIndex) {
// refer to Ensemble8PopUpTree.png - in this case the arrows are being
// shown as the new cells are instantiated with the arrows in the
// children list, and are only hidden in updateItem.
if ((!isEmpty && oldValue != null) || firstRun) {
if (!isEmpty || firstRun) {
updateItem(null, true);
firstRun = false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -277,6 +277,93 @@ public class ListCellTest {
assertEquals("Water", cell.getItem());
}

//---------- tests around JDK-8251941: broken cleanup with null item

/**
* Transition not-empty -> empty by items modification
*/
@Test
public void testNullItemRemoveAsLast() {
model.add(null);
cell.updateListView(list);
int last = model.size() - 1;
cell.updateIndex(last);
model.remove(last);
assertOffRangeState(last);
}

/**
* Sanity: transition not-empty -> not empty by items modification
*/
@Test
public void testNullItemRemoveAsFirst() {
int first = 0;
model.add(first, null);
cell.updateListView(list);
cell.updateIndex(first);
model.remove(first);
assertInRangeState(first);
}

/**
* Transition not-empty -> empty by updateIndex
*/
@Test
public void testNullItemUpdateIndexOffRange() {
model.add(0, null);
cell.updateListView(list);
cell.updateIndex(0);
// update to off range > max
cell.updateIndex(model.size());
assertOffRangeState(model.size());
}

/**
* Transition not-empty -> empty by updateIndex
*/
@Test
public void testNullItemUpdateIndexNegative() {
model.add(0, null);
cell.updateListView(list);
cell.updateIndex(0);
// update to off range < 0
cell.updateIndex(-1);
assertOffRangeState(-1);
}

/**
* Sanity: in-range null item.
*/
@Test
public void testNullItem() {
// null item in range, verify state
model.add(0, null);
cell.updateListView(list);
cell.updateIndex(0);
assertInRangeState(0);
}

/**
* Asserts state for the given off-range index.
* @param index
*/
protected void assertOffRangeState(int index) {
assertEquals("off range index", index, cell.getIndex());
assertNull("off range cell item must be null", cell.getItem());
assertTrue("off range cell must be empty", cell.isEmpty());
}

/**
* Asserts state for the given in-range index.
* @param index
*/
protected void assertInRangeState(int index) {
assertEquals("in range index", index, cell.getIndex());
assertEquals("in range cell item must be same as model item", model.get(index), cell.getItem());
assertFalse("in range cell must not be empty", cell.isEmpty());
}


/*********************************************************************
* Tests for the selection listener *
********************************************************************/
Expand Down

0 comments on commit c86bd35

Please sign in to comment.