Improve getNode() so that is finds the correct node. #1447

[dpvc]

Improves getNode() so that it doesn't find elements inside nodes that come from other MathML nodes (those with IDs). Resolves issue #1447. See that issue for examples that cause the problem.

[zorkow]

Much easier to understand. No idea what the original was doing... But what if there are multiple children without id?
Another problem I can see is that you now accumulate stack space. It's not tail recursive and in JS < ES6 it would not help anyway. Also would a query selector not be a better alternative (assuming there is no IE8 problem)?

One way to make it a bit more lightweight is to bind the name test outside the recursive context.

[dpvc]

All good observations, especially for a general search command.

This is not a general search command, however. It is used (in several places) for one specific purpose: when an embellished operator needs to have its operator to be stretched, the embellishments may need to be adjusted. For example, an munderover with a base that is stretched may need to have its over accent stretched as well. In these cases, the parent element (munderover in this example) needs to locate the elements that it has used for the base and over nodes. This routine allows the parent to locate those elements. These elements have class names that are things like mjx-base and mjx-over, and those elements are containers in which the actual child MathML nodes are rendered. Usually, they are the direct children of the element used for the munderover, but in some cases, there is an intervening element.

All the nodes that correspond to the original MathML elements are given ids, but the mjx-base and other such sub-parts of the munderover do not. So the test looks through the direct children of the given element for one with the correct type, but if the child has no id (there will be only one in that case), we look for its children instead.

In practice, the routine will only recurse once (at most), so I'm not too worked about tail recursion or the extra RegExp call here. But I suppose it could be done more directly as

getNode: function (node,type) {
  while (node && node.childNodes.length === 1 && node.firstChild.id == null) node = node.firstChild;
  if (node) {
    var name = RegExp("\\b"+type+"\\b");
    for (var i = 0, m = node.childNodes.length; i < m; i++) {
      var child = node.childNodes[i];
      if (name.test(child.className)) return child;
    }
  }
  return null;
}

which avoids the recursion all together.

[dpvc]

PS, I originally did use a query selector, but the problem is we want to get only the nodes that correspond to the direct children of the given node. In the case of something like msubsup (whose code is shared with msub and msup), when there is no mjx-sup element, we want to return null. But a query selection might find an mjx-sup in an element further down (in the subscript or the base). We want to avoid that. Note that this version of the routine avoids walking into any child node that has an id. Every element that comes from a MathML element will have an id, so we never walk into other MathML nodes. That means we only check the structural elements that are part of the output for the parent node, not the renderings of any of the children.

Since the children could be arbitrarily complicated, query selector might have to look through lots of output. Here, we are going to look through three or four elements at most.

[zorkow]

Thanks for the explanation. Sounds reasonable for a special purpose algorithm.
Can we put some of this into a comment for the function for future reference?

[dpvc]

OK, I've added comments, and replaced the routine by the non-recursive version.

[zorkow]

lgtm.