<Swing Dev> Review rquest for 7055065, NPE from JTable when quit editing with empty value in number culumns
Jonathan Lu
luchsh at linux.vnet.ibm.com
Fri Apr 20 10:57:25 UTC 2012
Hi Pavel,
Thanks a lot for review.
Best regards!
- Jonathan
2012/4/19 Pavel Porvatov <pavel.porvatov at oracle.com>
> Hi Jonathan,
>
> 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/<http://cr.openjdk.java.net/%7Eluchsh/7055065_2/>
>>
> The fix looks good. I approve it.
>
>
>> 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?
>>
> Swing repository is a legacy one and it's not used. I hope it will be
> removed soon to avoid such confusion.
>
> Regards, Pavel
>
>
>> 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/<http://cr.openjdk.java.net/%7Eluchsh/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/<http://cr.openjdk.java.net/%7Eluchsh/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.**setPreferredScrollableViewport**Size(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/20120420/fd7ef6f5/attachment.html>
More information about the swing-dev
mailing list