Review rquest for 7055065, NPE from JTable when quit editing with empty value in number culumns (original) (raw)

Jonathan Lu luchsh at linux.vnet.ibm.com
Thu Apr 19 09:51:42 UTC 2012

Hi Pavel,

I found Util.invokeOnEDT in awt repository, and have updated the test case, could you please take another look? http://cr.openjdk.java.net/~luchsh/7055065_2/

It indeed confused me when found the change in awt repository, will it be merged to swing repository or is it a special change for testing infrastructure so went to awt repository?

Thanks and best regards!

On 04/19/2012 11:24 AM, Jonathan Lu wrote:

Hi Pavel,

After a simple grep, I did not see any invokeOnEDT method from swing or 2d repository, has it been committed yet? Thanks and best regards! - Jonathan On 04/16/2012 07:51 PM, Pavel Porvatov wrote: 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 sorter = new TableRowSorter( 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.EXITONCLOSE); 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

More information about the swing-dev mailing list