<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
Fri Apr 20 13:40:02 UTC 2012
Hi Jonathan,
> Hi Pavel,
>
> Thanks a lot for review.
You are welcome! I marked bug 7055065 as "Fix Available". Not that on
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7055065
status will be updated later on.
Regards, Pavel
>
> Best regards!
> - Jonathan
>
> 2012/4/19 Pavel Porvatov <pavel.porvatov at oracle.com
> <mailto: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
> <http://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/20120420/7f25a6a3/attachment.html>
More information about the swing-dev
mailing list