<Swing Dev> Review rquest for 7055065, NPE from JTable when quit editing with empty value in number culumns
Pavel Porvatov
pavel.porvatov at oracle.com
Thu Apr 19 09:58:14 UTC 2012
Hi Jonathan,
> Hi Pavel,
>
> After a simple grep, I did not see any invokeOnEDT method from swing
> or 2d repository, has it been committed yet?
That's because you are using incorrect repository. See here:
http://hg.openjdk.java.net/jdk8/awt/jdk/rev/8fe9b93e2474
Swing and AWT teams use awt repository.
Regards, Pavel
>
> 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
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20120419/45065aa4/attachment.html>
More information about the swing-dev
mailing list