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")); } /**