Replace getElementsByClassName() with select().

Adrian Yee pointed out that `getElementsByClassName()` will find all elements with that class in the entire document, whereas calling Prototype's `select()` with a class name selector on a single element will find only elements with that class name under the element. This is more efficient, and much more likely to be correct.
bricoleurs · Mar 19, 2010 · 3b86f7b · 3b86f7b
1 parent 2ea3cfa
commit 3b86f7b
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 8 deletions.
diff --git a/comp/media/js/lib.js b/comp/media/js/lib.js
@@ -358,8 +358,8 @@ This function is called when a position drop down is changed in a story profile.
 function reorder(obj, container) {
 
     var container = $(container);
-    var selects = $A(document.getElementsByClassName("reorder", container));
-    var newIndex = obj.selectedIndex;
+    var selects   = container.select('.reorder');
+    var newIndex  = obj.selectedIndex;
 
     var order = $A();
     selects.each(function(select) {
@@ -1281,7 +1281,7 @@ FastAdd.prototype = {
         ]);
 
         var placed = false;
-        $A(document.getElementsByClassName('value', this.list)).each((function(sibling) {
+        this.list.select('.value').each((function(sibling) {
             if (Element.collectTextNodes(sibling).toLowerCase() > value.toLowerCase()) {
                 this.list.insertBefore(item, sibling.parentNode);
                 placed = true;
@@ -1306,8 +1306,8 @@ FastAdd.prototype = {
 var Tabs = Class.create();
 Tabs.prototype = {
     initialize: function(tabGroup, pageGroup) {
-        this.tabs = document.getElementsByClassName('tab', $(tabGroup));
-        this.pages = document.getElementsByClassName('page', $(pageGroup));
+        this.tabs  = $(tabGroup).select('.tab');
+        this.pages = $(pageGroup).select('.page');
 
         var selected = this.tabs.first();
         this.tabs.each(function(tab) {
@@ -1717,7 +1717,7 @@ var Container = {
 };
 
 Event.observe(window, 'load', function() {
-  $A(document.getElementsByClassName('listManager')).each(function(table) {
+  $$('.listManager').each(function(table) {
       alternateTableRows(table);
   })
 });
diff --git a/comp/widgets/media_prof/edit_meta.html b/comp/widgets/media_prof/edit_meta.html
@@ -175,7 +175,7 @@
     function deleteContrib(button) {
         var id = $(button).value;
         var index = $('contrib_order_' + id).selectedIndex;
-        $A(document.getElementsByClassName('reorder', 'contribs')).each(function(select) {
+        $('contribs').select('.reorder').each(function(select) {
             if (select.selectedIndex > index) select.selectedIndex--;
             Element.remove(select.options[select.options.length - 1]);
         });

diff --git a/comp/widgets/story_prof/edit_meta.html b/comp/widgets/story_prof/edit_meta.html
@@ -181,7 +181,7 @@
     function deleteContrib(button) {
         var id = $(button).value;
         var index = $('contrib_order_' + id).selectedIndex;
-        $A(document.getElementsByClassName('reorder', 'contribs')).each(function(select) {
+        $('contribs').select('.reorder').each(function(select) {
             if (select.selectedIndex > index) select.selectedIndex--;
             Element.remove(select.options[select.options.length - 1]);
         });

diff --git a/lib/Bric/Changes.pod b/lib/Bric/Changes.pod
@@ -163,6 +163,11 @@ The C<best_uri()> method of L<Bric::Util::Burner> now avoids expired stories
 when searching for aliased stories. Thanks to Zdravko Balorda for the catch
 (Bug #144). [David]
 
+=item *
+
+Fixed issue with the use of C<findElementsByClass()> when used with Prototype
+1.6.0.3. Thanks to Adrian Yee for the report and suggested fix. [David]
+
 =back
 
 =head1 Version 1.11.3 (2010-01-28)