yaronn / blessed-contrib

Build terminal dashboards using ascii/ansi art and javascript
MIT License
15.5k stars 840 forks source link

infinite loop when scrollable tree is focused causes `RangeError: Maximum call stack size exceeded` #219

Open chinggg opened 3 years ago

chinggg commented 3 years ago

I am using TreeElement to develop an app but it always crash when TreeElement is focused. I make breakpoints according to the traceback and finally understand the infinite loop. (see wechaty/cli#9)

Tree.prototype.focus = function() {
  this.rows.focus();   // this.rows: blessed.Widgets.ListElement
};

// if any element is focused
Element.prototype.focus = function() {
  return this.screen.focused = this;
};

The callback of Screen.prototype.__defineSetter__('focused', el) will then call this.focusPush(el). The last statement in focusPush() is this._focus(el, old), where old is the tail of array this.history.

The trigger of infinite loop lies in function Screen.prototype._focus():

Screen.prototype._focus = function(self, old) {
  // Find a scrollable ancestor if we have one.
  var el = self;
  while (el = el.parent) {
    if (el.scrollable) break;
  }

  // If we're in a scrollable element,
  // automatically scroll to the focused element.
  if (el && !el.detached) {
    // NOTE: This is different from the other "visible" values - it needs the
    // visible height of the scrolling element itself, not the element within
    // it.
    var visible = self.screen.height - el.atop - el.itop - el.abottom - el.ibottom;
    if (self.rtop < el.childBase) {
      el.scrollTo(self.rtop);
      self.screen.render();
    } else if (self.rtop + self.height - self.ibottom > el.childBase + visible) {
      // Explanation for el.itop here: takes into account scrollable elements
      // with borders otherwise the element gets covered by the bottom border:
      el.scrollTo(self.rtop - (el.height - self.height) + el.itop, true);
      self.screen.render();
    }
  }
  // omitted
};

The initial value of el is still the ListElement inside TreeElement, and el.parent is the TreeElement. I find the tree scrollable, so the while loop breaks with el = TreeElement, then screen.render() is likely to be called in the following if-statements.

screen.render() will make childrens of the screen to render, so the TreeElement will be rendered. The if-statement in functioon Tree.prototype.render(), if evaluated true, will focus the ListElement again, which causes the infinite loop.

Tree.prototype.render = function() {
  if (this.screen.focused === this.rows) this.rows.focus();

  this.rows.width = this.width - 3;
  this.rows.height = this.height - 3;
  Box.prototype.render.call(this);
};

To sum up, tree.rows.focus() calls screen._focus(), screen._focus() will call screen.render() if tree.scrollable is true. screen.render() calls tree.render(), tree.render() will call tree.rows.focus() again if tree.screen.focused === tree.rows, which causes the infinite loop.

The example/explorer.js will not cause infinite loop, I debug it and find tree.scrollable is undefined so it will not crash. But in my app I set the scrollable option.

if (this.screen.focused === this.rows) this.rows.focus();, this line really confused me.

chinggg commented 3 years ago

To reproduce in example/explorer.js, add scrollable: true to tree option, it will throw RangeError: Maximum call stack size exceeded immediately.