Merge pull request #1002 from mathjax/issue3098b

Update check for spacelike mml nodes to be more sensible.  (mathjax/MathJax#3098)

ts/a11y/semantic-enrich.ts

@@ -237,7 +237,7 @@ export function EnrichedMathItemMixin<N, T, D, B extends Constructor<AbstractMat
       const speech = attributes.getExplicit('data-semantic-speech') as string;
       // TODO (explorer) For tree role move all speech etc. to container
       // element.
-      if (!attributes.getExplicit('data-semantic-parent') && speech) {
+      if (!attributes.hasExplicit('data-semantic-parent') && speech) {
         return speech;
       }
       for (let child of node.childNodes) {

ts/core/MmlTree/Attributes.ts

@@ -83,6 +83,13 @@ export class Attributes {
     Object.assign(this.attributes, list);
   }
 
+  /**
+   * @param {string} name   The name of the attribute to remove
+   */
+  public unset(name: string) {
+    delete this.attributes[name];
+  }
+
   /**
    * @param {string} name  The name of the attribute whose value is to be returned
    * @return {Property}    The value of the named attribute (including inheritance and defaults)
@@ -101,10 +108,28 @@ export class Attributes {
    *                       node (not inherited or defaulted), null otherwise
    */
   public getExplicit(name: string): Property {
-    if (!this.attributes.hasOwnProperty(name)) {
-      return undefined;
+    return (this.hasExplicit(name) ? this.attributes[name] : undefined);
+  }
+
+  /**
+   * @param {string} name  The value of the attribute whose presence is to be checked
+   * @return {boolean}     True if the attribute is explicitly given on this node
+   */
+  public hasExplicit(name: string): boolean {
+    return this.attributes.hasOwnProperty(name);
+  }
+
+  /**
+   * @param {string[]} names   The attribute names to look for.
+   * @return {boolean}         True if one of the names is an explicit attribute, false otherwise
+   */
+  public hasOneOf(names: string[]): boolean {
+    for (const name of names) {
+      if (this.hasExplicit(name)) {
+        return true;
+      }
     }
-    return this.attributes[name];
+    return false;
   }
 
   /**

ts/core/MmlTree/MmlNodes/maction.ts

@@ -98,9 +98,8 @@ export class MmlMaction extends AbstractMmlNode {
   protected verifyAttributes(options: PropertyList) {
     super.verifyAttributes(options);
     if (this.attributes.get('actiontype') !== 'toggle' &&
-        this.attributes.getExplicit('selection') !== undefined) {
-      const attributes = this.attributes.getAllAttributes();
-      delete attributes.selection;
+        this.attributes.hasExplicit('selection')) {
+      this.attributes.unset('selection');
     }
   }
 

ts/core/MmlTree/MmlNodes/mspace.ts

@@ -31,6 +31,18 @@ import {MmlNode, AbstractMmlTokenNode, TEXCLASS} from '../MmlNode.js';
 
 export class MmlMspace extends AbstractMmlTokenNode {
 
+  /**
+   * Attributes that make an mpsace not spacelike
+   */
+  public static NONSPACELIKE = [
+    /* 'width' */  // spec says not to allow breaks here, but we allow it
+    'height',
+    'depth',
+    'style',
+    'mathbackground',
+    'background'
+  ];
+
   /**
    * @override
    */
@@ -77,10 +89,12 @@ export class MmlMspace extends AbstractMmlTokenNode {
   }
 
   /**
+   * Only make mspace be space-like if it doesn't have certain attributes
+   *
    * @override
    */
   public get isSpacelike() {
-    return true;
+    return !this.attributes.hasExplicit('linebreak') && this.canBreak;
   }
 
   /**
@@ -90,21 +104,15 @@ export class MmlMspace extends AbstractMmlTokenNode {
    */
   public get hasNewline() {
     const linebreak = this.attributes.get('linebreak');
-    return (this.canBreak && (linebreak === 'newline' || linebreak === 'indentingnewline'));
+    return this.canBreak && (linebreak === 'newline' || linebreak === 'indentingnewline');
   }
 
   /**
    * @return {boolean}   True if mspace is allowed to break, i.e.,
    *                     no height/depth, no styles, and no background color.
    */
   public get canBreak(): boolean {
-    const attributes = this.attributes;
-    return (/*attributes.getExplicit('width') === undefined &&*/  // we break spaces with width ...
-            attributes.getExplicit('height') === undefined &&
-            attributes.getExplicit('depth') === undefined &&
-            attributes.getExplicit('style') === undefined &&      //   ... but not ones with styles
-            attributes.getExplicit('mathbackground') === undefined &&
-            attributes.getExplicit('background') === undefined);
+    return !this.attributes.hasOneOf(MmlMspace.NONSPACELIKE);
   }
 
 }

ts/core/MmlTree/MmlNodes/mtable.ts

@@ -105,8 +105,8 @@ export class MmlMtable extends AbstractMmlNode {
       if (attributes[name]) {
         this.attributes.setInherited(name, attributes[name][1]);
       }
-      if (this.attributes.getExplicit(name) !== undefined) {
-        delete (this.attributes.getAllAttributes())[name];
+      if (this.attributes.hasExplicit(name)) {
+        this.attributes.unset(name);
       }
     }
     super.setInheritedAttributes(attributes, display, level, prime);

ts/core/MmlTree/MmlNodes/mtext.ts

@@ -31,6 +31,15 @@ import {AbstractMmlTokenNode, TEXCLASS} from '../MmlNode.js';
 
 export class MmlMtext extends AbstractMmlTokenNode {
 
+  /**
+   * Attributes that make an mpsace not spacelike
+   */
+  public static NONSPACELIKE = [
+    'style',
+    'mathbackground',
+    'background'
+  ];
+
   /**
    * @override
    */
@@ -51,11 +60,15 @@ export class MmlMtext extends AbstractMmlTokenNode {
   }
 
   /**
-   * <mtext> is always space-like according to the spec
+   * According to the spec, <mtext> is always space-like,
+   * but we make it so only if it contains only spaces and
+   * doesn't have certain attributes.
+   *
    * @override
    */
   public get isSpacelike() {
-    return true;
+    return !!this.getText().match(/^\s*$/) &&
+           !this.attributes.hasOneOf(MmlMtext.NONSPACELIKE);
   }
 
 }

ts/core/MmlTree/MmlNodes/munderover.ts

@@ -144,7 +144,7 @@ export class MmlMunderover extends AbstractMmlBaseNode {
   protected setInheritedAccent(n: number, accent: string, display: boolean, level: number,
                                prime: boolean, force: boolean) {
     let node = this.childNodes[n];
-    if (this.attributes.getExplicit(accent) == null && node.isEmbellished) {
+    if (!this.attributes.hasExplicit(accent) && node.isEmbellished) {
       let value = node.coreMO().attributes.get('accent');
       this.attributes.setInherited(accent, value);
       if (value !== this.attributes.getDefault(accent)) {

ts/input/tex/FilterUtil.ts

@@ -62,20 +62,20 @@ namespace FilterUtil {
    * Visitor that removes superfluous attributes from nodes. I.e., if a node has
    * an attribute, which is also an inherited attribute it will be removed. This
    * is necessary as attributes are set bottom up in the parser.
-   * @param {ParseOptions} data The parse options.
+   * @param {ParseOptions} data   The parse options.
    */
   export let cleanAttributes = function(arg: {data: ParseOptions}) {
     let node = arg.data.root;
     node.walkTree((mml: MmlNode, _d: any) => {
-      let attribs = mml.attributes as any;
+      let attribs = mml.attributes;
       if (!attribs) {
         return;
       }
-      const keep = new Set((attribs.get('mjx-keep-attrs') || '').split(/ /));
-      delete (attribs.getAllAttributes())['mjx-keep-attrs'];
+      const keep = new Set((attribs.get('mjx-keep-attrs') as string || '').split(/ /));
+      attribs.unset('mjx-keep-attrs');
       for (const key of attribs.getExplicitNames()) {
-        if (!keep.has(key) && attribs.attributes[key] === mml.attributes.getInherited(key)) {
-          delete attribs.attributes[key];
+        if (!keep.has(key) && attribs.get(key) === mml.attributes.getInherited(key)) {
+          attribs.unset(key);
         }
       }
     }, {});
@@ -121,11 +121,11 @@ namespace FilterUtil {
           m2.setProperty('relationsCombined', true);
         } else {
           // @test Preset Rspace Lspace
-          if (mo.attributes.getExplicit('rspace') == null) {
+          if (!mo.attributes.hasExplicit('rspace')) {
             // @test Mulitrel Mathvariant 3, Mulitrel Mathvariant 4
             NodeUtil.setAttribute(mo, 'rspace', '0pt');
           }
-          if (m2.attributes.getExplicit('lspace') == null) {
+          if (!m2.attributes.hasExplicit('lspace')) {
             // @test Mulitrel Mathvariant 3, Mulitrel Mathvariant 4
             NodeUtil.setAttribute(m2, 'lspace', '0pt');
           }
@@ -173,7 +173,7 @@ namespace FilterUtil {
       return exp.filter(x => {
         return x !== space &&
           (x !== 'stretchy' ||
-           attr.getExplicit('stretchy'));
+           attr.hasExplicit('stretchy'));
       });
     };
     let attr1 = node1.attributes;
@@ -252,7 +252,7 @@ namespace FilterUtil {
       }
       const base = mml.childNodes[(mml as any).base];
       const mo = base.coreMO();
-      if (base.getProperty('movablelimits') && !mo.attributes.getExplicit('movablelimits')) {
+      if (base.getProperty('movablelimits') && !mo.attributes.hasExplicit('movablelimits')) {
         let node = options.nodeFactory.create('node', subsup, mml.childNodes);
         NodeUtil.copyAttributes(mml, node);
         if (mml.parent) {

ts/input/tex/NodeUtil.ts

@@ -164,7 +164,7 @@ namespace NodeUtil {
    * @param {string} attr An attribute name.
    */
   export function removeAttribute(node: MmlNode, attr: string): void  {
-    delete (node.attributes.getAllAttributes())[attr];
+    node.attributes.unset(attr);
   }
 
 

ts/input/tex/textmacros/TextParser.ts

@@ -160,7 +160,7 @@ export class TextParser extends TexParser {
       mml = this.create('node', 'TeXAtom', [mml]);  // make sure the math is an ORD
     }
     for (const name of ['mathsize', 'mathcolor']) {
-      if (env[name] && !mml.attributes.getExplicit(name)) {
+      if (env[name] && !mml.attributes.hasExplicit(name)) {
         if (!mml.isToken && !mml.isKind('mstyle')) {
           mml = this.create('node', 'mstyle', [mml]);
         }
@@ -183,7 +183,7 @@ export class TextParser extends TexParser {
     const env = this.stack.env;
     if (!mml.isToken) return;
     for (const name of ['mathsize', 'mathcolor', 'mathvariant']) {
-      if (env[name] && !mml.attributes.getExplicit(name)) {
+      if (env[name] && !mml.attributes.hasExplicit(name)) {
         NodeUtil.setAttribute(mml, name, env[name]);
       }
     }

ts/output/common.ts

@@ -432,7 +432,7 @@ export abstract class CommonOutputJax<
       } else {
         postbreak = false;
         if ((child.isKind('mstyle') && !child.attributes.get('style') &&
-             !child.attributes.getExplicit('mathbackground')) || child.isKind('semantics')) {
+             !child.attributes.hasExplicit('mathbackground')) || child.isKind('semantics')) {
           this.markInlineBreaks(child.childNodes[0]);
           if (child.getProperty('process-breaks')) {
             child.setProperty('inline-breaks', true);

ts/output/common/Wrapper.ts

@@ -818,7 +818,7 @@ export class CommonWrapper<
     if (!this.node.isToken) return;
     const attributes = this.node.attributes;
     let variant = attributes.get('mathvariant') as string;
-    if (attributes.getExplicit('mathvariant')) {
+    if (attributes.hasExplicit('mathvariant')) {
       if (!this.font.getVariant(variant)) {
         console.warn(`Invalid variant: ${variant}`);
         variant = 'normal';
@@ -890,7 +890,7 @@ export class CommonWrapper<
     //
     // If there is a fontsize and no mathsize attribute, is that
     //
-    if (fontsize && !attributes.getExplicit('mathsize')) {
+    if (fontsize && !attributes.hasExplicit('mathsize')) {
       mathsize = fontsize;
     }
     //

ts/output/common/Wrappers/mo.ts

@@ -313,7 +313,7 @@ export function CommonMoMixin<
       //
       //  Check for a null delimiter and add the null-delimiter space
       //
-      if (bbox.w === 0 && this.node.attributes.getExplicit('fence') &&
+      if (bbox.w === 0 && this.node.attributes.hasExplicit('fence') &&
           (this.node as MmlMo).getText() === '' &&
           (this.node.texClass === TEXCLASS.OPEN || this.node.texClass === TEXCLASS.CLOSE) &&
           !this.jax.options.mathmlSpacing) {
@@ -627,7 +627,7 @@ export function CommonMoMixin<
         this.variant = (this.node.attributes.get('displaystyle') ? '-largeop' : '-smallop');
         return;
       }
-      if (!this.node.attributes.getExplicit('mathvariant') &&
+      if (!this.node.attributes.hasExplicit('mathvariant') &&
           this.node.getProperty('pseudoscript') === false) {
         this.variant = '-tex-variant';
         return;