Remove setHtml(SafeHtml) and other Html-related methods from Label.
This removes setHtml, setTextOrHtml, and getTextOrHtml from Label, and adds the appropriate tests.
Review at http://gwt-code-reviews.appspot.com/1011801
Review by: jlabanca@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@9081 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ui/HTML.java b/user/src/com/google/gwt/user/client/ui/HTML.java
index 6fe54bf..98c7fd3 100644
--- a/user/src/com/google/gwt/user/client/ui/HTML.java
+++ b/user/src/com/google/gwt/user/client/ui/HTML.java
@@ -17,6 +17,9 @@
import com.google.gwt.dom.client.Document;
import com.google.gwt.dom.client.Element;
+import com.google.gwt.i18n.client.BidiUtils;
+import com.google.gwt.i18n.shared.BidiFormatter;
+import com.google.gwt.i18n.shared.DirectionEstimator;
import com.google.gwt.safehtml.shared.SafeHtml;
/**
@@ -149,6 +152,38 @@
}
/**
+ * Sets the widget element's direction.
+ * @deprecated Use {@link #setDirectionEstimator} and / or pass explicit
+ * direction to {@link #setText} instead
+ */
+ @Deprecated
+ public void setDirection(Direction direction) {
+ BidiUtils.setDirectionOnElement(getElement(), direction);
+ initialElementDir = direction;
+
+ // For backwards compatibility, assure there's no span wrap, and update the
+ // content direction.
+ setInnerTextOrHtml(getTextOrHtml(true), true);
+ isSpanWrapped = false;
+ textDir = initialElementDir;
+ updateHorizontalAlignment();
+ }
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * Note: if the widget already has non-empty content, this will update
+ * its direction according to the new estimator's result. This may cause
+ * flicker, and thus should be avoided; DirectionEstimator should be set
+ * before the widget has any content.
+ */
+ public void setDirectionEstimator(DirectionEstimator directionEstimator) {
+ this.directionEstimator = directionEstimator;
+ // Refresh appearance
+ setHTML(getTextOrHtml(true));
+ }
+
+ /**
* Sets the label's content to the given HTML.
* See {@link #setText(String)} for details on potential effects on direction
* and alignment.
@@ -156,7 +191,21 @@
* @param html the new widget's HTML content
*/
public void setHTML(String html) {
- setTextOrHtml(html, true);
+ if (directionEstimator == null) {
+ isSpanWrapped = false;
+ getElement().setInnerHTML(html);
+
+ // Preserves the initial direction of the widget. This is different from
+ // passing the direction parameter explicitly as DEFAULT, which forces the
+ // widget to inherit the direction from its parent.
+ if (textDir != initialElementDir) {
+ textDir = initialElementDir;
+ BidiUtils.setDirectionOnElement(getElement(), initialElementDir);
+ updateHorizontalAlignment();
+ }
+ } else {
+ setHTML(html, directionEstimator.estimateDirection(html, true));
+ }
}
/**
@@ -170,7 +219,21 @@
* direction should be inherited from the widget's parent element.
*/
public void setHTML(String html, Direction dir) {
- setTextOrHtml(html, dir, true);
+ textDir = dir;
+
+ // Set the text and the direction.
+ if (isElementInline) {
+ isSpanWrapped = true;
+ getElement().setInnerHTML(BidiFormatter.getInstanceForCurrentLocale(
+ true /* alwaysSpan */).spanWrapWithKnownDir(dir, html, true));
+ } else {
+ isSpanWrapped = false;
+ BidiUtils.setDirectionOnElement(getElement(), dir);
+ getElement().setInnerHTML(html);
+ }
+
+ // Update the horizontal alignment if needed.
+ updateHorizontalAlignment();
}
/**
@@ -179,13 +242,25 @@
* @see com.google.gwt.safehtml.client.HasSafeHtml#setHTML(SafeHtml)
* @param html the html to set.
*/
- @Override
public void setHTML(SafeHtml html) {
setHTML(html.asString());
}
- @Override
public void setHTML(SafeHtml html, Direction dir) {
setHTML(html.asString(), dir);
}
+
+ protected String getTextOrHtml(boolean isHtml) {
+ Element element = isSpanWrapped ? getElement().getFirstChildElement()
+ : getElement();
+ return isHtml ? element.getInnerHTML() : element.getInnerText();
+ }
+
+ private void setInnerTextOrHtml(String content, boolean isHtml) {
+ if (isHtml) {
+ getElement().setInnerHTML(content);
+ } else {
+ getElement().setInnerText(content);
+ }
+ }
}
diff --git a/user/src/com/google/gwt/user/client/ui/Label.java b/user/src/com/google/gwt/user/client/ui/Label.java
index 9811f76..b689c3b 100644
--- a/user/src/com/google/gwt/user/client/ui/Label.java
+++ b/user/src/com/google/gwt/user/client/ui/Label.java
@@ -46,7 +46,6 @@
import com.google.gwt.i18n.shared.DirectionEstimator;
import com.google.gwt.i18n.shared.HasDirectionEstimator;
import com.google.gwt.i18n.shared.WordCountDirectionEstimator;
-import com.google.gwt.safehtml.shared.SafeHtml;
/**
* A widget that contains arbitrary text, <i>not</i> interpreted as HTML.
@@ -103,24 +102,19 @@
* The direction of the widget's content.
* Note: this may not match the direction of the widget's top DOM element
* ({@code getElement()}).
- * See {@link #setTextOrHtml(String, Direction, boolean)} for details.
+ * See {@link #setText(String, Direction)} for details.
*/
- private Direction textDir;
+ Direction textDir;
/**
* The widget's DirectionEstimator object.
*/
- private DirectionEstimator directionEstimator;
-
- /**
- * The widget's horizontal alignment.
- */
- private HorizontalAlignmentConstant horzAlign;
+ DirectionEstimator directionEstimator;
/**
* The initial direction of the widget's element.
*/
- private Direction initialElementDir;
+ Direction initialElementDir;
/**
* Whether the widget is inline (a <span> element).
@@ -132,7 +126,7 @@
* could be calculating the element's display property, but this may have some
* overhead, and is problematic when the element is yet unattached.
*/
- private boolean isElementInline;
+ boolean isElementInline;
/**
* Whether the widget contains a nested <span> element used to
@@ -144,7 +138,12 @@
* <span> must be used to carry the content's direction, with an LRM or
* RLM character afterwards to prevent the garbling.
*/
- private boolean isSpanWrapped;
+ boolean isSpanWrapped;
+
+ /**
+ * The widget's horizontal alignment.
+ */
+ private HorizontalAlignmentConstant horzAlign;
/**
* Creates an empty label.
@@ -169,6 +168,17 @@
}
/**
+ * Creates a label with the specified text.
+ *
+ * @param text the new label's text
+ * @param wordWrap <code>false</code> to disable word wrapping
+ */
+ public Label(String text, boolean wordWrap) {
+ this(text);
+ setWordWrap(wordWrap);
+ }
+
+ /**
* Creates a label with the specified text and direction.
*
* @param text the new label's text
@@ -181,17 +191,6 @@
}
/**
- * Creates a label with the specified text.
- *
- * @param text the new label's text
- * @param wordWrap <code>false</code> to disable word wrapping
- */
- public Label(String text, boolean wordWrap) {
- this(text);
- setWordWrap(wordWrap);
- }
-
- /**
* This constructor may be used by subclasses to explicitly use an existing
* element. This element must be either a <div> or <span> element.
*
@@ -297,7 +296,9 @@
}
public String getText() {
- return getTextOrHtml(false);
+ Element element = isSpanWrapped ? getElement().getFirstChildElement()
+ : getElement();
+ return element.getInnerText();
}
public Direction getTextDirection() {
@@ -356,7 +357,7 @@
// For backwards compatibility, assure there's no span wrap, and update the
// content direction.
- setInnerTextOrHtml(getTextOrHtml(true), true);
+ getElement().setInnerText(getText());
isSpanWrapped = false;
textDir = initialElementDir;
updateHorizontalAlignment();
@@ -382,7 +383,7 @@
public void setDirectionEstimator(DirectionEstimator directionEstimator) {
this.directionEstimator = directionEstimator;
// Refresh appearance
- setTextOrHtml(getTextOrHtml(true), true);
+ setText(getText());
}
/**
@@ -411,7 +412,21 @@
* @param text the widget's new text
*/
public void setText(String text) {
- setTextOrHtml(text, false);
+ if (directionEstimator == null) {
+ isSpanWrapped = false;
+ getElement().setInnerText(text);
+
+ // Preserves the initial direction of the widget. This is different from
+ // passing the direction parameter explicitly as DEFAULT, which forces the
+ // widget to inherit the direction from its parent.
+ if (textDir != initialElementDir) {
+ textDir = initialElementDir;
+ BidiUtils.setDirectionOnElement(getElement(), initialElementDir);
+ updateHorizontalAlignment();
+ }
+ } else {
+ setText(text, directionEstimator.estimateDirection(text, false));
+ }
}
/**
@@ -435,7 +450,21 @@
* direction should be inherited from the widget's parent element.
*/
public void setText(String text, Direction dir) {
- setTextOrHtml(text, dir, false);
+ textDir = dir;
+
+ // Set the text and the direction.
+ if (isElementInline) {
+ isSpanWrapped = true;
+ getElement().setInnerHTML(BidiFormatter.getInstanceForCurrentLocale(
+ true /* alwaysSpan */).spanWrapWithKnownDir(dir, text, false));
+ } else {
+ isSpanWrapped = false;
+ BidiUtils.setDirectionOnElement(getElement(), dir);
+ getElement().setInnerText(text);
+ }
+
+ // Update the horizontal alignment if needed.
+ updateHorizontalAlignment();
}
public void setWordWrap(boolean wrap) {
@@ -443,116 +472,11 @@
wrap ? "normal" : "nowrap");
}
- protected String getTextOrHtml(boolean isHtml) {
- Element element = isSpanWrapped ? getElement().getFirstChildElement()
- : getElement();
- return isHtml ? element.getInnerHTML() : element.getInnerText();
- }
-
- /**
- * Sets the label's content to the given safe html. See
- * {@link #setText(String)} for details on potential effects on direction and
- * alignment.
- *
- * @param html the widget's new safe html
- */
- protected void setHTML(SafeHtml html) {
- setTextOrHtml(html.asString(), true);
- }
-
- /**
- * Sets the label's content to the given safe html. See
- * {@link #setText(String)} for details on potential effects on direction and
- * alignment.
- *
- * @param html the widget's new safe html
- * @param dir the content's direction
- */
- protected void setHTML(SafeHtml html, Direction dir) {
- setTextOrHtml(html.asString(), dir, true);
- }
-
- /**
- * Sets the label's content to the given value (either plain text or HTML).
- * See {@link #setText(String)} for details on potential effects on direction
- * and alignment.
- *
- * @param content the widget's new content
- * @param isHtml whether the content is HTML
- */
- protected void setTextOrHtml(String content, boolean isHtml) {
- if (directionEstimator == null) {
- isSpanWrapped = false;
- setInnerTextOrHtml(content, isHtml);
-
- // Preserves the initial direction of the widget. This is different from
- // passing the direction parameter explicitly as DEFAULT, which forces the
- // widget to inherit the direction from its parent.
- if (textDir != initialElementDir) {
- textDir = initialElementDir;
- BidiUtils.setDirectionOnElement(getElement(), initialElementDir);
- updateHorizontalAlignment();
- }
- } else {
- setTextOrHtml(content, directionEstimator.estimateDirection(content,
- isHtml), isHtml);
- }
- }
-
- /**
- * Sets the label's content to the given value (either plain text or HTML),
- * applying the given direction. See
- * {@link #setText(String, com.google.gwt.i18n.client.HasDirection.Direction)
- * setText(String, Direction)} for details on potential effects on alignment.
- * <p>
- * Implementation details:
- * <ul>
- * <li>If the widget's element is a <div>, sets its dir attribute
- * according to the given direction.
- * <li>Otherwise (i.e. the widget's element is a <span>), the direction
- * is set using a nested <span dir=...> element which holds the content
- * of the widget. This nested span may be followed by a zero-width Unicode
- * direction character (LRM or RLM). This manipulation is necessary to prevent
- * garbling in case the direction of the widget is opposite to the direction
- * of its context. See {@link com.google.gwt.i18n.shared.BidiFormatter} for
- * more details.
- * </ul>
- *
- * @param content the widget's new content
- * @param dir the content's direction
- * @param isHtml whether the content is HTML
- */
- protected void setTextOrHtml(String content, Direction dir, boolean isHtml) {
- textDir = dir;
-
- // Set the text and the direction.
- if (isElementInline) {
- isSpanWrapped = true;
- getElement().setInnerHTML(BidiFormatter.getInstanceForCurrentLocale(
- true /* alwaysSpan */).spanWrapWithKnownDir(dir, content, isHtml));
- } else {
- isSpanWrapped = false;
- BidiUtils.setDirectionOnElement(getElement(), dir);
- setInnerTextOrHtml(content, isHtml);
- }
-
- // Update the horizontal alignment if needed.
- updateHorizontalAlignment();
- }
-
- private void setInnerTextOrHtml(String content, boolean isHtml) {
- if (isHtml) {
- getElement().setInnerHTML(content);
- } else {
- getElement().setInnerText(content);
- }
- }
-
/**
* Sets the horizontal alignment of the widget according to the current
* AutoHorizontalAlignment setting.
*/
- private void updateHorizontalAlignment() {
+ protected void updateHorizontalAlignment() {
HorizontalAlignmentConstant align;
if (autoHorizontalAlignment == null) {
align = null;
diff --git a/user/test/com/google/gwt/user/client/ui/HTMLTest.java b/user/test/com/google/gwt/user/client/ui/HTMLTest.java
index ee84fc1..bdb50b9 100644
--- a/user/test/com/google/gwt/user/client/ui/HTMLTest.java
+++ b/user/test/com/google/gwt/user/client/ui/HTMLTest.java
@@ -37,6 +37,13 @@
return "com.google.gwt.user.User";
}
+ public void testDirectionEstimator() {
+ HTML html = new HTML();
+ html.setText("<b>bar</b>", Direction.RTL);
+ html.setDirectionEstimator(true);
+ assertEquals("<b>bar</b>", html.getText());
+ }
+
// test that the SafeHtml constructor creates the HTML element correctly.
public void testSafeHtmlConstructor() {
HTML htmlElement = new HTML(SafeHtmlUtils.fromSafeConstant(html));
@@ -111,6 +118,18 @@
assertEquals(Direction.LTR, htmlElement.getDirection());
}
+ public void testSetText() {
+ // test that setting plain text works
+ HTML html1 = new HTML();
+ html1.setText("<b>test</b>");
+ assertEquals("<b>test</b>", html1.getText());
+
+ // test that setting plain text with direction works
+ HTML html2 = new HTML();
+ html2.setText("<b>foo</b>", Direction.RTL);
+ assertEquals("<b>foo</b>", html2.getText());
+ }
+
/**
* Asserts that both the {@link Label#getTextDirection} and the physical dir
* attribute match the expected direction.
diff --git a/user/test/com/google/gwt/user/client/ui/LabelTest.java b/user/test/com/google/gwt/user/client/ui/LabelTest.java
index 00a949d..98276d0 100644
--- a/user/test/com/google/gwt/user/client/ui/LabelTest.java
+++ b/user/test/com/google/gwt/user/client/ui/LabelTest.java
@@ -22,7 +22,6 @@
import com.google.gwt.i18n.client.BidiUtils;
import com.google.gwt.i18n.client.HasDirection.Direction;
import com.google.gwt.junit.client.GWTTestCase;
-import com.google.gwt.safehtml.shared.SafeHtmlUtils;
import com.google.gwt.user.client.ui.HasHorizontalAlignment.AutoHorizontalAlignmentConstant;
import com.google.gwt.user.client.ui.HasHorizontalAlignment.HorizontalAlignmentConstant;
@@ -125,17 +124,23 @@
HasAutoHorizontalAlignment.ALIGN_CONTENT_START);
}
- @SuppressWarnings("deprecation")
- public void testSetSafeHtml() {
- Label label = new Label("foo");
- label.setHTML(SafeHtmlUtils.fromSafeConstant(html1));
-
- assertEquals(html1, label.getTextOrHtml(true).toLowerCase());
-
- label.setHTML(SafeHtmlUtils.fromSafeConstant(html2), Direction.LTR);
-
- assertEquals(html2, label.getTextOrHtml(true).toLowerCase());
+ public void testSetDirection() {
+ Label label = new Label(createAttachedSpanElement());
+ label.setDirectionEstimator(true);
+ label.setText(IW_TEXT);
+
+ // Should be span wrapped.
+ assertTrue(label.getElement().getInnerHTML().toLowerCase().contains("span"));
+
+ // Should not be span wrapped.
+ label.setDirection(Direction.RTL);
+ assertEquals(Direction.RTL, label.getDirection());
+ assertFalse(label.getElement().getInnerHTML().toLowerCase().contains("span"));
+
+ // Should not be span wrapped.
+ label.setDirection(Direction.LTR);
assertEquals(Direction.LTR, label.getDirection());
+ assertFalse(label.getElement().getInnerHTML().toLowerCase().contains("span"));
}
/**
