242982 - 100% cpu in any view, no events (workaround 1.7/1.8 getElementsByAttribute incompatibility)

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040421
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040421

Switching to any of the views hangs with 100% cpu when running calendar code
from cvs on Moz1.7.  No events are displayed in grids.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.




Problem is due to changes made for Bug 240186, which changes the behavior of
document.getElementsByAttribute.  The result in 1.8 is a 'live' list, so adding
or removing elements of the list while traversing the list changes the index of
all elements after it.  The changes made to accommodate the new 'live' behavior
are not compatible with the old behavior.

In particular, many places where the code did something like

  var selectedBoxes = getElementsByAttribute("selected","true");
  for (var i = 0; i < selectedBoxes.length; i++)
     selectedBoxes[i].parentNode.removeChild(selectedBoxes[i]);

were changed to something like

  var selectedBoxes = getElementsByAttribute("selected","true");
  while(selectedBoxes.item(0)) {
     var box = selectedBoxes.item(0);
     box.parentNode.removeChild(box);
  }
Note removing the node from its parent also removes it from the 'live' list.

Similarly, code that did something like

  var markedBoxes = document.getElementByAttribute("selected","true");
  for (var i = 0; i < markedBoxes.length; i++) 
     markesBoxes[i].removeAttribute("selected");

was changed to 

  var markedBoxes = document.getElementByAttribute("selected","true");
  while(markedBoxes.item(0)) {
     markedBoxes.item(0).removeAttribute("selected"):
  }

Note changing the attribute also removes it from the 'live' list.

But the new 'live' behavior of the lists returned by getElementsByAttribute is
incompatible with the old 'dead' behavior. 

A workaround that should work with both the old 1.7 behavior and the new 1.8
behavior is to copy the list into a temporary array first, and then delete the
elements of the array.

  var liveList = document.getElementByAttribute("selected","true");
  var deadArray = new Array(liveList.length);
  for (var i = 0; i < deadArray.length; i++)
     deadArray[i] = liveList.item(i);
  liveList = null;
  for (var i = 0; i < deadArray.length; i++)
     deadArray[i].removeAttribute("selected"):

Comment 1

[gekacheka]

Attachment: Patch calendarWindow.js (base class methods)

Adds CalendarView utility methods called by all derived views.
  removeElementsByAttribute
  removeAttributeFromElements
  getArrayOfElementsByAttribute (called by above 2)

Comment 2

[gekacheka]

Attachment: dayView.js patch: call calendarView methods

Patch dayView to call calendarView methods.
   removeElementsByAttribute
   removeAttributeFromElements

Comment 3

[gekacheka]

Attachment: weekView.js patch: call calendarView methods

Patch weekView.js to call calendarView methods
  removeElementsByAttribute
  removeAttributeFromElements

Comment 4

[gekacheka]

Attachment: multiweekView.js patch: call calendarView methods

Patch week view to call calendar view methods
  removeElementsByAttribute
  removeAttributeFromElements

Comment 5

[gekacheka]

Attachment: monthView.js patch: call calendarView methods

Patch monthView.js to call calendarView methods
  removeElementsByAttribute
  removeAttributeFromElements

Comment 6

[gekacheka]

Attachment: dragDrop.js: copy result of getElementsByAttribute before removing same attribute "draggedover"

Comment 7

[gekacheka]

Attachment: unifinderTodo.js: copy result of getElementsByAttribute before removing same attribute "checked"

Comment 8

[Boris Zbarsky [:bzbarsky]]

Or you could walk the list in reverse order and delete elements -- that would
work with both the new and old behavior, no?

That said, that's much slower than the code I wrote (which is why I wrote the
code as I did); I was not aware that current calendar XPIs had the requirement
to be installable on old builds....

Comment 9

[gekacheka]

Attachment: calendarWindow.js patch (base class methods)

Updated to remove in reverse order, avoiding copy.

Comment 10

[gekacheka]

Attachment: dragDrop.js patch (remove attributes in reverse order)

Removes in reverse order, avoiding copy.

Comment 11

[gekacheka]

Attachment: unifinderTodo.patch (remove in reverse order)

Remove in reverse order, avoid copy.

Comment 12

[Mostafa Hosseini]

All reviewed patches checked into CVS.
Problem fixed and the code looks nicer.
Thanks a lot.

Comment 13

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Sunbird Only. Be afraid for your sanity!