<Swing Dev> Review rquest for 7055065, NPE from JTable when quit editing with empty value in number culumns
Charles Lee
littlee at linux.vnet.ibm.com
Fri Apr 20 05:25:05 UTC 2012
Hi Jonathan,
The patch is committed @
Changeset: dfa2ea47257d
Author: luchsh
Date: 2012-04-20 13:13 +0800
URL:http://hg.openjdk.java.net/jdk8/awt/jdk/rev/dfa2ea47257d
7055065: NullPointerException when sorting JTable with empty cell
Reviewed-by: rupashka
Please verify it.
Thanks Pavel for reviewing it.
On 04/19/2012 05:51 PM, Jonathan Lu wrote:
> 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!
> - Jonathan
>
> 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<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
>>>>>>
>>>>>
>>>>
>>>
>>
>
--
Yours Charles
More information about the swing-dev
mailing list