<Swing Dev> Review rquest for 7055065, NPE from JTable when quit editing with empty value in number culumns

[Pavel Porvatov]

Hi Jonathan,
> Hi Pavel,
>
> Thanks for reviewing, here's the webrev patch and automatic test. 
> Could you please help to take another look?
> http://cr.openjdk.java.net/~luchsh/7055065/
The fix looks good. Could you please fix few minor changes:

1. Don't use full class names like javax.swing.SwingUtilities when 
possible. I didn't find such rule in our Code Conventions but we follow 
this rule.

2. Swap two lines please
   frame.setVisible(true);
   frame.setLocation(0, 0);
That's not critical for the test but we shouldn't provide bad examples

3. I've recently introduced the Util#invokeOnEDT method. It can simplify 
your test and the getHeaderClickPoint and getCellClickPoint methods can 
be removed.

Regards, Pavel
>
> Thanks & regards!
> - Jonathan
>
> On 04/13/2012 05:59 PM, Pavel Porvatov wrote:
>> Hi Jonathan,
>>
>> The fix looks good. Could you please write an automatic test, put it 
>> into an appropriate place of repository and make a webrev with fix 
>> and test?
>>
>> Regards, Pavel
>>> Hello swing-dev,
>>>
>>> I've got a patch for bug 7055065, can anybody please help to take a 
>>> look?
>>> http://cr.openjdk.java.net/~luchsh/7055065/
>>>
>>> And also a simple test case for this issue here.
>>>
>>> /*
>>>  * Copyright (c) 2012 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
>>>  * under the terms of the GNU General Public License version 2 only, as
>>>  * published by the Free Software Foundation.
>>>  *
>>>  * This code is distributed in the hope that it will be useful, but 
>>> WITHOUT
>>>  * ANY WARRANTY; without even the implied warranty of 
>>> MERCHANTABILITY or
>>>  * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public 
>>> License
>>>  * version 2 for more details (a copy is included in the LICENSE 
>>> file that
>>>  * accompanied this code).
>>>  *
>>>  * You should have received a copy of the GNU General Public License 
>>> version
>>>  * 2 along with this work; if not, write to the Free Software 
>>> Foundation,
>>>  * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>>>  *
>>>  * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 
>>> 94065 USA
>>>  * or visit www.oracle.com if you need additional information or 
>>> have any
>>>  * questions.
>>>  */
>>>
>>> /*
>>>  * Portions Copyright (c) 2012 IBM Corporation
>>>  */
>>>
>>> import javax.swing.JFrame;
>>> import javax.swing.JPanel;
>>> import javax.swing.JScrollPane;
>>> import javax.swing.JTable;
>>> import javax.swing.table.AbstractTableModel;
>>> import javax.swing.table.TableModel;
>>> import javax.swing.table.TableRowSorter;
>>> import java.awt.Dimension;
>>> import java.awt.GridLayout;
>>>
>>> public class TableDemo extends JPanel {
>>>
>>>     public TableDemo() {
>>>         super(new GridLayout(1, 0));
>>>
>>>         final String[] columnNames = { "String", "Number" };
>>>         final Object[][] data = { { "aaaa", new Integer(5) },
>>>                 { "bbbb", new Integer(3) }, { "cccc", new Integer(2) },
>>>                 { "dddd", new Integer(20) }, { "eeee", new 
>>> Integer(10) } };
>>>         final JTable table = new JTable(data, columnNames);
>>>
>>>         table.setPreferredScrollableViewportSize(new Dimension(500, 
>>> 400));
>>>         table.setFillsViewportHeight(true);
>>>         TableModel dataModel = new AbstractTableModel() {
>>>
>>>             public int getColumnCount() {
>>>                 return columnNames.length;
>>>             }
>>>             public int getRowCount() {
>>>                 return data.length;
>>>             }
>>>             public Object getValueAt(int row, int col) {
>>>                 return data[row][col];
>>>             }
>>>             public String getColumnName(int column) {
>>>                 return columnNames[column];
>>>             }
>>>             public Class<?> getColumnClass(int c) {
>>>                 return getValueAt(0, c).getClass();
>>>             }
>>>             public boolean isCellEditable(int row, int col) {
>>>                 return col != 5;
>>>             }
>>>             public void setValueAt(Object aValue, int row, int 
>>> column) {
>>>                 data[row][column] = aValue;
>>>             }
>>>         };
>>>         table.setModel(dataModel);
>>>         TableRowSorter<TableModel> sorter = new 
>>> TableRowSorter<TableModel>(
>>>                 dataModel);
>>>         table.setRowSorter(sorter);
>>>
>>>         JScrollPane scrollPane = new JScrollPane(table);
>>>         add(scrollPane);
>>>     }
>>>
>>>     public static void main(String[] args) throws Exception {
>>>         javax.swing.SwingUtilities.invokeAndWait(new Runnable() {
>>>             public void run() {
>>>                 JFrame frame = new JFrame("SimpleTableDemo");
>>>                 frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>>>
>>>                 TableDemo newContentPane = new TableDemo();
>>>                 newContentPane.setOpaque(true);
>>>                 frame.setContentPane(newContentPane);
>>>
>>>                 frame.pack();
>>>                 frame.setVisible(true);
>>>             }
>>>         });
>>>     }
>>> }
>>>
>>> To reproduce the problem,
>>> please click on one cell from the "Number" column, delete all the 
>>> content but do not press enter to quit editing status, then click 
>>> the column title of "Number" column to sort, NPE will be thrown.
>>>
>>> The cause of this problem is when trying to quit editing with empty 
>>> content of a cell  and also try to accept the partially edited value 
>>> using stopCellEditing(), following two actions will be taken.
>>> 1, the cellEditor of current table will be set to null.
>>> 2, the value type of this column does not have a constructor to 
>>> accept "Object[]" parameter, false will be returned.
>>> Then swing will try to cancel edition without accepting the 
>>> partially edited value using cancelCellEditing() which will get null 
>>> for the value of cellEditor thus lead to this NPE.
>>>
>>> The patch is trying to return earlier before the type compatibility 
>>> check of partially edited values when empty cell values encountered.
>>>
>>> Cheers!
>>> - Jonathan
>>>
>>
>