[gwt-contrib] Fix issue 6959. (issue1587803)

Sat, 05 Nov 2011 05:30:17 -0700 

Reviewers: rdayal,

Description:
Fix issue 6959.

One cannot tell a synthetic editor used for traversal from a "real"
editor. This is because ListEditor#createEditorForTraversal uses '0' as
the index. I'm thus changing slightly the EditorSource contract to allow
-1 to be passed to create(int) as a signal that we're creating such a
synthetic editor. HasDataEditorSource then uses this signal to not mess
with the HasData.

Please review this at http://gwt-code-reviews.appspot.com/1587803/

Affected files:
  M user/src/com/google/gwt/editor/client/adapters/EditorSource.java
  M user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
  M user/src/com/google/gwt/editor/client/adapters/ListEditor.java
  M user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java
  M user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java


Index: user/src/com/google/gwt/editor/client/adapters/EditorSource.java
diff --git a/user/src/com/google/gwt/editor/client/adapters/EditorSource.java b/user/src/com/google/gwt/editor/client/adapters/EditorSource.java index 6a1d7798529688971c2c67a1cf63b778984d8ee3..6083fb9c228249bca83d9c16e3ef932d7fff6490 100644 
--- a/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
+++ b/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
@@ -16,13 +16,14 @@
 package com.google.gwt.editor.client.adapters;

 import com.google.gwt.editor.client.Editor;
+import com.google.gwt.editor.client.EditorContext;

 import java.util.ArrayList;
 import java.util.List;

 /**
* An entity capable of creating and destroying instances of Editors. This type - * is used by Editors which operate on ordered data, sich as {@link ListEditor}. + * is used by Editors which operate on ordered data, such as {@link ListEditor}. 
  *
  * @param <E> the type of Editor required
  * @see com.google.gwt.editor.client.testing.FakeEditorSource
@@ -30,9 +31,13 @@ import java.util.List;
 public abstract class EditorSource<E extends Editor<?>> {
   /**
    * Create a new Editor.
+   * <p>
+ * The {@code index} can be -1 to ask for a synthetic editor used only for traversal. 
    *
    * @param index the position at which the new Editor should be displayed
    * @return an {@link Editor} of type E
+   * @see ListEditor#createEditorForTraversal()
+ * @see EditorContext#traverseSyntheticCompositeEditor(com.google.gwt.editor.client.EditorVisitor) 
    */
   public abstract E create(int index);

@@ -45,6 +50,7 @@ public abstract class EditorSource<E extends Editor<?>> {
    * @return a List of {@link Editor}s of type E
    */
   public List<E> create(int count, int index) {
+    assert index >= 0;
     List<E> toReturn = new ArrayList<E>(count);
     for (int i = 0; i < count; i++) {
       toReturn.add(create(index + i));
Index: user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
diff --git a/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java b/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java index f2bb40fdb4a0b7ef377932f2db667958492d7624..fbd196549b85f47d531423a92469fe2aecf96eb7 100644 
--- a/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
+++ b/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
@@ -35,12 +35,20 @@ public class HasDataEditor<T> extends ListEditor<T, LeafValueEditor<T>> { 

     @Override
     public IndexedEditor<T> create(int index) {
-      return new IndexedEditor<T>(index, data);
+      // A negative index signals a synthetic editor used for traversal; we
+      // don't give it the HasData so that it doesn't mess with it.
+ // See http://code.google.com/p/google-web-toolkit/issues/detail?id=6959 
+      return new IndexedEditor<T>(index, index < 0 ? null : data);
     }

     @Override
     public void dispose(LeafValueEditor<T> subEditor) {
-      data.setRowCount(data.getRowCount() - 1);
+      // A negative index signals a synthetic editor used for traversal; we
+      // don't mess with the HasData for such synthetic sub-editors.
+ // See http://code.google.com/p/google-web-toolkit/issues/detail?id=6959 
+      if (((IndexedEditor<T>) subEditor).index >= 0) {
+        data.setRowCount(data.getRowCount() - 1);
+      }
     }

     @Override
Index: user/src/com/google/gwt/editor/client/adapters/ListEditor.java
diff --git a/user/src/com/google/gwt/editor/client/adapters/ListEditor.java b/user/src/com/google/gwt/editor/client/adapters/ListEditor.java index 592a188cde90a0c996e3c382345e1689ba4cb9af..54afef06b0f07a72bda5e7f9e4b63dfc1f4249ea 100644 
--- a/user/src/com/google/gwt/editor/client/adapters/ListEditor.java
+++ b/user/src/com/google/gwt/editor/client/adapters/ListEditor.java
@@ -63,7 +63,7 @@ public class ListEditor<T, E extends Editor<T>> implements
    * @return an {@link Editor} of type E
    */
   public E createEditorForTraversal() {
-    E toReturn = editorSource.create(0);
+    E toReturn = editorSource.create(-1);
     editorSource.dispose(toReturn);
     return toReturn;
   }
Index: user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java
diff --git a/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java b/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java index 073aec36c087423030f8c1a1ccbce15a488c62c4..0107dae0b783a44e7dcddae8e5ed52c2f9e1a1fc 100644 
--- a/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java
+++ b/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java
@@ -37,7 +37,7 @@ public class FakeEditorSource<T> extends EditorSource<FakeLeafValueEditor<T>> { 
    * Return value for {@link #getLastKnownPosition} if the editor was not
    * created by this FakeEditorSource.
    */
-  public static final int UNKNOWN = -1;
+  public static final int UNKNOWN = -3;
private final Map<FakeLeafValueEditor<T>, Integer> lastKnownPosition = new HashMap<FakeLeafValueEditor<T>, Integer>(); 

   @Override
Index: user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java diff --git a/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java b/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java index 9e400faec0e0d90b6fa89764870953309a62609e..52283d1ca3b2d66f445f2db78c405e38ae87c48a 100644 
--- a/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java
+++ b/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java
@@ -16,6 +16,8 @@
 package com.google.gwt.editor.client.adapters;

 import com.google.gwt.core.client.GWT;
+import com.google.gwt.editor.client.EditorContext;
+import com.google.gwt.editor.client.EditorVisitor;
 import com.google.gwt.editor.client.SimpleBeanEditorDriver;
 import com.google.gwt.event.shared.GwtEvent;
 import com.google.gwt.event.shared.HandlerRegistration;
@@ -205,6 +207,33 @@ public class HasDataEditorTest extends GWTTestCase {
     assertEquals(expectedValue, editor.getList());
     assertEquals(expectedValue, hasData.getRowData());
   }
+
+  /**
+   * See http://code.google.com/p/google-web-toolkit/issues/detail?id=6959
+   */
+  public void testTraverseSyntheticCompositeEditor() {
+    List<Integer> expectedValue = Arrays.asList(1, 2, 3, 4, 5);
+
+    EditorVisitor visitor = new EditorVisitor() {
+      public <T> boolean visit(EditorContext<T> ctx) {
+        if (ctx.asCompositeEditor() != null) {
+          ctx.traverseSyntheticCompositeEditor(this);
+        }
+        return true;
+      }
+    };
+
+    // check that it won't throw
+    driver.accept(visitor);
+
+    driver.edit(expectedValue);
+
+    // Shouldn't affect the editor and HasData
+    driver.accept(visitor);
+
+    assertEquals(expectedValue, editor.getList());
+    assertEquals(expectedValue, hasData.getRowData());
+  }

   @Override
   protected void gwtSetUp() throws Exception {


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to