Repository: Khan/KaTeX
Pull request #923 (2017-10-09 12:28:51)

Implemented `\href' command
Implemented `\href` command in LaTeX hyperref package.
It seems that LaTeX simply ignores special commands for hyper-linking and processes spacing as usual.
Since I don't make out how to simulate this spacing behaviour in KaTeX, I adopt the following strategy:

1. If it has the only element, then use its math-type;
2. if it has more than two elements, and the types of both ends of elements coincides, then use it as a class for the anchor tag;
3. otherwise, we inject empty span-tag aside both ends of generated anchor tag, both with the same classes as the first and the last ones respectively.

khanbot: CLA signature looks good :+1:
edemaine: Thanks @konn for working on this.  `\href` is definitely something we should support.

I don't understand what you mean by your three rules though.  `\href` should not be setting class, should it?  Isn't that the purpose of `\class` (see #90)?

Isn't the second argument of `\href` treated as if it's in `\text`?  You should be able to use code similar to `\text`, `\raisebox`, etc. which switch to text mode.
konn: > I don't understand what you mean by your three rules though. \href should not be setting class, should it? Isn't that the purpose of \class (see #90)?

Well, if you don't specify any math-type via class of an anchor tag, you get the different spacing behaviour than latex's default.

Consider the following input:

```latex
\documentclass[a4]{article}
\usepackage{hyperref}
\usepackage{amsmath,amssymb}
\begin{document}
$0 \href{http://en.wikipedia.org/wiki/Equals\_sign}{=} 1$.

$0 = 1$.
\end{document}
```

If you typeset the above by xelatex, lualatex (or latex + dvipdfmx with `dvipdfmx` argument for `hyperref` package), you get the following result, with the upper equals-sign linked to Wikipedia:

![default](https://user-images.githubusercontent.com/21860/31377140-0b331498-ade2-11e7-86cc-deba8aeee8c9.jpg)

Here, we see that both `\href`'ed version of `=` and plain `=` result in the same spacing.
On the other hand, if we drop the math-type handling in KaTeX, we get the following result:

<img width="694" alt="2017-10-10 17 43 43" src="https://user-images.githubusercontent.com/21860/31377334-a251928c-ade2-11e7-9cc2-c62ea3f471a8.png">

So, to get the same spacing adjustment, we have to set a class of anchor tag to `mord`, `mrel` or so.

> Isn't the second argument of \href treated as if it's in \text? You should be able to use code similar to \text, \raisebox, etc. which switch to text mode.

As above, LaTeX's (more precisely, `hyperref`'s) `\href` commands accept a math as its second argument, not only text. So we can't treat them as if they're in `\text`.
edemaine: Ah, now I understand where you're getting the classes.  (I was worried that they were being specified by the user, but rather they're grabbed from the rendered arguments.)  Now this seems like the right approach to me.  Cool!
edemaine: @konn Just checking: Is this ready for review again?
konn: @edemaine Yes. I think it’s ready.
kevinbarabash: I'll have another look this weekend.
kevinbarabash: @konn sorry about the delay.  Please let me know if you have any questions/concerns about any of the feedback.
konn: @kevinbarabash No problem, thank you for your review!

> This could also use some unit tests to verify that `\hrefs` parse and don't parse in the expected way, see katex-spec.js. Also, it'd be nice to have a snapshot test for the MathML that we're outputting, see mathml-spec.js (running npm test -- -u will update the snapshot file).

OK, I will try to write some unit-tests.
kevinbarabash: > OK, I will try to write some unit-tests.

@konn let me know if you need help with this.
konn: @kevinbarabash Is there any time to look into this?
kevinbarabash: @konn I've reviewed `parseStringGroupWithBalancedBraces`, looks good.  I will merge this after the tests pass.  Sorry for taking so long with this.

Patch:
diff --git a/src/Parser.js b/src/Parser.js
index 61e00438..88f32cac 100644
--- a/src/Parser.js
+++ b/src/Parser.js
@@ -759,6 +759,9 @@ export default class Parser {
         if (innerMode === "size") {
             return this.parseSizeGroup(optional);
         }
+        if (innerMode === "url") {
+            return this.parseUrlGroup(optional);
+        }
 
         // By the time we get here, innerMode is one of "text" or "math".
         // We switch the mode of the parser, recurse, then restore the old mode.
@@ -807,6 +810,52 @@ export default class Parser {
         return firstToken.range(lastToken, str);
     }
 
+    /**
+     * Parses a group, essentially returning the string formed by the
+     * brace-enclosed tokens plus some position information, possibly
+     * with nested braces.
+     *
+     * @param {string} modeName  Used to describe the mode in error messages
+     * @param {boolean=} optional  Whether the group is optional or required
+     * @return {?Token}
+     */
+    parseStringGroupWithBalancedBraces(modeName, optional) {
+        if (optional && this.nextToken.text !== "[") {
+            return null;
+        }
+        const outerMode = this.mode;
+        this.mode = "text";
+        this.expect(optional ? "[" : "{");
+        let str = "";
+        let nest = 0;
+        const firstToken = this.nextToken;
+        let lastToken = firstToken;
+        while (nest > 0 || this.nextToken.text !== (optional ? "]" : "}")) {
+            if (this.nextToken.text === "EOF") {
+                throw new ParseError(
+                    "Unexpected end of input in " + modeName,
+                    firstToken.range(this.nextToken, str));
+            }
+            lastToken = this.nextToken;
+            str += lastToken.text;
+            if (lastToken.text === "{") {
+                nest += 1;
+            } else if (lastToken.text === "}") {
+                if (nest <= 0) {
+                    throw new ParseError(
+                        "Unbalanced brace of input in " + modeName,
+                        firstToken.range(this.nextToken, str));
+                } else {
+                    nest -= 1;
+                }
+            }
+            this.consume();
+        }
+        this.mode = outerMode;
+        this.expect(optional ? "]" : "}");
+        return firstToken.range(lastToken, str);
+    }
+
     /**
      * Parses a regex-delimited group: the largest sequence of tokens
      * whose concatenated strings match `regex`. Returns the string
@@ -852,6 +901,23 @@ export default class Parser {
         return newArgument(new ParseNode("color", match[0], this.mode), res);
     }
 
+    /**
+     * Parses a url string.
+     */
+    parseUrlGroup(optional) {
+        const res = this.parseStringGroupWithBalancedBraces("url", optional);
+        if (!res) {
+            return null;
+        }
+        const raw = res.text;
+        // hyperref package allows backslashes alone in href, but doesn't generate
+        // valid links in such cases; we interpret this as "undefiend" behaviour,
+        // and keep them as-is. Some browser will replace backslashes with
+        // forward slashes.
+        const url = raw.replace(/\\([#$%&~_^{}])/g, '$1');
+        return newArgument(new ParseNode("url", url, this.mode), res);
+    }
+
     /**
      * Parses a size specification, consisting of magnitude and unit.
      */
diff --git a/src/buildCommon.js b/src/buildCommon.js
index 9a3e6d3f..8c765e86 100644
--- a/src/buildCommon.js
+++ b/src/buildCommon.js
@@ -226,6 +226,18 @@ const makeSpan = function(classes, children, options) {
     return span;
 };
 
+/**
+ * Makes an anchor with the given href, list of classes, list of children,
+ * and options.
+ */
+const makeAnchor = function(href, classes, children, options) {
+    const anchor = new domTree.anchor(href, classes, children, options);
+
+    sizeElementFromChildren(anchor);
+
+    return anchor;
+};
+
 /**
  * Prepends the given children to the given span, updating height, depth, and
  * maxFontSize.
@@ -511,6 +523,7 @@ export default {
     makeSymbol: makeSymbol,
     mathsym: mathsym,
     makeSpan: makeSpan,
+    makeAnchor: makeAnchor,
     makeFragment: makeFragment,
     makeVList: makeVList,
     makeOrd: makeOrd,
diff --git a/src/buildHTML.js b/src/buildHTML.js
index 6f11e605..51f939a2 100644
--- a/src/buildHTML.js
+++ b/src/buildHTML.js
@@ -158,7 +158,7 @@ export const buildExpression = function(expression, options, isRealGroup) {
 };
 
 // Return math atom class (mclass) of a domTree.
-const getTypeOfDomTree = function(node) {
+export const getTypeOfDomTree = function(node) {
     if (node instanceof domTree.documentFragment) {
         if (node.children.length) {
             return getTypeOfDomTree(
diff --git a/src/domTree.js b/src/domTree.js
index 259f26ed..e49fdac6 100644
--- a/src/domTree.js
+++ b/src/domTree.js
@@ -142,6 +142,127 @@ class span {
     }
 }
 
+/**
+ * This node represents an anchor (<a>) element with a hyperlink, a list of classes,
+ * a list of children, and an inline style. It also contains information about its
+ * height, depth, and maxFontSize.
+ */
+class anchor {
+    constructor(href, classes, children, options) {
+        this.href = href || "";
+        this.classes = classes || [];
+        this.children = children || [];
+        this.height = 0;
+        this.depth = 0;
+        this.maxFontSize = 0;
+        this.style = {};
+        this.attributes = {};
+        if (options) {
+            if (options.style.isTight()) {
+                this.classes.push("mtight");
+            }
+            if (options.getColor()) {
+                this.style.color = options.getColor();
+            }
+        }
+    }
+
+    /**
+     * Sets an arbitrary attribute on the anchor. Warning: use this wisely. Not all
+     * browsers support attributes the same, and having too many custom attributes
+     * is probably bad.
+     */
+    setAttribute(attribute, value) {
+        this.attributes[attribute] = value;
+    }
+
+    tryCombine(sibling) {
+        return false;
+    }
+
+    /**
+     * Convert the anchor into an HTML node
+     */
+    toNode() {
+        const a = document.createElement("a");
+
+        // Apply the href
+        a.setAttribute('href', this.href);
+
+        // Apply the class
+        if (this.classes.length) {
+            a.className = createClass(this.classes);
+        }
+
+        // Apply inline styles
+        for (const style in this.style) {
+            if (Object.prototype.hasOwnProperty.call(this.style, style)) {
+                a.style[style] = this.style[style];
+            }
+        }
+
+        // Apply attributes
+        for (const attr in this.attributes) {
+            if (Object.prototype.hasOwnProperty.call(this.attributes, attr)) {
+                a.setAttribute(attr, this.attributes[attr]);
+            }
+        }
+
+        // Append the children, also as HTML nodes
+        for (let i = 0; i < this.children.length; i++) {
+            a.appendChild(this.children[i].toNode());
+        }
+
+        return a;
+    }
+
+    /**
+     * Convert the a into an HTML markup string
+     */
+    toMarkup() {
+        let markup = "<a";
+
+        // Add the href
+        markup += `href="${markup += utils.escape(this.href)}"`;
+        // Add the class
+        if (this.classes.length) {
+            markup += ` class="${utils.escape(createClass(this.classes))}"`;
+        }
+
+        let styles = "";
+
+        // Add the styles, after hyphenation
+        for (const style in this.style) {
+            if (this.style.hasOwnProperty(style)) {
+                styles += utils.hyphenate(style) + ":" + this.style[style] + ";";
+            }
+        }
+
+        if (styles) {
+            markup += " style=\"" + utils.escape(styles) + "\"";
+        }
+
+        // Add the attributes
+        for (const attr in this.attributes) {
+            if (attr !== "href" &&
+                Object.prototype.hasOwnProperty.call(this.attributes, attr)) {
+                markup += ` ${attr}="${utils.escape(this.attributes[attr])}"`;
+            }
+        }
+
+        markup += ">";
+
+        // Add the markup of the children, also as markup
+        for (const child of this.children) {
+            markup += child.toMarkup();
+        }
+
+        markup += "</a>";
+
+        return markup;
+    }
+}
+
 /**
  * This node represents a document fragment, which contains elements, but when
  * placed into the DOM doesn't have any representation itself. Thus, it only
@@ -445,6 +566,7 @@ class lineNode {
 
 export default {
     span: span,
+    anchor: anchor,
     documentFragment: documentFragment,
     symbolNode: symbolNode,
     svgNode: svgNode,
diff --git a/src/functions.js b/src/functions.js
index 1dacaacf..c6adecac 100644
--- a/src/functions.js
+++ b/src/functions.js
@@ -579,5 +579,8 @@ defineFunction(["\\verb"], {
         "\\verb ended by end of line instead of matching delimiter");
 });
 
+// Hyperlinks
+import "./functions/href";
+
 // MathChoice
 import "./functions/mathchoice";
diff --git a/src/functions/href.js b/src/functions/href.js
new file mode 100644
index 00000000..7df756a1
--- /dev/null
+++ b/src/functions/href.js
@@ -0,0 +1,73 @@
+// @flow
+import defineFunction, {ordargument} from "../defineFunction";
+import buildCommon from "../buildCommon";
+import mathMLTree from "../mathMLTree";
+
+import * as html from "../buildHTML";
+import * as mml from "../buildMathML";
+
+defineFunction({
+    type: "href",
+    names: ["\\href"],
+    props: {
+        numArgs: 2,
+        argTypes: ["url", "original"],
+    },
+    handler: (context, args) => {
+        const body = args[1];
+        const href  = args[0].value;
+        return {
+            type: "href",
+            href: href,
+            body: ordargument(body),
+        };
+    },
+    htmlBuilder: (group, options) => {
+        const elements = html.buildExpression(
+            group.value.body,
+            options,
+            false
+        );
+
+        const href = group.value.href;
+
+        /**
+         * Determining class for anchors.
+         * 1. if it has the only element, use its class;
+         * 2. if it has more than two elements, and the classes
+         *    of its first and last elements coincide, then use it;
+         * 3. otherwise, we will inject an empty <span>s at both ends,
+         *    with the same classes of both ends of elements, with the
+         *    first span having the same class as the first element of body,
+         *    and the second one the same as the last.
+         */
+
+        let classes = []; // Default behaviour for Case 3.
+        let first; // mathtype of the first child
+        let last;  // mathtype of the last child
+        // Invariants: both first and last must be non-null if classes is null.
+        if (elements.length === 1) { // Case 1
+            classes = elements[0].classes;
+        } else if (elements.length >= 2) {
+            first = html.getTypeOfDomTree(elements[0]) || 'mord';
+            last = html.getTypeOfDomTree(elements[elements.length - 1]) || 'mord';
+            if (first === last) { // Case 2 : type of both ends coincides
+                classes = [first];
+            } else { // Case 3: both ends have different types.
+                const anc = buildCommon.makeAnchor(href, [], elements, options);
+                return new buildCommon.makeFragment([
+                    new buildCommon.makeSpan([first], [], options),
+                    anc,
+                    new buildCommon.makeSpan([last], [], options),
+                ]);
+            }
+        }
+        return new buildCommon.makeAnchor(href, classes, elements, options);
+    },
+    mathmlBuilder: (group, options) => {
+        const inner = mml.buildExpression(group.value.body, options);
+        const math = new mathMLTree.MathNode("mrow", inner);
+        math.setAttribute("href", group.value.href);
+        return math;
+    },
+});
diff --git a/src/types.js b/src/types.js
index 7d65cfe4..4173deeb 100644
--- a/src/types.js
+++ b/src/types.js
@@ -10,13 +10,15 @@ export type Mode = "math" | "text";
 // LaTeX argument type.
 //   - "size": A size-like thing, such as "1em" or "5ex"
 //   - "color": An html color, like "#abc" or "blue"
+//   - "url": An url string, in which "\" will be ignored
+//   -        if it precedes [#$%&~_^\{}]
 //   - "original": The same type as the environment that the
 //                 function being parsed is in (e.g. used for the
 //                 bodies of functions like \textcolor where the
 //                 first argument is special and the second
 //                 argument is parsed normally)
 //   - Mode: Node group parsed in given mode.
-export type ArgType = "color" | "size" | "original" | Mode;
+export type ArgType = "color" | "size" | "url" | "original" | Mode;
 
 // LaTeX display style.
 export type StyleStr = "text" | "display";


Test patch:
diff --git a/test/__snapshots__/mathml-spec.js.snap b/test/__snapshots__/mathml-spec.js.snap
index 3b378772..723e3642 100644
--- a/test/__snapshots__/mathml-spec.js.snap
+++ b/test/__snapshots__/mathml-spec.js.snap
@@ -220,6 +220,25 @@ exports[`A MathML builder should render mathchoice as if there was nothing 4`] =
 
 `;
 
+exports[`A MathML builder should set href attribute for href appropriately 1`] = `
+
+<math>
+  <semantics>
+    <mrow>
+      <mrow href="http://example.org">
+        <mi>
+          α
+        </mi>
+      </mrow>
+    </mrow>
+    <annotation encoding="application/x-tex">
+      \\href{http://example.org}{\\alpha}
+    </annotation>
+  </semantics>
+</math>
+
+`;
+
 exports[`A MathML builder should use <menclose> for colorbox 1`] = `
 
 <math>
diff --git a/test/katex-spec.js b/test/katex-spec.js
index 09142097..fb33a71b 100644
--- a/test/katex-spec.js
+++ b/test/katex-spec.js
@@ -2420,6 +2420,36 @@ describe("An aligned environment", function() {
     });
 });
 
+describe("An href command", function() {
+    it("should parse its input", function() {
+        expect("\\href{http://example.com/}{example here}").toParse();
+    });
+
+    it("should allow letters [#$%&~_^] without escaping", function() {
+        const url = "http://example.org/~bar/#top?foo=$foo&bar=ba^r_boo%20baz";
+        const hash = getParsed(`\\href{${url}}{\\alpha}`)[0];
+        expect(hash.value.href).toBe(url);
+    });
+
+    it("should allow balanced braces in url", function() {
+        const url = "http://example.org/{too}";
+        const hash = getParsed(`\\href{${url}}{\\alpha}`)[0];
+        expect(hash.value.href).toBe(url);
+    });
+
+    it("should not allow unbalanced brace(s) in url", function() {
+        expect("\\href{http://example.com/{a}{bar}").toNotParse();
+        expect("\\href{http://example.com/}a}{bar}").toNotParse();
+    });
+
+    it("should allow escape for letters [#$%&~_^{}]", function() {
+        const url = "http://example.org/~bar/#top?foo=$}foo{&bar=bar^r_boo%20baz";
+        const input = url.replace(/([#$%&~_^{}])/g, '\\$1');
+        const ae = getParsed(`\\href{${input}}{\\alpha}`)[0];
+        expect(ae.value.href).toBe(url);
+    });
+});
+
 describe("A parser that does not throw on unsupported commands", function() {
     // The parser breaks on unsupported commands unless it is explicitly
     // told not to
diff --git a/test/mathml-spec.js b/test/mathml-spec.js
index 2eabf49a..7b285058 100644
--- a/test/mathml-spec.js
+++ b/test/mathml-spec.js
@@ -59,6 +59,11 @@ describe("A MathML builder", function() {
         expect(getMathML("\\colorbox{red}{b}")).toMatchSnapshot();
     });
 
+    it('should set href attribute for href appropriately', () => {
+        expect(getMathML("\\href{http://example.org}{\\alpha}")).toMatchSnapshot();
+        expect(getMathML("p \\Vdash \\beta \\href{http://example.org}{+ \\alpha} \\times \\gamma"));
+    });
+
     it('should render mathchoice as if there was nothing', () => {
         const cmd = "\\sum_{k = 0}^{\\infty} x^k";
         expect(getMathML(`\\displaystyle\\mathchoice{${cmd}}{T}{S}{SS}`))