Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 8, 2016
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7116cde
Improve getNode() so that it doesn't find elements inside nodes that …
dpvc Apr 29, 2016
e1d430d
Make getNode() non-recursive and add comments for what it does. Issu…
dpvc May 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 11 additions & 25 deletions unpacked/jax/output/CommonHTML/jax.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,34 +336,20 @@
ucMatch: HTML.ucMatch,
setScript: HTML.setScript,

getNodesByClass: (document.getElementsByClassName ?
function (node,type) {return node.getElementsByClassName(type)} :
function (node,type) {
var NODES = [];
var nodes = node.getElementsByTagName("span");
var name = RegExp("\\b"+type+"\\b");
for (var i = 0, m = nodes.length; i < m; i++) {
if (name.test(nodes[i].className)) NODES.push = nodes[i];
}
return NODES;
}
),
//
// Look through the children of a node for one with the given type
// but don't step into child nodes that are from MathML elements
// themselves (they will have IDs).
//
getNode: function (node,type) {
var nodes = this.getNodesByClass(node,type);
if (nodes.length === 1) return nodes[0];
var closest = nodes[0], N = this.getNodeDepth(node,closest);
for (var i = 1, m = nodes.length; i < m; i++) {
var n = this.getNodeDepth(node,nodes[i]);
if (n < N) {closest = nodes[i]; N = n}
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;
if (child.id == null) return this.getNode(child,type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@dpvc dpvc May 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@dpvc dpvc May 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
return closest;
},
getNodeDepth: function (parent,node) {
var n = 0;
while (node && node !== parent) {node = node.parentNode; n++}
return n;
},



/********************************************/

Expand Down