Make ListBox's bidi support more robust, allowing subclasses to create option elements without going through ListBox. Such subclasses are currently broken by ListBox bidi support's use of an array, itemTexts, that is assumed to be synchronized with the select's option element descendants. The fix is to drop the array and instead keep the bidi data (when needed) as an attribute on the option elements.
Review at http://gwt-code-reviews.appspot.com/1357806
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@9734 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ui/ListBox.java b/user/src/com/google/gwt/user/client/ui/ListBox.java
index 371e71c..afe11c5 100644
--- a/user/src/com/google/gwt/user/client/ui/ListBox.java
+++ b/user/src/com/google/gwt/user/client/ui/ListBox.java
@@ -29,8 +29,6 @@
import com.google.gwt.i18n.shared.HasDirectionEstimator;
import com.google.gwt.i18n.shared.WordCountDirectionEstimator;
-import java.util.ArrayList;
-
/**
* A widget that presents a list of choices to the user, either as a list box or
* as a drop-down list.
@@ -78,6 +76,12 @@
* </g:item>
* </g:ListBox>
* </pre>
+ * <p>
+ * <h3>Important usage note</h3>
+ * <b>Subclasses should neither read nor write option text directly from the
+ * option elements created by this class, since such text may need to be wrapped
+ * in Unicode bidi formatting characters. They can use the getOptionText and/or
+ * setOptionText methods for this purpose instead.</b>
*/
@SuppressWarnings("deprecation")
public class ListBox extends FocusWidget implements SourcesChangeEvents,
@@ -86,6 +90,8 @@
public static final DirectionEstimator DEFAULT_DIRECTION_ESTIMATOR =
WordCountDirectionEstimator.get();
+ private static final String BIDI_ATTR_NAME = "bidiwrapped";
+
private static final int INSERT_AT_END = -1;
/**
@@ -112,7 +118,6 @@
}
private DirectionEstimator estimator;
- private ArrayList<String> itemTexts = new ArrayList<String>();
/**
* Creates an empty list box in single selection mode.
@@ -236,7 +241,7 @@
*/
public String getItemText(int index) {
checkIndex(index);
- return itemTexts.get(index);
+ return getOptionText(getSelectElement().getOptions().getItem(index));
}
public String getName() {
@@ -340,14 +345,13 @@
public void insertItem(String item, Direction dir, String value, int index) {
SelectElement select = getSelectElement();
OptionElement option = Document.get().createOptionElement();
- option.setText(unicodeWrapIfNeeded(item, dir));
+ setOptionText(option, item, dir);
option.setValue(value);
int itemCount = select.getLength();
if (index < 0 || index > itemCount) {
index = itemCount;
}
- itemTexts.add(index, item);
if (index == itemCount) {
select.add(option, null);
} else {
@@ -395,7 +399,6 @@
public void removeItem(int index) {
checkIndex(index);
getSelectElement().remove(index);
- itemTexts.remove(index);
}
/**
@@ -456,9 +459,7 @@
if (text == null) {
throw new NullPointerException("Cannot set an option to have null text");
}
- getSelectElement().getOptions().getItem(index).setText(unicodeWrapIfNeeded(
- text, dir));
- itemTexts.set(index, text);
+ setOptionText(getSelectElement().getOptions().getItem(index), text, dir);
}
/**
@@ -522,6 +523,22 @@
}
/**
+ * Retrieves the text of an option element. If the text was set by
+ * {@link #setOptionText} and was wrapped with Unicode bidi formatting
+ * characters, also removes those additional formatting characters.
+ *
+ * @param option an option element
+ * @return the element's text
+ */
+ protected String getOptionText(OptionElement option) {
+ String text = option.getText();
+ if (option.hasAttribute(BIDI_ATTR_NAME) && text.length() > 1) {
+ text = text.substring(1, text.length() - 1);
+ }
+ return text;
+ }
+
+ /**
* <b>Affected Elements:</b>
* <ul>
* <li>-item# = the option at the specified index.</li>
@@ -541,6 +558,38 @@
}
}
+ /**
+ * Sets the text of an option element. If the direction of the text is
+ * opposite to the page's direction, also wraps it with Unicode bidi
+ * formatting characters to prevent garbling, and indicates that this was done
+ * by setting the option's <code>BIDI_ATTR_NAME</code> custom attribute.
+ *
+ * @param option an option element
+ * @param text text to be set to the element
+ * @param dir the text's direction. If {@code null} and direction estimation
+ * is turned off, direction is ignored.
+ */
+ protected void setOptionText(OptionElement option, String text,
+ Direction dir) {
+ if (dir == null && estimator != null) {
+ dir = estimator.estimateDirection(text);
+ }
+ if (dir == null) {
+ option.setText(text);
+ option.removeAttribute(BIDI_ATTR_NAME);
+ } else {
+ String formattedText =
+ BidiFormatter.getInstanceForCurrentLocale().unicodeWrapWithKnownDir(
+ dir, text, false /* isHtml */, false /* dirReset */);
+ option.setText(formattedText);
+ if (formattedText.length() > text.length()) {
+ option.setAttribute(BIDI_ATTR_NAME, "");
+ } else {
+ option.removeAttribute(BIDI_ATTR_NAME);
+ }
+ }
+ }
+
private void checkIndex(int index) {
if (index < 0 || index >= getItemCount()) {
throw new IndexOutOfBoundsException();
@@ -550,13 +599,4 @@
private SelectElement getSelectElement() {
return getElement().cast();
}
-
- private String unicodeWrapIfNeeded(String text, Direction dir) {
- if (dir == null && estimator != null) {
- dir = estimator.estimateDirection(text);
- }
- return dir == null ? text :
- BidiFormatter.getInstanceForCurrentLocale().unicodeWrapWithKnownDir(dir,
- text, false /* isHtml */, false /* dirReset */);
- }
}
