Fixing a few Cell Widget bugs. I combined these bugs because they are all quick fixes and fairly straightforward.
(Issue 5971) CompositeCell does not implement isEditing. I implemented isEditing in CompositeCell.
(Issue 5993) TextInputCell and EditTextCell double escape values before putting them into text boxes. We no longer use the SafeHtmlRenderer to render the content of the input value, as input values are always treat their values as text. SafeHtml isn't valid in an attribute context.
Selecting a range in a CellTable only works on the first page. DefaultSelectionEventManager now correctly subtracts the page start index when getting the selected values from the CellTable.
Non-bubbling events (change/load/error/focus/blur) aren't captured in CellTable in IE9. Switched IE9 to use the StandardBase implementation, which is much simpler and works for IE9.
Review at http://gwt-code-reviews.appspot.com/1408801
Review by: fabiomfv@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@9954 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/tools/api-checker/config/gwt22_23userApi.conf b/tools/api-checker/config/gwt22_23userApi.conf
index b417a48..88cb8f5 100644
--- a/tools/api-checker/config/gwt22_23userApi.conf
+++ b/tools/api-checker/config/gwt22_23userApi.conf
@@ -119,3 +119,5 @@
# Overloaded SimplePanel constructor to accept a Widget.
com.google.gwt.user.client.ui.SimplePanel::SimplePanel(Lcom/google/gwt/dom/client/Element;) OVERLOADED_METHOD_CALL
+# Renamed CellBasedWidgetImplSafari to CellBasedWidgetImplStandardBase.
+com.google.gwt.user.cellview.client.CellBasedWidgetImplSafari MISSING
diff --git a/user/src/com/google/gwt/cell/client/CompositeCell.java b/user/src/com/google/gwt/cell/client/CompositeCell.java
index 30727b8..34d2b09 100644
--- a/user/src/com/google/gwt/cell/client/CompositeCell.java
+++ b/user/src/com/google/gwt/cell/client/CompositeCell.java
@@ -113,6 +113,18 @@
}
@Override
+ public boolean isEditing(Context context, Element parent, C value) {
+ Element curChild = getContainerElement(parent).getFirstChildElement();
+ for (HasCell<C, ?> hasCell : hasCells) {
+ if (isEditingImpl(context, curChild, value, hasCell)) {
+ return true;
+ }
+ curChild = curChild.getNextSiblingElement();
+ }
+ return false;
+ }
+
+ @Override
public void onBrowserEvent(Context context, Element parent, C value,
NativeEvent event, ValueUpdater<C> valueUpdater) {
int index = 0;
@@ -199,6 +211,11 @@
sb.appendHtmlConstant("</span>");
}
+ private <X> boolean isEditingImpl(Context context, Element cellParent, C object,
+ HasCell<C, X> hasCell) {
+ return hasCell.getCell().isEditing(context, cellParent, hasCell.getValue(object));
+ }
+
private <X> void onBrowserEventImpl(Context context, Element parent,
final C object, NativeEvent event, final ValueUpdater<C> valueUpdater,
final HasCell<C, X> hasCell) {
diff --git a/user/src/com/google/gwt/cell/client/EditTextCell.java b/user/src/com/google/gwt/cell/client/EditTextCell.java
index e180a69..a8a8cd9 100644
--- a/user/src/com/google/gwt/cell/client/EditTextCell.java
+++ b/user/src/com/google/gwt/cell/client/EditTextCell.java
@@ -30,10 +30,6 @@
/**
* An editable text cell. Click to edit, escape to cancel, return to commit.
- *
- * Important TODO: This cell still treats its value as HTML for rendering
- * purposes, which is entirely wrong. It should be able to treat it as a proper
- * string (especially since that's all the user can enter).
*/
public class EditTextCell extends
AbstractEditableCell<String, EditTextCell.ViewData> {
@@ -146,9 +142,11 @@
}
/**
- * Construct a new EditTextCell that will use a given {@link SafeHtmlRenderer}.
+ * Construct a new EditTextCell that will use a given {@link SafeHtmlRenderer}
+ * to render the value when not in edit mode.
*
- * @param renderer a {@link SafeHtmlRenderer SafeHtmlRenderer<String>} instance
+ * @param renderer a {@link SafeHtmlRenderer SafeHtmlRenderer<String>}
+ * instance
*/
public EditTextCell(SafeHtmlRenderer<String> renderer) {
super("click", "keyup", "keydown", "blur");
@@ -206,17 +204,19 @@
if (viewData != null) {
String text = viewData.getText();
- SafeHtml html = renderer.render(text);
if (viewData.isEditing()) {
- // Note the template will not treat SafeHtml specially
- sb.append(template.input(html.asString()));
+ /*
+ * Do not use the renderer in edit mode because the value of a text
+ * input element is always treated as text. SafeHtml isn't valid in the
+ * context of the value attribute.
+ */
+ sb.append(template.input(text));
} else {
// The user pressed enter, but view data still exists.
- sb.append(html);
+ sb.append(renderer.render(text));
}
} else if (value != null) {
- SafeHtml html = renderer.render(value);
- sb.append(html);
+ sb.append(renderer.render(value));
}
}
diff --git a/user/src/com/google/gwt/cell/client/TextInputCell.java b/user/src/com/google/gwt/cell/client/TextInputCell.java
index e90d295..05b5ab0 100644
--- a/user/src/com/google/gwt/cell/client/TextInputCell.java
+++ b/user/src/com/google/gwt/cell/client/TextInputCell.java
@@ -23,7 +23,6 @@
import com.google.gwt.safehtml.shared.SafeHtml;
import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
import com.google.gwt.text.shared.SafeHtmlRenderer;
-import com.google.gwt.text.shared.SimpleSafeHtmlRenderer;
/**
* An {@link AbstractCell} used to render a text input.
@@ -129,30 +128,26 @@
private static Template template;
- private final SafeHtmlRenderer<String> renderer;
-
/**
* Constructs a TextInputCell that renders its text without HTML markup.
*/
public TextInputCell() {
- this(SimpleSafeHtmlRenderer.getInstance());
+ super("change", "keyup");
+ if (template == null) {
+ template = GWT.create(Template.class);
+ }
}
/**
* Constructs a TextInputCell that renders its text using the given
* {@link SafeHtmlRenderer}.
*
- * @param renderer a non-null SafeHtmlRenderer
+ * @param renderer parameter is ignored
+ * @deprecated the value of a text input is never treated as html
*/
+ @Deprecated
public TextInputCell(SafeHtmlRenderer<String> renderer) {
- super("change", "keyup");
- if (template == null) {
- template = GWT.create(Template.class);
- }
- if (renderer == null) {
- throw new IllegalArgumentException("renderer == null");
- }
- this.renderer = renderer;
+ this();
}
@Override
@@ -194,9 +189,7 @@
String s = (viewData != null) ? viewData.getCurrentValue() : value;
if (s != null) {
- SafeHtml html = renderer.render(s);
- // Note: template will not treat SafeHtml specially
- sb.append(template.input(html.asString()));
+ sb.append(template.input(s));
} else {
sb.appendHtmlConstant("<input type=\"text\" tabindex=\"-1\"></input>");
}
diff --git a/user/src/com/google/gwt/user/cellview/CellView.gwt.xml b/user/src/com/google/gwt/user/cellview/CellView.gwt.xml
index ec6bcf2..4cb8426 100644
--- a/user/src/com/google/gwt/user/cellview/CellView.gwt.xml
+++ b/user/src/com/google/gwt/user/cellview/CellView.gwt.xml
@@ -35,15 +35,15 @@
<any>
<when-property-is name="user.agent" value="ie6"/>
<when-property-is name="user.agent" value="ie8"/>
- <when-property-is name="user.agent" value="ie9"/>
</any>
</replace-with>
- <!-- Safari-specific CellBasedWidgetImpl implementation. -->
- <replace-with class="com.google.gwt.user.cellview.client.CellBasedWidgetImplSafari">
+ <!-- StandardBase CellBasedWidgetImpl implementation. -->
+ <replace-with class="com.google.gwt.user.cellview.client.CellBasedWidgetImplStandardBase">
<when-type-is class="com.google.gwt.user.cellview.client.CellBasedWidgetImpl"/>
<any>
<when-property-is name="user.agent" value="safari"/>
+ <when-property-is name="user.agent" value="ie9"/>
</any>
</replace-with>
diff --git a/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java b/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplStandardBase.java
similarity index 76%
rename from user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java
rename to user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplStandardBase.java
index c24d875..0e82b60 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplStandardBase.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2010 Google Inc.
+ * Copyright 2011 Google Inc.
*
* 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
@@ -19,13 +19,13 @@
import com.google.gwt.core.client.Scheduler.ScheduledCommand;
/**
- * Webkit specified Impl used by cell based widgets.
+ * StandardBase implementation of {@link CellBasedWidgetImpl}.
*/
-public class CellBasedWidgetImplSafari extends CellBasedWidgetImplStandard {
+public class CellBasedWidgetImplStandardBase extends CellBasedWidgetImplStandard {
@Override
public void resetFocus(ScheduledCommand command) {
- // Webkit will not focus an element that was created in this event loop.
+ // Some browsers will not focus an element that was created in this event loop.
Scheduler.get().scheduleDeferred(command);
}
}
diff --git a/user/src/com/google/gwt/view/client/DefaultSelectionEventManager.java b/user/src/com/google/gwt/view/client/DefaultSelectionEventManager.java
index fd752b6..c9e33a6 100644
--- a/user/src/com/google/gwt/view/client/DefaultSelectionEventManager.java
+++ b/user/src/com/google/gwt/view/client/DefaultSelectionEventManager.java
@@ -384,7 +384,7 @@
* Update the selection model based on a user selection event.
*
* @param selectionModel the selection model to update
- * @param row the selected row index relative to the page start
+ * @param row the absolute index of the selected row
* @param rowValue the selected row value
* @param action the {@link SelectAction} to apply
* @param selectRange true to select the range from the last selected row
@@ -621,9 +621,9 @@
// Get the list of values to select.
List<T> toUpdate = new ArrayList<T>();
int itemCount = display.getVisibleItemCount();
- int start = range.getStart();
- int end = start + range.getLength();
- for (int i = start; i < end && i < itemCount; i++) {
+ int relativeStart = range.getStart() - display.getVisibleRange().getStart();
+ int relativeEnd = relativeStart + range.getLength();
+ for (int i = relativeStart; i < relativeEnd && i < itemCount; i++) {
toUpdate.add(display.getVisibleItem(i));
}
diff --git a/user/test/com/google/gwt/cell/client/CompositeCellTest.java b/user/test/com/google/gwt/cell/client/CompositeCellTest.java
index a40461c..2dd7d7f 100644
--- a/user/test/com/google/gwt/cell/client/CompositeCellTest.java
+++ b/user/test/com/google/gwt/cell/client/CompositeCellTest.java
@@ -83,6 +83,42 @@
assertFalse(cell.dependsOnSelection());
}
+ public void testIsEditingFalse() {
+ List<HasCell<String, ?>> cells = createHasCells(3);
+ CompositeCell<String> cell = new CompositeCell<String>(cells);
+ Element parent = Document.get().createDivElement();
+ parent.setInnerHTML(getExpectedInnerHtml());
+ assertFalse(cell.isEditing(new Context(0, 0, null), parent, "test"));
+ }
+
+ public void testIsEditingTrue() {
+ List<HasCell<String, ?>> cells = createHasCells(3);
+ // Add a cell that is being edited.
+ final MockCell<String> mock = new MockCell<String>(false, null) {
+ @Override
+ public boolean isEditing(Context context, Element parent, String value) {
+ return true;
+ }
+ };
+ cells.add(new HasCell<String, String>() {
+ public Cell<String> getCell() {
+ return mock;
+ }
+
+ public FieldUpdater<String, String> getFieldUpdater() {
+ return null;
+ }
+
+ public String getValue(String object) {
+ return object;
+ }
+ });
+ CompositeCell<String> cell = new CompositeCell<String>(cells);
+ Element parent = Document.get().createDivElement();
+ parent.setInnerHTML(getExpectedInnerHtml());
+ assertTrue(cell.isEditing(new Context(0, 0, null), parent, "test"));
+ }
+
/**
* Fire an event to no cell in particular.
*/
@@ -137,7 +173,7 @@
Cell<String> cell = createCell();
Element parent = Document.get().createDivElement();
parent.setInnerHTML(getExpectedInnerHtml());
- Context context = new Context( 0, 0, null);
+ Context context = new Context(0, 0, null);
cell.setValue(context, parent, "test");
assertEquals(3, parent.getChildCount());
diff --git a/user/test/com/google/gwt/cell/client/EditTextCellTest.java b/user/test/com/google/gwt/cell/client/EditTextCellTest.java
index c088c71..9abc4f9 100644
--- a/user/test/com/google/gwt/cell/client/EditTextCellTest.java
+++ b/user/test/com/google/gwt/cell/client/EditTextCellTest.java
@@ -137,6 +137,33 @@
assertEquals("newValue", sb.toSafeHtml().asString());
}
+ /**
+ * Test rendering the cell with a malicious value.
+ */
+ public void testRenderUnsafeHtml() {
+ EditTextCell cell = createCell();
+ SafeHtmlBuilder sb = new SafeHtmlBuilder();
+ Context context = new Context(0, 0, null);
+ cell.render(context, "<script>malicious</script>", sb);
+ assertEquals("<script>malicious</script>", sb.toSafeHtml().asString());
+ }
+
+ /**
+ * Test rendering the cell with a malicious value in edit mode.
+ */
+ public void testRenderUnsafeHtmlWhenEditing() {
+ EditTextCell cell = createCell();
+ ViewData viewData = new ViewData("originalValue");
+ viewData.setText("<script>malicious</script>");
+ viewData.setEditing(true);
+ cell.setViewData(DEFAULT_KEY, viewData);
+ SafeHtmlBuilder sb = new SafeHtmlBuilder();
+ Context context = new Context(0, 0, DEFAULT_KEY);
+ cell.render(context, "<script>malicious</script>", sb);
+ assertEquals("<input type=\"text\" value=\"<script>malicious</script>\" "
+ + "tabindex=\"-1\"></input>", sb.toSafeHtml().asString());
+ }
+
public void testViewData() {
// Start in edit mode.
ViewData viewData = new ViewData("originalValue");
diff --git a/user/test/com/google/gwt/cell/client/TextInputCellTest.java b/user/test/com/google/gwt/cell/client/TextInputCellTest.java
index bcaa2df..b6aea9e 100644
--- a/user/test/com/google/gwt/cell/client/TextInputCellTest.java
+++ b/user/test/com/google/gwt/cell/client/TextInputCellTest.java
@@ -15,8 +15,10 @@
*/
package com.google.gwt.cell.client;
+import com.google.gwt.cell.client.Cell.Context;
import com.google.gwt.dom.client.Document;
import com.google.gwt.dom.client.NativeEvent;
+import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
/**
* Tests for {@link TextInputCell}.
@@ -42,6 +44,19 @@
expected);
}
+ /**
+ * Test rendering the cell with a malicious value.
+ */
+ public void testRenderUnsafeHtml() {
+ Cell<String> cell = createCell();
+ SafeHtmlBuilder sb = new SafeHtmlBuilder();
+ Context context = new Context(0, 0, null);
+ cell.render(context, "<script>malicious</script>", sb);
+ assertEquals(
+ "<input type=\"text\" value=\"<script>malicious</script>\" tabindex=\"-1\">"
+ + "</input>", sb.toSafeHtml().asString());
+ }
+
@Override
protected TextInputCell createCell() {
return new TextInputCell();
diff --git a/user/test/com/google/gwt/view/client/DefaultSelectionEventManagerTest.java b/user/test/com/google/gwt/view/client/DefaultSelectionEventManagerTest.java
index 9369a01..2d729fd 100644
--- a/user/test/com/google/gwt/view/client/DefaultSelectionEventManagerTest.java
+++ b/user/test/com/google/gwt/view/client/DefaultSelectionEventManagerTest.java
@@ -202,6 +202,24 @@
assertSelected(model, "test 3");
}
+ /**
+ * Test that selecting a range works when the visible range doesn't start at index 0.
+ */
+ public void testDoMultiSelectionRangeWithPaging() {
+ MultiSelectionModel<String> model = new MultiSelectionModel<String>();
+ display.setVisibleRange(10, 10);
+ display.setRowData(10, createData(10, 10));
+ display.setSelectionModel(model);
+
+ // Select range, but really only one value because nothing is selected.
+ manager.doMultiSelection(model, display, 13, "test 13", null, true, false);
+ assertSelected(model, "test 13");
+
+ // Select a range.
+ manager.doMultiSelection(model, display, 15, "test 15", null, true, false);
+ assertSelected(model, "test 13", "test 14", "test 15");
+ }
+
public void testDoMultiSelectionSelect() {
MultiSelectionModel<String> model = new MultiSelectionModel<String>();
display.setSelectionModel(model);
