Revision: 9954
Author: [email protected]
Date: Thu Apr 7 05:39:53 2011
Log: 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: [email protected]
http://code.google.com/p/google-web-toolkit/source/detail?r=9954
Added:
/trunk/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplStandardBase.java
Deleted:
/trunk/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java
Modified:
/trunk/tools/api-checker/config/gwt22_23userApi.conf
/trunk/user/src/com/google/gwt/cell/client/CompositeCell.java
/trunk/user/src/com/google/gwt/cell/client/EditTextCell.java
/trunk/user/src/com/google/gwt/cell/client/TextInputCell.java
/trunk/user/src/com/google/gwt/user/cellview/CellView.gwt.xml
/trunk/user/src/com/google/gwt/view/client/DefaultSelectionEventManager.java
/trunk/user/test/com/google/gwt/cell/client/CompositeCellTest.java
/trunk/user/test/com/google/gwt/cell/client/EditTextCellTest.java
/trunk/user/test/com/google/gwt/cell/client/TextInputCellTest.java
/trunk/user/test/com/google/gwt/view/client/DefaultSelectionEventManagerTest.java
=======================================
--- /dev/null
+++
/trunk/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplStandardBase.java
Thu Apr 7 05:39:53 2011
@@ -0,0 +1,31 @@
+/*
+ * 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
+ * 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.gwt.user.cellview.client;
+
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
+
+/**
+ * StandardBase implementation of {@link CellBasedWidgetImpl}.
+ */
+public class CellBasedWidgetImplStandardBase extends
CellBasedWidgetImplStandard {
+
+ @Override
+ public void resetFocus(ScheduledCommand command) {
+ // Some browsers will not focus an element that was created in this
event loop.
+ Scheduler.get().scheduleDeferred(command);
+ }
+}
=======================================
---
/trunk/user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImplSafari.java
Wed Oct 13 12:59:56 2010
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- * Copyright 2010 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
- * 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.gwt.user.cellview.client;
-
-import com.google.gwt.core.client.Scheduler;
-import com.google.gwt.core.client.Scheduler.ScheduledCommand;
-
-/**
- * Webkit specified Impl used by cell based widgets.
- */
-public class CellBasedWidgetImplSafari extends CellBasedWidgetImplStandard
{
-
- @Override
- public void resetFocus(ScheduledCommand command) {
- // Webkit will not focus an element that was created in this event
loop.
- Scheduler.get().scheduleDeferred(command);
- }
-}
=======================================
--- /trunk/tools/api-checker/config/gwt22_23userApi.conf Tue Apr 5
10:47:39 2011
+++ /trunk/tools/api-checker/config/gwt22_23userApi.conf Thu Apr 7
05:39:53 2011
@@ -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
=======================================
--- /trunk/user/src/com/google/gwt/cell/client/CompositeCell.java Mon Mar
21 12:22:19 2011
+++ /trunk/user/src/com/google/gwt/cell/client/CompositeCell.java Thu Apr
7 05:39:53 2011
@@ -111,6 +111,18 @@
public boolean handlesSelection() {
return handlesSelection;
}
+
+ @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,
@@ -198,6 +210,11 @@
cell.render(context, hasCell.getValue(value), sb);
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,
=======================================
--- /trunk/user/src/com/google/gwt/cell/client/EditTextCell.java Wed Dec 1
05:40:20 2010
+++ /trunk/user/src/com/google/gwt/cell/client/EditTextCell.java Thu Apr 7
05:39:53 2011
@@ -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));
}
}
=======================================
--- /trunk/user/src/com/google/gwt/cell/client/TextInputCell.java Wed Dec
1 05:40:20 2010
+++ /trunk/user/src/com/google/gwt/cell/client/TextInputCell.java Thu Apr
7 05:39:53 2011
@@ -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>");
}
=======================================
--- /trunk/user/src/com/google/gwt/user/cellview/CellView.gwt.xml Fri Mar
11 09:47:37 2011
+++ /trunk/user/src/com/google/gwt/user/cellview/CellView.gwt.xml Thu Apr
7 05:39:53 2011
@@ -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>
=======================================
---
/trunk/user/src/com/google/gwt/view/client/DefaultSelectionEventManager.java
Mon Mar 21 12:22:19 2011
+++
/trunk/user/src/com/google/gwt/view/client/DefaultSelectionEventManager.java
Thu Apr 7 05:39:53 2011
@@ -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));
}
=======================================
--- /trunk/user/test/com/google/gwt/cell/client/CompositeCellTest.java Wed
Dec 1 05:40:20 2010
+++ /trunk/user/test/com/google/gwt/cell/client/CompositeCellTest.java Thu
Apr 7 05:39:53 2011
@@ -82,6 +82,42 @@
assertTrue(cell.getConsumedEvents().contains("click"));
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());
=======================================
--- /trunk/user/test/com/google/gwt/cell/client/EditTextCellTest.java Wed
Dec 1 05:40:20 2010
+++ /trunk/user/test/com/google/gwt/cell/client/EditTextCellTest.java Thu
Apr 7 05:39:53 2011
@@ -136,6 +136,33 @@
cell.render(context, "originalValue", sb);
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.
=======================================
--- /trunk/user/test/com/google/gwt/cell/client/TextInputCellTest.java Wed
Sep 22 12:58:01 2010
+++ /trunk/user/test/com/google/gwt/cell/client/TextInputCellTest.java Thu
Apr 7 05:39:53 2011
@@ -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}.
@@ -41,6 +43,19 @@
testOnBrowserEvent(getExpectedInnerHtml(), event, "oldValue", null,
null,
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() {
=======================================
---
/trunk/user/test/com/google/gwt/view/client/DefaultSelectionEventManagerTest.java
Tue Mar 1 12:21:09 2011
+++
/trunk/user/test/com/google/gwt/view/client/DefaultSelectionEventManagerTest.java
Thu Apr 7 05:39:53 2011
@@ -201,6 +201,24 @@
manager.doMultiSelection(model, display, 3, "test 3", null, false,
true);
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>();
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
