Repository: Khan/KaTeX
Pull request #1690 (2018-08-31 21:52:45)

Allow 6-digit color spec w/o #
This PR takes a minimalist approach to addressing issue #1657. This does **not** completely change the color specification syntax to align with the `color` package or the `xcolor` package. It does not accommodate RGB notation or a cmyk color model.  It does, however, allow a 6-digit HTML color specification to pass without a leading `#` symbol, as in `xcolor`'s HTML color model.

codecov[bot]: # [Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1690?src=pr&el=h1) Report
> Merging [#1690](https://codecov.io/gh/Khan/KaTeX/pull/1690?src=pr&el=desc) into [master](https://codecov.io/gh/Khan/KaTeX/commit/d3ec100d02419672bb2de6772dff50b5b401488f?src=pr&el=desc) will **decrease** coverage by `5.16%`.
> The diff coverage is `50%`.


[![Impacted file tree graph](https://codecov.io/gh/Khan/KaTeX/pull/1690/graphs/tree.svg?width=650&token=vfDnj2P77o&height=150&src=pr)](https://codecov.io/gh/Khan/KaTeX/pull/1690?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1690      +/-   ##
==========================================
- Coverage   93.84%   88.68%   -5.17%     
==========================================
  Files          78       73       -5     
  Lines        4568     4426     -142     
  Branches      801        0     -801     
==========================================
- Hits         4287     3925     -362     
- Misses        248      501     +253     
+ Partials       33        0      -33
```


| [Impacted Files](https://codecov.io/gh/Khan/KaTeX/pull/1690?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/Parser.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL1BhcnNlci5qcw==) | `82.85% <50%> (-14.25%)` | :arrow_down: |
| [src/ParseError.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL1BhcnNlRXJyb3IuanM=) | `4.34% <0%> (-95.66%)` | :arrow_down: |
| [src/functions/href.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL2Z1bmN0aW9ucy9ocmVmLmpz) | `10.52% <0%> (-89.48%)` | :arrow_down: |
| [src/Settings.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL1NldHRpbmdzLmpz) | `29.41% <0%> (-44.12%)` | :arrow_down: |
| [src/domTree.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL2RvbVRyZWUuanM=) | `56.79% <0%> (-35.81%)` | :arrow_down: |
| [src/mathMLTree.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL21hdGhNTFRyZWUuanM=) | `66.1% <0%> (-32.21%)` | :arrow_down: |
| [src/buildTree.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL2J1aWxkVHJlZS5qcw==) | `64.7% <0%> (-29.42%)` | :arrow_down: |
| [src/tree.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL3RyZWUuanM=) | `58.82% <0%> (-23.53%)` | :arrow_down: |
| [src/parseTree.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL3BhcnNlVHJlZS5qcw==) | `83.33% <0%> (-16.67%)` | :arrow_down: |
| [src/macros.js](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree#diff-c3JjL21hY3Jvcy5qcw==) | `81.84% <0%> (-15.28%)` | :arrow_down: |
| ... and [29 more](https://codecov.io/gh/Khan/KaTeX/pull/1690/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1690?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Khan/KaTeX/pull/1690?src=pr&el=footer). Last update [d3ec100...ad53ad4](https://codecov.io/gh/Khan/KaTeX/pull/1690?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

ronkok: The latest commit revises the RegEx pattern as recommended by @ylemkimon. (Thanks!) This simplifies the code but it does not raise a parse error on a mis-typed predefined color name. `\color{cherry}` will render in black.

I'd like to raise an error on a mis-typed predefined color name, but the only way I can think to accomplish that is via a RegEx pattern that enumerates all 150 of the [HTML predefined colors](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords). Is that something I should do?
ylemkimon: > I'd like to raise an error on a mis-typed predefined color name, but the only way I can think to accomplish that is via a RegEx pattern that enumerates all 150 of the [HTML predefined colors](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords). Is that something I should do?

I think it is impossible without access to DOM APIs and there is no need to check the color name is valid.

Patch:
diff --git a/docs/supported.md b/docs/supported.md
index e3361d19..5b4fd607 100644
--- a/docs/supported.md
+++ b/docs/supported.md
@@ -508,7 +508,7 @@ $\textcolor{#228B22}{F=ma}$ `\textcolor{#228B22}{F=ma}`<br>
 $\colorbox{aqua}{A}$ `\colorbox{aqua}{A}`<br>
 $\fcolorbox{red}{aqua}{A}$ `\fcolorbox{red}{aqua}{A}`
 
-For color definition, KaTeX color functions will accept the standard HTML [predefined color names](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords). They will also accept an RGB argument in CSS hexa­decimal style.
+For color definition, KaTeX color functions will accept the standard HTML [predefined color names](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value#Color_keywords). They will also accept an RGB argument in CSS hexa­decimal style. The "#" is optional before a six-digit specification.
 
 **Font**
 
diff --git a/src/Parser.js b/src/Parser.js
index 541a2b5d..9d581164 100644
--- a/src/Parser.js
+++ b/src/Parser.js
@@ -743,14 +743,21 @@ export default class Parser {
         if (!res) {
             return null;
         }
-        const match = (/^(#[a-f0-9]{3}|#[a-f0-9]{6}|[a-z]+)$/i).exec(res.text);
+        const match = (/^(#[a-f0-9]{3}|#?[a-f0-9]{6}|[a-z]+)$/i).exec(res.text);
         if (!match) {
             throw new ParseError("Invalid color: '" + res.text + "'", res);
         }
+        let color = match[0];
+        if (/^[0-9a-f]{6}$/i.test(color)) {
+            // We allow a 6-digit HTML color spec without a leading "#".
+            // This follows the xcolor package's HTML color model.
+            // Predefined color names are all missed by this RegEx pattern.
+            color = "#" + color;
+        }
         return newArgument({
             type: "color-token",
             mode: this.mode,
-            color: match[0],
+            color,
         }, res);
     }
 


Test patch:
diff --git a/test/errors-spec.js b/test/errors-spec.js
index 2f1525c1..e41091fc 100644
--- a/test/errors-spec.js
+++ b/test/errors-spec.js
@@ -296,10 +296,10 @@ describe("Lexer:", function() {
     });
 
     describe("#_innerLexColor", function() {
-        it("reject hex notation without #", function() {
-            expect`\textcolor{1a2b3c}{foo}`.toFailWithParseError(
-                   "Invalid color: '1a2b3c'" +
-                   " at position 12: \\textcolor{1̲a̲2̲b̲3̲c̲}{foo}");
+        it("reject 3-digit hex notation without #", function() {
+            expect`\textcolor{1a2}{foo}`.toFailWithParseError(
+                   "Invalid color: '1a2'" +
+                   " at position 12: \\textcolor{1̲a̲2̲}{foo}");
         });
     });
 
diff --git a/test/katex-spec.js b/test/katex-spec.js
index ffe4117c..4b7f6be4 100644
--- a/test/katex-spec.js
+++ b/test/katex-spec.js
@@ -780,6 +780,7 @@ describe("A color parser", function() {
     const newColorExpression = r`\redA{x}`;
     const customColorExpression1 = r`\textcolor{#fA6}{x}`;
     const customColorExpression2 = r`\textcolor{#fA6fA6}{x}`;
+    const customColorExpression3 = r`\textcolor{fA6fA6}{x}`;
     const badCustomColorExpression1 = r`\textcolor{bad-color}{x}`;
     const badCustomColorExpression2 = r`\textcolor{#fA6f}{x}`;
     const badCustomColorExpression3 = r`\textcolor{#gA6}{x}`;
@@ -800,14 +801,17 @@ describe("A color parser", function() {
     it("should parse a custom color", function() {
         expect(customColorExpression1).toParse();
         expect(customColorExpression2).toParse();
+        expect(customColorExpression3).toParse();
     });
 
     it("should correctly extract the custom color", function() {
         const parse1 = getParsed(customColorExpression1)[0];
         const parse2 = getParsed(customColorExpression2)[0];
+        const parse3 = getParsed(customColorExpression3)[0];
 
         expect(parse1.color).toEqual("#fA6");
         expect(parse2.color).toEqual("#fA6fA6");
+        expect(parse3.color).toEqual("#fA6fA6");
     });
 
     it("should not parse a bad custom color", function() {