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

[Prasanta Sadhukhan]

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