Hi Prasanta,
I'm not suggesting to remove the null check. I suggested to add check
for -1 since the bounds of the cell with index -1 should never exists
and it doesn't make scene to call the method to calculate its bounds.
--Semyon
On 12/03/2017 10:33 PM, Prasanta Sadhukhan wrote:
HI Semyon,
I guess app can also override getCellBounds and return null (maybe
mistakenly) so we need to check for return null.
Regards
Prasanta
On 11/29/2017 10:39 PM, Semyon Sadetsky wrote:
Hi Prasanta,
I suggest to call list.getCellBounds() only if index != -1 since the
result will be always null in that case.
--Semyon
On 11/29/2017 01:04 AM, Prasanta Sadhukhan wrote:
OK. http://cr.openjdk.java.net/~psadhukhan/8191639/webrev.03/
Regards
Prasanta
On 11/29/2017 12:35 AM, Sergey Bylokhov wrote:
On 27/11/2017 23:45, Prasanta Sadhukhan wrote:
Ok. I have added the check for scrollRectToVisible() too.
http://cr.openjdk.java.net/~psadhukhan/8191639/webrev.02
Should the test use some specific L&F? Currently it is pass on
macOS and Aqua L&F before the fix.
Regards
Prasanta
On 11/28/2017 1:01 PM, Sergey Bylokhov wrote:
On 27/11/2017 22:40, Prasanta Sadhukhan wrote:
On 11/28/2017 3:33 AM, Sergey Bylokhov wrote:
Hi, Prasanta.
A few notes about the fix:
- Can we get an NPE in adjustScrollPositionIfNecessary at
2391 list.scrollRectToVisible(cellBounds); Not sure that
scrollRectToVisible is ready for null.
cellBounds is not null for this call due to this check in
2308 if (cellBounds != null &&
!visRect.contains(cellBounds)) {
In the fix you added a check at line
2384 if (cellBounds != null) {
And later you will pass the cellBounds as a parameter:
2393 list.scrollRectToVisible(cellBounds);
So either cellBounds can be null or the new check is not needed.
- I think the assignment below can be simplified
cellBounds = startRect != null ? startRect : null;
- In the getNextPageIndex() probably we can "return index"
immediately instead of additional indentation?
Done. Modified webrev
http://cr.openjdk.java.net/~psadhukhan/8191639/webrev.01/
Regards
Prasanta
On 22/11/2017 03:05, Prasanta Sadhukhan wrote:
Hi All,
Bug: https://bugs.openjdk.java.net/browse/JDK-8191639
It is seen that when JList.locationToIndex() or
getCellBounds() is overridden to return -1 or null respectively,
it causes NPE when PageUp/PageDown is pressed in JList.
From the spec
[https://docs.oracle.com/javase/9/docs/api/javax/swing/JList.html#getCellBounds-int-int-]
of getCellBounds(), it is seen that it can return null under
some circumstances. But, JList jdk code assumes it is always
non-null.
Proposed fix is to check for null return value of
getCellBounds() and bail out.
webrev: http://cr.openjdk.java.net/~psadhukhan/8191639/webrev.00/
Regards
Prasanta
