Fix deletion of comments on the publish comments screen
We were getting a ClassCastException while deleting a comment on
the publish comments screen, because the editor assumed that it
was only contained within the AbstractPatchContentTable. Instead
use a common interface for the containing UI member.
Bug: issue 376
Change-Id: Id4fb59a85862f935f34c945ee70a0a61d69ec896
Signed-off-by: Shawn O. Pearce <sop@google.com>
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
index 7b2e63e..9691119 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
@@ -15,6 +15,7 @@
package com.google.gerrit.client.changes;
import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.client.patches.CommentEditorContainer;
import com.google.gerrit.client.patches.CommentEditorPanel;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.rpc.GerritCallback;
@@ -55,7 +56,8 @@
import java.util.Map;
import java.util.Set;
-public class PublishCommentScreen extends AccountScreen implements ClickHandler {
+public class PublishCommentScreen extends AccountScreen implements
+ ClickHandler, CommentEditorContainer {
private static SavedState lastState;
private final PatchSet.Id patchSetId;
@@ -63,7 +65,7 @@
private ChangeDescriptionBlock descBlock;
private Panel approvalPanel;
private NpTextArea message;
- private Panel draftsPanel;
+ private FlowPanel draftsPanel;
private Button send;
private Button cancel;
private boolean saveStateOnUnload = true;
@@ -147,6 +149,39 @@
}
}
+ @Override
+ public void notifyDraftDelta(int delta) {
+ }
+
+ @Override
+ public void remove(CommentEditorPanel editor) {
+ commentEditors.remove(editor);
+
+ // The editor should be embedded into a panel holding all
+ // editors for the same file.
+ //
+ FlowPanel parent = (FlowPanel) editor.getParent();
+ parent.remove(editor);
+
+ // If the panel now holds no editors, remove it.
+ //
+ int editorCount = 0;
+ for (Widget w : parent) {
+ if (w instanceof CommentEditorPanel) {
+ editorCount++;
+ }
+ }
+ if (editorCount == 0) {
+ parent.removeFromParent();
+ }
+
+ // If that was the last file with a draft, remove the heading.
+ //
+ if (draftsPanel.getWidgetCount() == 1) {
+ draftsPanel.clear();
+ }
+ }
+
private void initMessage(final Panel body) {
body.add(new SmallHeading(Util.C.headingCoverMessage()));
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
index 6c6b513..e91dbc5 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
@@ -39,7 +39,7 @@
import com.google.gwt.user.client.Event;
import com.google.gwt.user.client.History;
import com.google.gwt.user.client.ui.Button;
-import com.google.gwt.user.client.ui.FlexTable;
+import com.google.gwt.user.client.ui.Focusable;
import com.google.gwt.user.client.ui.UIObject;
import com.google.gwt.user.client.ui.Widget;
import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
@@ -50,7 +50,8 @@
import java.util.ArrayList;
import java.util.List;
-public abstract class AbstractPatchContentTable extends NavigationTable<Object> {
+public abstract class AbstractPatchContentTable extends NavigationTable<Object>
+ implements CommentEditorContainer {
protected PatchTable fileList;
protected AccountInfoCache accountCache = AccountInfoCache.empty();
protected Patch.Key patchKey;
@@ -94,13 +95,35 @@
table.setStyleName(Gerrit.RESOURCES.css().patchContentTable());
}
- void notifyDraftDelta(final int delta) {
+ public void notifyDraftDelta(final int delta) {
if (fileList != null) {
fileList.notifyDraftDelta(patchKey, delta);
}
}
@Override
+ public void remove(CommentEditorPanel panel) {
+ final int nRows = table.getRowCount();
+ for (int row = 0; row < nRows; row++) {
+ final int nCells = table.getCellCount(row);
+ for (int cell = 0; cell < nCells; cell++) {
+ if (table.getWidget(row, cell) == panel) {
+ destroyEditor(row, cell);
+ Widget p = table;
+ while (p != null) {
+ if (p instanceof Focusable) {
+ ((Focusable) p).setFocus(true);
+ break;
+ }
+ p = p.getParent();
+ }
+ return;
+ }
+ }
+ }
+ }
+
+ @Override
public void setRegisterKeys(final boolean on) {
super.setRegisterKeys(on);
if (on && keysComment != null && regComment == null) {
@@ -416,7 +439,7 @@
accountCache = aic;
}
- static void destroyEditor(final FlexTable table, final int row, final int col) {
+ private void destroyEditor(final int row, final int col) {
table.clearCell(row, col);
final int span = table.getFlexCellFormatter().getRowSpan(row, col);
boolean removeRow = true;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorContainer.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorContainer.java
new file mode 100644
index 0000000..b1381ca
--- /dev/null
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorContainer.java
@@ -0,0 +1,21 @@
+// Copyright (C) 2010 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.client.patches;
+
+public interface CommentEditorContainer {
+ void notifyDraftDelta(int delta);
+
+ void remove(CommentEditorPanel panel);
+}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
index da841a7..2af1c71 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java
@@ -30,8 +30,6 @@
import com.google.gwt.user.client.Window;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwt.user.client.ui.Button;
-import com.google.gwt.user.client.ui.FlexTable;
-import com.google.gwt.user.client.ui.Focusable;
import com.google.gwt.user.client.ui.Widget;
import com.google.gwtexpui.globalkey.client.NpTextArea;
import com.google.gwtjsonrpc.client.VoidResult;
@@ -276,12 +274,9 @@
}
private void notifyDraftDelta(final int delta) {
- Widget p = getParent();
- if (p != null) {
- p = p.getParent();
- if (p != null) {
- ((AbstractPatchContentTable) p).notifyDraftDelta(delta);
- }
+ CommentEditorContainer c = getContainer();
+ if (c != null) {
+ c.notifyDraftDelta(delta);
}
}
@@ -316,24 +311,20 @@
}
private void removeUI() {
- final FlexTable table = (FlexTable) getParent();
- final int nRows = table.getRowCount();
- for (int row = 0; row < nRows; row++) {
- final int nCells = table.getCellCount(row);
- for (int cell = 0; cell < nCells; cell++) {
- if (table.getWidget(row, cell) == this) {
- AbstractPatchContentTable.destroyEditor(table, row, cell);
- Widget p = table;
- while (p != null) {
- if (p instanceof Focusable) {
- ((Focusable) p).setFocus(true);
- break;
- }
- p = p.getParent();
- }
- return;
- }
- }
+ CommentEditorContainer c = getContainer();
+ if (c != null) {
+ c.remove(this);
}
}
+
+ private CommentEditorContainer getContainer() {
+ Widget p = getParent();
+ while (p != null) {
+ if (p instanceof CommentEditorContainer) {
+ return (CommentEditorContainer) p;
+ }
+ p = p.getParent();
+ }
+ return null;
+ }
}
