Skip to content

Commit

Permalink
Fix #533, #534, #541.
Browse files Browse the repository at this point in the history
- #534: Implement getTypeOfGroup for font groups.
- #533, #541: Improve the ways spaces are applied to lists. Since
  CSS adjacency implements mathematical spacing, it's incorrect to
  introduce "convenience spans" for spaces and display changes into
  the generated HTML -- those spans break adjacency. Apply display
  changes directly, and shift space spans into adjacent atoms.

Requires updates to two screenshotter tests, LimitControls and
SupSubLeftAlignReset. The new results for these tests are closer
to TeX output than the old results.

Also requires updates to Jasmine tests, since those assumed output
structures that have changed.
  • Loading branch information
kohler committed Nov 28, 2016
1 parent 38c14e5 commit d3ecb33
Show file tree
Hide file tree
Showing 16 changed files with 88 additions and 19 deletions.
19 changes: 16 additions & 3 deletions src/buildCommon.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,16 @@ var mainitLetters = [
* Makes a symbolNode after translation via the list of symbols in symbols.js.
* Correctly pulls out metrics for the character, and optionally takes a list of
* classes to be attached to the node.
*
* TODO: make argument order closer to makeSpan
*/
var makeSymbol = function(value, style, mode, options, classes) {
var makeSymbol = function(value, fontFamily, mode, options, classes) {
// Replace the value with its replaced value from symbol.js
if (symbols[mode][value] && symbols[mode][value].replace) {
value = symbols[mode][value].replace;
}

var metrics = fontMetrics.getCharacterMetrics(value, style);
var metrics = fontMetrics.getCharacterMetrics(value, fontFamily);

var symbolNode;
if (metrics) {
Expand All @@ -52,7 +54,7 @@ var makeSymbol = function(value, style, mode, options, classes) {
// TODO(emily): Figure out a good way to only print this in development
typeof console !== "undefined" && console.warn(
"No character metrics for '" + value + "' in style '" +
style + "'");
fontFamily + "'");
symbolNode = new domTree.symbolNode(value, 0, 0, 0, 0, classes);
}

Expand Down Expand Up @@ -190,6 +192,16 @@ var makeSpan = function(classes, children, options) {
return span;
};

/**
* Prepends the given children to the given span, updating height, depth, and
* maxFontSize.
*/
var prependChildren = function(span, children) {
span.children = children.concat(span.children);

sizeElementFromChildren(span);
};

/**
* Makes a document fragment with the given list of children.
*/
Expand Down Expand Up @@ -450,6 +462,7 @@ module.exports = {
makeFragment: makeFragment,
makeVList: makeVList,
makeOrd: makeOrd,
prependChildren: prependChildren,
sizingMultiplier: sizingMultiplier,
spacingFunctions: spacingFunctions,
};
76 changes: 64 additions & 12 deletions src/buildHTML.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,57 @@ var utils = require("./utils");

var makeSpan = buildCommon.makeSpan;

var isSpace = function(node) {
return node instanceof domTree.span && node.classes[0] === "mspace";
};

/**
* Take a list of nodes, build them in order, and return a list of the built
* nodes. This function handles the `prev` node correctly, and passes the
* previous element from the list as the prev of the next element.
* previous element from the list as the prev of the next element, ignoring
* spaces. documentFragments are flattened into their contents, so the
* returned list contains no fragments.
*/
var buildExpression = function(expression, options, prev) {
// Parse expressions into `groups`.
var groups = [];
for (var i = 0; i < expression.length; i++) {
var group = expression[i];
groups.push(buildGroup(group, options, prev));
prev = group;
var output = buildGroup(group, options, prev);
if (output instanceof domTree.documentFragment) {
Array.prototype.push.apply(groups, output.children);
} else {
groups.push(output);
}
if (!isSpace(output)) {
prev = group;
}
}
// At this point `groups` consists entirely of `symbolNode`s and `span`s.

// Explicit spaces (e.g., \;, \,) should be ignored with respect to atom
// spacing (e.g., "add thick space between mord and mrel"). Since CSS
// adjacency rules implement atom spacing, spaces should be invisible to
// CSS. So we splice them out of `groups` and into the atoms themselves.
var spaces = null;
for (i = 0; i < groups.length; i++) {
if (isSpace(groups[i])) {
spaces = spaces || [];
spaces.push(groups[i]);
groups.splice(i, 1);
i--;

This comment has been minimized.

Copy link
@kevinbarabash

kevinbarabash Nov 28, 2016

Member

Is this splice and decrement necessary? Either way the for loop will advance to the next item.

This comment has been minimized.

Copy link
@kohler

kohler Nov 28, 2016

Author Collaborator

Yes they are necessary. splice destructively modifies groups to remove the space.

This comment has been minimized.

Copy link
@kevinbarabash

kevinbarabash Nov 28, 2016

Member

I see, we're returning groups at the end. Sorry about all of the silly questions.

} else if (spaces) {
if (groups[i] instanceof domTree.symbolNode) {
groups[i] = makeSpan(groups[i].classes, [groups[i]]);
}
buildCommon.prependChildren(groups[i], spaces);
spaces = null;
}
}
if (spaces) {
Array.prototype.push.apply(groups, spaces);
}

return groups;
};

Expand Down Expand Up @@ -80,12 +119,13 @@ var getTypeOfGroup = function(group) {
return getTypeOfGroup(group.value.base);
} else if (group.type === "llap" || group.type === "rlap") {
return getTypeOfGroup(group.value);
} else if (group.type === "color") {
return getTypeOfGroup(group.value.value);
} else if (group.type === "sizing") {
return getTypeOfGroup(group.value.value);
} else if (group.type === "styling") {
return getTypeOfGroup(group.value.value);
} else if (group.type === "color" || group.type === "sizing"
|| group.type === "styling") {
// Return type of rightmost element of group.
var atoms = group.value.value;
return getTypeOfGroup(atoms[atoms.length - 1]);
} else if (group.type === "font") {
return getTypeOfGroup(group.value.body);
} else if (group.type === "delimsizing") {
return groupToType[group.value.delimType];
} else {
Expand Down Expand Up @@ -697,14 +737,14 @@ groupTypes.spacing = function(group, options, prev) {
// things has an entry in the symbols table, so these will be turned
// into appropriate outputs.
return makeSpan(
["mord", "mspace"],
["mspace"],
[buildCommon.mathsym(group.value, group.mode)]
);
} else {
// Other kinds of spaces are of arbitrary width. We use CSS to
// generate these.
return makeSpan(
["mord", "mspace",
["mspace",
buildCommon.spacingFunctions[group.value].className]);
}
};
Expand Down Expand Up @@ -1109,7 +1149,19 @@ groupTypes.styling = function(group, options, prev) {
var inner = buildExpression(
group.value.value, newOptions, prev);

return makeSpan([options.style.reset(), newStyle.cls()], inner, newOptions);
// Add style-resetting classes to the inner list. Handle nested changes.
for (var i = 0; i < inner.length; i++) {
var pos = utils.indexOf(inner[i].classes, newStyle.reset());
if (pos < 0) {
inner[i].classes.push(options.style.reset(), newStyle.cls());
} else {
// This is a nested style change, as `\textstyle a\scriptstyle b`.
// Only override the old style (the reset class).
inner[i].classes[pos] = options.style.reset();
}
}

return new buildCommon.makeFragment(inner);
};

groupTypes.font = function(group, options, prev) {
Expand Down
9 changes: 5 additions & 4 deletions test/katex-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1581,7 +1581,8 @@ describe("A bin builder", function() {

it("should correctly interact with color objects", function() {
expect(getBuilt("\\blue{x}+y")[1].classes).toContain("mbin");
expect(getBuilt("\\blue{x+}+y")[1].classes).toContain("mord");
expect(getBuilt("\\blue{x+}+y")[1].classes).toContain("mbin");
expect(getBuilt("\\blue{x+}+y")[2].classes).toContain("mord");
});
});

Expand Down Expand Up @@ -1695,17 +1696,17 @@ describe("A phantom builder", function() {
});

it("should make the children transparent", function() {
var children = getBuilt("\\phantom{x+1}")[0].children;
var children = getBuilt("\\phantom{x+1}");
expect(children[0].style.color).toBe("transparent");
expect(children[1].style.color).toBe("transparent");
expect(children[2].style.color).toBe("transparent");
});

it("should make all descendants transparent", function() {
var children = getBuilt("\\phantom{x+\\blue{1}}")[0].children;
var children = getBuilt("\\phantom{x+\\blue{1}}");
expect(children[0].style.color).toBe("transparent");
expect(children[1].style.color).toBe("transparent");
expect(children[2].children[0].style.color).toBe("transparent");
expect(children[2].style.color).toBe("transparent");
});
});

Expand Down
Binary file added test/screenshotter/images/BoldSpacing-chrome.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/screenshotter/images/BoldSpacing-firefox.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/screenshotter/images/ColorSpacing-chrome.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/screenshotter/images/LimitControls-chrome.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/screenshotter/images/LimitControls-firefox.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/screenshotter/images/StyleSwitching-chrome.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/screenshotter/images/StyleSwitching-firefox.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/screenshotter/images/SupSubLeftAlignReset-chrome.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/screenshotter/images/SupSubLeftAlignReset-firefox.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions test/screenshotter/ss_data.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ ArrayType: 1\begin{array}{c}2\\3\end{array}4
Baseline: a+b-c\cdot d/e
BasicTest: a
BinomTest: \dbinom{a}{b}\tbinom{a}{b}^{\binom{a}{b}+17}
BoldSpacing: \mathbf{A}^2+\mathbf{B}_3*\mathscr{C}'
Cases: |
f(a,b)=\begin{cases}
a+1&\text{if }b\text{ is odd} \\
Expand All @@ -36,6 +37,7 @@ Cases: |
Colors:
tex: \blue{a}\color{#0f0}{b}\color{red}{c}
nolatex: different syntax and different scope
ColorSpacing: \color{red}{\displaystyle \int x} + 1
DashesAndQuotes: \text{``a'' b---c -- d----`e'-{-}-f}--``x''
DeepFontSizing:
tex: |
Expand Down Expand Up @@ -82,6 +84,7 @@ MathRm: \mathrm{Ax2k\breve{a}\omega\Omega\imath+\KaTeX}
MathSf: \mathsf{Ax2k\breve{a}\omega\Omega\imath+\KaTeX}
MathScr: \mathscr{Ax2k\breve{a}\omega\Omega\imath+\KaTeX}
MathTt: \mathtt{Ax2k\breve{a}\omega\Omega\imath+\KaTeX}
NegativeSpaceBetweenRel: A =\!= B
NestedFractions: |
\dfrac{\frac{a}{b}}{\frac{c}{d}}\dfrac{\dfrac{a}{b}}
{\dfrac{c}{d}}\frac{\frac{a}{b}}{\frac{c}{d}}
Expand Down

0 comments on commit d3ecb33

Please sign in to comment.