Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-6562442

JList.setModel() may silently fail if ListModel.equals() returns true

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: P5
    • Resolution: Not an Issue
    • Affects Version/s: 6, 11
    • Fix Version/s: 11
    • Component/s: client-libs
    • Labels:

      Description

      FULL PRODUCT VERSION :


      A DESCRIPTION OF THE PROBLEM :
      If ListModel has been subclassed to support value-based rather than reference-based equality (which, given that it encapsulates a list, is a very reasonable/common thing to do), then JList.setModel() will result in inconsistent state - the UI delegate painting using one model, but being notified of changes only from the other model.

      If when setModel() is called the new and old models compare as equal, setModel() will update the JList.dataModel field (which determines what is used by the UI delegate to do painting), but the UI delegate's PropertyChangeListener will not be notified. As a result the UI delegate will not register as a DataListener for the new model, and changes made to the new model will never be displayed to the user.

      This is a subtle and annoying bug. To my annoyance I found that someone already notified Sun about this in 2001 (bug 4528403) but it has not been fixed or even documented... so I've just had to spend an entire day tracking it down.

      Anyway, it should be quite straightforward to fix. The problem is that the JList notifies the UI delegate of the model change using:
      firePropertyChange("model", oldValue, dataModel);
      which naturally assumes that the models are identical if equals() returns true, since that is the documented behaviour of PropertyChangeSupport, and does not send the property change notification. The solution therefore is to fix the way JList fires the property change so it always results in a notification.

      I therefore propose:
      - ideally, just set the old value to null:
      if (oldValue != dataModel) firePropertyChange("model", null, dataModel);
      Everything will work fine if you do this. Obviously it could break existing listeners to this event, but they are already unavoidably broken because of this bug.
      - if this is consider too risky, you could do what we already do plus:
      if (oldValue != dataModel) firePropertyChange("model_bugfix", null, dataModel);
      and then update the BasicListUI etc to listen for the “model_bugfix” property instead of “model”.

      - I think either of these alternatives are eminently acceptable but if not PLEASE at least document the broken behaviour in the ListModel and DefaultListModel JavaDoc. Without these fixes implementers of these classes need to avoid overriding the default (reference-based) equals().

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      See source code below

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      Item1, Item2 and Item3 should be displayed in the JList, since all 3 are present in the model.
      ACTUAL -
      No items are displayed in the JList.

      REPRODUCIBILITY :
      This bug can be reproduced always.

      ---------- BEGIN SOURCE ----------
      package com.apama.swing;

      import java.util.Arrays;
      import javax.swing.*;

      class MyListModel extends DefaultListModel {
      @Override
      public boolean equals(Object obj) {
      if (obj instanceof MyListModel) {
      MyListModel model = (MyListModel) obj;
      return Arrays.equals(this.toArray(), model.toArray());
      }
      return false;
      }
      }

      public class ListModelBugDemo {
      public static void main(String[] args) {
              JFrame frame = new JFrame();
              frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
       
              MyListModel model1 = new MyListModel();
              MyListModel model2 = new MyListModel();

              model1.addElement("Item1");
              model2.addElement("Item1");

              JList list = new JList(model1);
              frame.getContentPane().add(new JScrollPane(list));
              
              list.setModel(model2);
              
              model2.addElement("Item2");
              frame.pack();
              model2.addElement("Item3"); // comment out this line and you see the other two items!
              
              frame.setSize(400, 400);
              frame.setLocationRelativeTo(null);
              
              frame.setVisible(true);
      }
      }

      ---------- END SOURCE ----------

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              pbansal Pankaj Bansal
              Reporter:
              ryeung Roger Yeung (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Imported:
                Indexed: