<Swing Dev> [10] RFR JDK-8191639:NPE from BasicListUI.Actions.getNextPageIndex

[Sergey Bylokhov]

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
>>
>>
> 


-- 
Best regards, Sergey.