Reviewers: Dan Rice,
Description:
Fix to keep FocusWidget's setElement() implementation from clobbering
tabindex when it's already set. This comes up in practice when calling,
e.g.,
TextBox.wrap() on a static element that already had a perfectly good
tabindex.
Second attempt at this patch -- the first one broke tests because of the
weird DOM structure created by CheckBox/RadioButton. This one uses
onAttach()
rather than setElement(), which is cleaner anyway, and I've tested it
pretty
thoroughly.
Please review this at http://gwt-code-reviews.appspot.com/153817
Affected files:
M user/src/com/google/gwt/user/client/ui/FocusWidget.java
M user/test/com/google/gwt/user/client/ui/TextBoxTest.java
Index: user/src/com/google/gwt/user/client/ui/FocusWidget.java
===================================================================
--- user/src/com/google/gwt/user/client/ui/FocusWidget.java (revision 7639)
+++ user/src/com/google/gwt/user/client/ui/FocusWidget.java (working copy)
@@ -264,15 +264,17 @@
}
@Override
- protected void setElement(com.google.gwt.user.client.Element elem) {
- super.setElement(elem);
+ protected void onAttach() {
+ super.onAttach();
// Accessibility: setting tab index to be 0 by default, ensuring
element
- // appears in tab sequence. Note that this call will not interfere with
- // any calls made to FocusWidget.setTabIndex(int) by user code, because
- // FocusWidget.setTabIndex(int) cannot be called until setElement(elem)
- // has been called.
- setTabIndex(0);
- }
-
+ // appears in tab sequence. We must ensure that the element doesn't
already
+ // have a tabIndex set. This is not a problem for normal widgets, but
when
+ // a widget is used to wrap an existing static element, it can already
have
+ // a tabIndex.
+ int tabIndex = getTabIndex();
+ if (-1 == tabIndex) {
+ setTabIndex(0);
+ }
+ }
}
Index: user/test/com/google/gwt/user/client/ui/TextBoxTest.java
===================================================================
--- user/test/com/google/gwt/user/client/ui/TextBoxTest.java (revision 7639)
+++ user/test/com/google/gwt/user/client/ui/TextBoxTest.java (working copy)
@@ -14,6 +14,9 @@
* the License.
*/
package com.google.gwt.user.client.ui;
+
+import com.google.gwt.dom.client.DivElement;
+import com.google.gwt.dom.client.Document;
/**
* Testing TextBox.
@@ -32,7 +35,7 @@
// As our setText does not honor max length, no way to text it in the
wild
// here.
}
-
+
public void testMinLength() {
TextBox b = createTextBoxBase();
b.setVisibleLength(5);
@@ -44,4 +47,14 @@
// Now check visible length.
assertEquals(5, b.getVisibleLength());
}
+
+ public void testNoNukeTabIndex() {
+ Document doc = Document.get();
+ DivElement div = doc.createDivElement();
+ div.setInnerHTML("<input type='text' id='tb' tabindex='1'></input>");
+ doc.getBody().appendChild(div);
+
+ TextBox tb = TextBox.wrap(doc.getElementById("tb"));
+ assertEquals(1, tb.getTabIndex());
+ }
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
