Handle extra semantic nodes in all explorers and in collapsing expressions - #1543
Open
dpvc wants to merge 13 commits into
Open
Handle extra semantic nodes in all explorers and in collapsing expressions#1543dpvc wants to merge 13 commits into
dpvc wants to merge 13 commits into
Conversation
… get the split nodes
…flame highlight everything
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1543 +/- ##
===========================================
- Coverage 86.93% 86.89% -0.04%
===========================================
Files 388 388
Lines 87571 87632 +61
Branches 3290 4968 +1678
===========================================
+ Hits 76129 76151 +22
- Misses 11442 11481 +39 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR improves the explorers to all use the semantic structure information to include extra nodes in the same way that they are in the speech explorer (so that they appear to be part of the DOM structure even when they aren't). It also allows the collapsed math to include the extra nodes.
To do this, the analysis of the
data-semantic-structureattribute is moved to a utility file, and made available through thesemantic-enrichcomponent, since it is the one that is common to the speech, explorer, and complexity components (and since it is about the semantic tree, so that makes sense).Much of the handling of the extra nodes is moved into common classes in
Explorer.ts,Region.ts,MouseExplorers.ts, etc., to reduce redundancy.Because the extra nodes are not in the same DOM element, the collapsing is harder to produce when there are extra nodes, as y9u can't insert an
mactionnode that encloses all of the needed nodes. Instead, we insert separatemactionnodes around the original and the extra nodes, and link them together as a "collapse group". When one is clicked, the othermactionnodes in the same group are also triggered. This is special handling by MathJax'smactionevent listener, so will not be handled properly if native MathML rendering is used. Of course, MathML-Core doesn't includemactionnodes, so using native MathML rendering for expressions with\togglewould be problematic anyway.Details
There are a significant number of changes, here, and working alphabetically like I usually do would be more difficult, so I am going to work out of order in order to introduce changes that are needed for later explanations. I hope that makes things easier to understand.
Starting with
a11y/speech/StructureUtil.ts, this is a new utility file that handles the semantic-structure computations. This replaces the corresponding functions that were ina11y/explorer/KeyExplorer.ts, which have been removed. Because these may need to be performed before the output is produced (e.g., for inserting themactionsuse for collapsing sub-expressions), this all has to work on the internal MathML tree, not the DOM tree, so functions that relied onquerySelectorAll()needed to be rewritten.These are now grouped into a
StructureUtilobject with static methods, since that is the paradigm that you recommended in place of straight objects. Thetokenize()andparse()methods are unchanged from the originals. ThebuildMap()function is replaced here bysemanticNodes(), which produces a slightly different mapping. It maps an id to a list that includes the original id and the ids of any extra nodes (not all the children ids of the original node, which turn out to never be needed), and only the nodes with extra nodes are mapped, in order to reduce the data being stored (e.g., when collapsing is enabled, that would mean there was a map for every math expression on the page that would include ids and lists for every element with a semantic id -- a lot of data; now the maps only include entries if there are extra nodes involved, which most expressions won't have).The original
buildMap()produced lists of all the semantic child nodes, and later (in the SpeechExplorer'sgetSplitNodes()function), the DOM children were obtained usingquerySelectorAll()and removed from the semantic children to get the extra nodes. We don't have aquerySelectorAll()for the internal MathML tree, and because the child nodes are removed, we don't really need them, we just need the extra nodes.In order to compute this mapping, we need the semantic structure. It was obtained before using a query selector, but we don't have access to that here, so we use
walkTree()to find the node with that attribute. Since that is likely high up int he expression tree, it would be inefficient to continue walking the tree once we find it, so we stop the walking once it is found by returningtrue. This is a new feature ofwalkTree()that is added incore/Tree/Node.js, with similar changes for the corresponding walkers incore/Tree/Wrapper.tsandcore/MmlTree/MmoNode.ts.To produce the semantic mapping, we first produce a temporary mapping of semantic ids to the semantic id of the closest parent with an id. This map can be computed by a single walking of the tree, which is done in the
mapParents()function. The actualsemanticNodemap is then computed by walking the semantic structure tree bottom up, obtaining the extra nodes (ones not currently enclosed in a parent node) from the semantic child nodes, and removing any whose closest parent is the current node, leaving only the extra nodes that aren't in children of the current node and returning those. We move up the tree producing the map of those extra nodes for a given node.The
a11y/semantic-enrich.tsfile now has asemanticNodesproperty to hold this tree, and aparseSemanticNodes()method to compute it if it isn't already available. This gets called by the explorers and the collapse component when the extra nodes are needed.The
getSplitNodes()function that was ina11y/explorer/KeyExplorer.tshas been moved to theExplorerMathItemina11y/explorer.ts, so that it has access to thesemanticNodesstored in the math item, and can be accessed by any of the explorers. This is where we look up the actual DOM nodes. In the past, these were cached because we would have had to get the subtrees, the child nodes, and do a set difference to get them. But now that we have the simplersemanticNodesmap, the cache has been removed (to reduce the memory usage) and the DOM nodes are looked up each time. That should be relatively fast, and I think is a reasonable trade off for better memory usage.The
explorer.tsfile used to serialize the MathML and pass that to the explorer pool, but that value was never actually used, so has been removed, along with the imports and parameters that supported it.The
[has-speech="true"]CSS selector is removed from the CSS for the container, since that is needed for proper positioning of some enclosing rectangles for the magnification explorers when speech and Braille are disabled.In
a11y/explorer/ExplorerPool.js, theinit()function has themmlargument removed (as discussed above), since no explorer actually uses it. I also changed the import of the mouse explorers to be the individual explorers rather than using themeobject to hold them. This seems simple enough since there are only three of them.Because a number of explorers need access to the
ExplorerMathItem(in order to access thesemanticNodes), this now needs to be passed to the explorers, so it is added to theExplorerInittype, and the...restis removed, since it was just for themmlproperty (no longer passed) and the math item, which is now used almost universally (and can be ignored by the ones that done't need it).This leads to changes in setting up the
allExplorersobject, which now use the explorer names directly (withoutme.) and don't use...restparameters, usingiteminstead as the only thing that would have been inrestanyway. Finally, theValueHoverercreation now passes just the attribute name that is needed, rather than the twonodeQuery()andnodeAccess()functions, as they are created automatically from the attribute name inMouseExplorers.ts(see below).The base
AbstractExplorerclass ina11y/explorer/Explorer.tsgets two new service functions that are used by the key and mouse explorers (so needed to be in a common ancestor). These functions are more complicated than the other ones in the abstract class, so might seem out of place, in which case, there could be a new subclass ofAbstractExplorerthat contains these, if that feels better.The first is
inBBox()that tests if a point is inside a rectangle. The second isnodeAtXY()that determines the DOM node where a mouse event occurs. This replaces thegetNode()function fromMouseExplorers.ts, and is a generalization of (and replaces) thefindClicked()function fromKeyExplorers.ts. This traverses the DOM tree from the container node of the expression moving through the children that contain the (x,y) until you get to a node with no more children, keeping track of the last node that matches thenodeQuery(), which will be the node that we are looking for. Some nodes are skipped, like text or comment nodes, or the enclosing nodes added by the highlighter. The nodes for labels and SVG tables need special handling (as themjx-labelsnodes are zero-height, so we would not move into its children, and the SVG tables use nestedsvgelements that overlap the entire expression, so you might traverse the wrong one). Others may want to be skipped (like the info icon and the speech nodes in the SpeechExplorer, or background colorrectnodes).One important reason for this approach rather than the one from
getNode()is that, for SVG nodes, this will find the node by its bounding box, not just the "ink" of the glyph, making the mouse explorers work more consistently between the CHTML and SVG output. It does mean, however, that\llap{}or other zero-width items will not be found.The
a11y/explorer/Region.tsfile has some important changes in theHoverRegion. First, there is a newsplitNodesproperty that is used to temporarily hold thesplitNodes, which are needed when a node is cloned for the magnification region so that all the needed nodes are copied (without this PR, only the first node is magnified). Because the cloning is done in theUpdate()function,splitNodesshould lie set before callingUpdate(). (It would have been nicer to pass thesplitNodestoUpdate(), but adding a parameter to that caused some typing problems, so this seemed the next best solution).There are additions to the CSS: the
display:flexare to get the height of the magnification region correct in situations in SVG output. Thetext-align: centeris for CHTML output when an item is split (e.g.,{x + y\\z}) and both lines are displayed. This is not always correct, as there are situations where the lines should be left or right aligned instead, but it would take a lot more work to find and include the containers that are needed to get that correct. That could be done in a future PR if it turns out to be necessary.The change to
position()is so that the size and position of an enclose is used when there are extra nodes.The
cloneNode()function now uses thesplitNodes()to produce the enclosure (that was the reason for thesplitNodesproperty here).The
chtmlClone()function now handles multiple lines by checking if the id of the parts has already been used (i.e., a MathML node has been split and is in two separate lines). If so, it inserts abrto form a second line. Not perfect, but better than nothing. Again, this could be improved in the future.The
svgClone()function is also modified to handle multiple lines properly. This requires better handling of the x and y coordinates, and also keeping track of the left, right, top, and bottom of the collection of elements used (in order to get theviewBoxfor the resultingsvgelement correct).The various explorers are the next thing to consider.
For the speech explorer in
a11y/explorer/KeyExplorer.ts, we no longer need thesubtrees, as thesemanticNodesare now available on the semantic-enrichment math item. ThefindClicked()function is replaced by thenodeAtXY()function from theAbstractExplorerclass. We pass the query function that it needs, as well as the list of nodes to skip, and the info icon element. The SRE attributes are replaced byATTRvalues, so there is no issue with getting them wrong, or if they change (that should probably be done with thedata-semanticattributes here and elsewhere as well).The
Click()handler now checks for the key magnifier being in play when the speech explorer is not, and responds to the click directly in that case (as the focus will not change as it would for speech or Braille is active, and the display is updated in theFocusIn()handler). That means that when the keyboard magnification is being used, you can click to change the magnified subexpression.The
getSplitNodes()function is now on the associated math item, so we need to usethis.item.getSplitNodes().The
cachePartsare no longer needed,getSplitNodes()has been moved, andsubtree()is no longer needed, as the computation of the extra semantic nodes doesn't require that any more. ThefindClicked()function has been replaced by thenodeAtXY()function from the abstract explorer. So all these have been removed.The
mmlparameter is no longer being passed to the constructor.The computation of subtrees in
Start()is no longer needed, and has been replaced by theparseSemanticNodes()later in that function. We need to set thesplitNodesfor the magnification region before calling itsUpdate()orShow()functions, since thecloneNode()function used to display the magnification requires that map.The
getSubtrees()and all the auxiliary functions have either been moved toStructureUtil.tsor are no longer needed, so are removed fromkeyExplorer.ts.In
a11y/MouseExplorer.ts, several new properties are added:currentis the DOM node that currently contains the mouse position (so we can tell when that changes),listeneris amousemoveevent listener that is used to track when the mouse moves to a new node,listeningto tell when themousemovelistener is in place (it is added and removed when needed), andtopBoxandnodeBoxto hold bounding boxes for the DOM node with the semantic structure and for themjx-containerelement. The last two are set in theconstructor()function.Because we want to use the bounding box of the SVG glyphs rather than the ink itself to trigger the magnification and other actions,
mouseoverandmouseoutevents are not sufficient to make this work. So the handler for those events are changed. Formouseover, if we themousemovelistener hasn't been added yet and the mouse is inside the top elements, we do the usualmouseoveraction and then add the mouseover event listener. Formouseout, we check if the mouse has left the top element, and if so, we unhighlight and hide the region, and do the usual mouse out action. In addition, if we have left the expression container, we remove themousemovelistener (this way, we only track mouse moves when the mouse is over an expression, not all the time, which would be very inefficient).The new
MouseMove()function checks the node that is under the mouse, and if it is not the same as the previous one, it unhighlights the old one and highlights the new one. It uses a newdisplay()method that can be overridden by the subclasses in order to show the proper enclosure and update the region. (It may be easier to look at this in the actual file, since ether diff is a bit awkward, here). The oldgetNode()method is removed in favor of the newnodeAtXY()function in the abstract explore.The default
display()method gets the split nodes (in order to enclose everything that is selected), and sets the region's copy of it for hover regions (so thecloneNode()can clone everything needed). Then the highlighter is asked to enclose and highlight the parts, and if the kind is a string the region is updated (we don't need that for HTMLElements because theShow()method callsUpdate()already, and it is not necessary to do that twice, especially since that is where the nodes are cloned and added to the region; we don't want to do that twice). Then the region is shown.In the past, the individual mouse classes were basically just subclasses that didn't override anything, they now each override the constructor to handle their unique parameters, and the FlameHoverer overrides the
display()method in order to properly handle collapse groups (described below). For theValueHoverer, the attribute string is used to create the query and accessor functions. For theContentHoverer, the query and accessors are always the same. For theFlameHoverer, we look for thedata-collapsibleattribute rather than usingisMactionNode(), so as not to get all mactions (we only need the ones from the collapsing component), and becauseisMactionNode()is modified, as describe below.The
display()method of theFlameHovereris overridden to handle the action groups introduced by the collapsing of nodes with extra nodes that aren't in its DOM tree (described in the discussion of collapsing below). We don't need to use the split nodes in this explorer, as the attributes on themactionnodes themselves can be used to obtain the needed DOM nodes. If we have hovered over one of the extra node'smactionnodes, we look up the original using the extra node'sdata-collapse-idvalue, which holds the original node's id. If the (main) maction node has thedata-collapse-groupattribute, that means there are extra nodes that also need to be highlighted, so we get the action group, otherwise we just use themactionnode itself. We enclose and highlight the nodes, and show the region.For the
TreeExplorer, the only change is that themmlproperty is no longer being passed to it.The remaining changes are for the handling collapsing of nodes that have extra structure that is outside the node's DOM tree.
In
a11y/complexity.ts, we compute the semantic nodes and pass them to the complexity visitor (so that when anmactionis added, we can handle any extra nodes for it). Ina11y/complexity/visitor.ts, the semantic map is passed on to thecollapse.makeCollapse()method for the same reason.In
a11y/complexity/collapse.ts, some comment spacing is adjusted. Then themakeCollapse() function gets the new semantic nodes map and passes it on to tomakeActions()(along with the top-levelnode(the internalmathMathML element), which is needed in order to locate the extra nodes). ThemakeActions()function looks up any extra node ids and passes these on tomakeAction(). Finally,makeAction()passes the root node and extra ids on to a newmakeActionGruop()function. (In addition, thevariantvariable was renameddef, since it includes thedata-mjx-collapsed` property, not just the variant.)The
makeActionGroup()function checks if there are extra nodes, and if not returns immediately. Otherwise, it marks the mainmactionnode as being the head of a group by setting itsdata-collapse-groupattribute. (This is used by the explorers and highlighters to tell when extra nodes must be included.) Then it walks the MathML tree looking for elements withdata-semantic-idthat are included in the extra list, and records them in a list. then it creates anmactionnode around each of the extra nodes (using an emptymtextas the "closed" element), and ties them to the mainmactionusing thedata-collapse-idattribute (pointing to the maction nodes' id). This sets things up so that the highlighters and explorers can process multiplemactionelements as a group.We've already seen some of these changes in the
FlameHoverer. The main changes are in the highlighters.In
a11y/explorer/Highlighter.ts, a newgetMactionGroup()function is added to obtain all themactionelements within a given group. This is implemented via aquerySelectorAll()that targets the mainmactionplus all the ones that point to it via theirdata-collapse-id. This function is used in thehighlightAll()method to get the needed parts to highlight.The
getMactionNodes()methods in the CHTML and SVG highlighters are modified to return only the mainmactionnodes (not the extra ones), and onlymactionnodes added for collapsing (not allmactionnodes).We also make sure that any extra enclosure nodes are marked properly as being added so that the explorers can identify and skip them.
Finally, the output wrappers for
mactionnodes are modified to handle the action groups. If anmactionis clicked that is part of a group, we walk to tree looking for the other entries in the group and change their selections as well. We also usehandleRetriesFor()around the re-render, in case the selected option requires a font range to be loaded (e.g.,\toggle{C}{\mathcal{C})\endtoggle). There is redundancy in the CHTML and SVG implementations that could be refactored into a common function, but I will do that in a separate PR later.