> Hi Edwin, > > These changes look good, r=mars. I'm surprised by how subtle the IE fixes > are. > > The one question I had was: do we need the tabIndex attribute to be readonly, > or is just setting to '-1' sufficient? Did we just prevent anyone from ever > passing their own tabIndex into the constructor of the Widget? Put another > way, does this effectively lock up tabIndex with "final int tabIndex = -1"? > > I have only a few suggestions for polish: > > • lines 147,170: Use .one() instead of .query() > • In the comment on line 192, the explanation is excellent, but please also > explain why you chose to use get('tabIndex') instead of > getAttribute('tabIndex') > • 374: instead of a for-in loop, which can be error-prone, consider using > Y.each() > > > Maris Hi Maris, Thanks for the review. I've addressed all the changes you requested, and we discussed your question on irc. After I merged in trunk, everything broke because: 1. src-js/lazrjs/yui/current/build became src-js/lazrjs/yui/ (removing two levels of directories) 2. build/yui/current/build became build/ (removing three levels of directories) This caused all the jstestdriver to fail until I updated the widgets.conf. BTW, if we upgrade to jstestdriver 1.2, we get a much clearer error message when files listed in widgets.conf don't exist, however, it causes firefox to display "not responding" errors that breaks the connection with jstestdriver. I'm not sure if Sidnei did anything to our current version of jstestdriver to prevent this from happening, but that is really not necessary to get this branch landed. Even after jstestdriver was working, running the individual tests manually still didn't work until I updated all of them to point at the new directory. Most of the examples had already been updated. I verified that all the examples work except for examples/actions/index.html and examples/combo.html, which appear to be incomplete. Please review these changes. Incremental diff: {{{ === modified file 'examples/actions/index.html' --- examples/actions/index.html 2009-11-18 23:09:08 +0000 +++ examples/actions/index.html 2009-11-24 22:40:02 +0000 @@ -2,8 +2,8 @@ "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> - Lazr-js examples: Activator - + Lazr-js examples: Actions + @@ -12,7 +12,7 @@ YUI({ - base: '../../build/yui/current/build/', + base: '../../build/', filter: 'raw' }).use('node', 'lazr.actions', function(Y) { @@ -77,9 +77,9 @@ We need to include individual css files because some of them have relative paths to images. --> - - - + + + === modified file 'examples/combo.html' --- examples/combo.html 2009-11-19 17:27:44 +0000 +++ examples/combo.html 2009-11-24 22:33:21 +0000 @@ -128,7 +128,7 @@

Javascript

-    <script type="text/javascript" src="../../build/yui/current/build/yui/yui-min.js"></script>
+    <script type="text/javascript" src="../../build/yui/yui-min.js"></script>
     <script type="text/javascript" src="../../build/lazr/lazr-meta.js"></script>
   
=== modified file 'examples/formoverlay/index.html' --- examples/formoverlay/index.html 2009-11-18 21:24:47 +0000 +++ examples/formoverlay/index.html 2009-11-24 22:33:21 +0000 @@ -123,7 +123,7 @@

Javascript

-    <script type="text/javascript" src="../../build/yui/current/build/yui/yui-min.js"></script>
+    <script type="text/javascript" src="../../build/yui/yui-min.js"></script>
     <script type="text/javascript" src="../../build/lazr/lazr-meta.js"></script>
   
=== modified file 'examples/lazr/index.html' --- examples/lazr/index.html 2009-11-24 21:34:47 +0000 +++ examples/lazr/index.html 2009-11-24 05:22:22 +0000 @@ -49,9 +49,6 @@