KaTeX (3/n): Support negative margins for spans #1559

rajveermalviya · 2025-06-10T15:50:39Z

Stacked on top of #1698.

Related: #46

gnprice · 2025-06-10T21:13:11Z

@rajveermalviya Copying here for visibility what you told me on our call today: the reason this is a draft (not ready to merge) is that you're still working on the widget test. Apart from that, you consider it ready to review.

gnprice

Thanks! Here's small comments from a partial skim.

gnprice · 2025-06-12T16:57:03Z

lib/widgets/katex.dart

+// Like [RenderPadding] but supports negative values.
+class RenderNegativePadding extends RenderShiftedBox {


Let's leave a TODO tracking our intention to get a version of this upstream:

Suggested change

// Like [RenderPadding] but supports negative values.

class RenderNegativePadding extends RenderShiftedBox {

// Like [RenderPadding] but supports negative values.

// TODO(upstream): give Padding an option to accept negative padding (at cost of hit-testing not working)

class RenderNegativePadding extends RenderShiftedBox {

gnprice · 2025-06-12T16:58:39Z

lib/widgets/katex.dart

+extension on EdgeInsetsGeometry {
+  bool get isNegative => !isNonNegative;


I think this is clearer (and hardly much longer) if just inlined.

In particular there's some ambiguity in that "is negative" sounds like it might require all the sides to be negative. So being more transparent, with one fewer layer of indirection, helps mitigate that.

rajveermalviya · 2025-06-20T21:26:45Z

Thanks for the initial comments @gnprice. Revision pushed.

gnprice

Thanks! I haven't yet re-reviewed this, as work on #1452 is still ongoing.

Would you rebase this atop the current #1452? That's helpful for me to be able to continue shipping them both in releases.

One comment below from when I tried a version of that rebase just now.

gnprice · 2025-07-09T01:28:51Z

lib/widgets/content.dart

+    final marginRight = switch (styles.marginRightEm) {
+      double marginRightEm when marginRightEm >= 0 => marginRightEm * em,


Is it intended to be possible to get to this point and have a negative marginRightEm?

I believe not — and it looks like if it did happen, this code would just ignore it, which seems like clearly a bug.

So instead of this "when" condition, let's have an assertion:

assert((styles.marginLeftEm ?? 0) >= 0);

Similarly for right margin.

Updated so that parser makes sure not to emit a node with negative margin styles, as it already emits a separate KatexNegativeMarginNode for handling that on the widget side. And widget side retains the assert(margin.isNonNegative);.

Great, thanks — that definitely sounds cleaner.

rajveermalviya · 2025-07-09T15:03:20Z

Thanks for the review @gnprice! Pushed an update, PTAL.

gnprice

Thanks! This logic looks good. Small comments below on the tests.

(I also have some ideas on how to refactor and simplify the logic — but for the same reasons as at #1698 (comment), because this has already shipped in recent releases, I'll want to save that kind of change for after this PR has been merged.)

gnprice · 2025-07-16T22:02:47Z

test/model/content_test.dart

+      MathBlockNode(
+        texSource: '\\KaTeX',
+        nodes: [
+          KatexSpanNode(
+            styles: KatexSpanStyles(),
+            text: null,
+            nodes: [
+              KatexStrutNode(heightEm: 0.8988, verticalAlignEm: -0.2155),
+              KatexSpanNode(
+                styles: KatexSpanStyles(),
+                text: null,
+                nodes: [
+                  KatexSpanNode(


nit: Let's make the boring parts of this more compact, and also involve less indentation. Like so:

Suggested change

MathBlockNode(

texSource: '\\KaTeX',

nodes: [

KatexSpanNode(

styles: KatexSpanStyles(),

text: null,

nodes: [

KatexStrutNode(heightEm: 0.8988, verticalAlignEm: -0.2155),

KatexSpanNode(

styles: KatexSpanStyles(),

text: null,

nodes: [

KatexSpanNode(

MathBlockNode(texSource: '\\KaTeX', nodes: [

KatexSpanNode(styles: KatexSpanStyles(), text: null, nodes: [

KatexStrutNode(heightEm: 0.8988, verticalAlignEm: -0.2155),

KatexSpanNode(styles: KatexSpanStyles(), text: null, nodes: [

KatexSpanNode(

and similarly below.

gnprice · 2025-07-16T22:08:52Z

test/model/content_test.dart

+  static const mathBlockKatexNegativeMargins = ContentExample(
+    'math block katex negative margins',
+    '```math\n\\KaTeX\n```',


This example is fine to have. But let's also have a more minimal, focused example that's just about negative margins. So:

Suggested change

static const mathBlockKatexNegativeMargins = ContentExample(

'math block katex negative margins',

'```math\n\\KaTeX\n```',

static const mathBlockKatexLogo = ContentExample(

'math block KaTeX logo',

'```math\n\\KaTeX\n```',

and then for a minimal example, try $$ 1 \! 2 $$. The minimal example can get a name like "katex negative margins".

For anyone who's investigating this in the future — or who makes a change, the change causes a regression here, and they need to investigate the regression to figure out the right fix — that minimal, focused example should be helpful as the place to look.

Implement handling most common types of `vlist` spans.

Turns out that anything under KatexVlistRowNode wasn't being tested by content tests, fix that by implementing this method. Fortunately there were no fixes needed in the tests.

Also add a comment explaining the reason of ignoring the `height` inline styles value.

Negative margin spans on web render to the offset being applied to the specific span and all the adjacent spans, so mimic the same behaviour here.

…ontent Skipped currently, because the parser incorrectly doesn't filter the negative margin styles on the vlist row span after generating a `KatexNegativeMarginNode` to specifically handle negative margins.

This fixes a bug where if negative margin on a vlist row is present the widget side code would hit an assert. Displaying the error (red screen) in debug mode, but in release mode the negative padding would be applied twice, once by `_KatexNegativeMargin` and another by `Padding` widget.

gnprice

Thanks for the revision! Comments below, mainly on the two new commits.

gnprice · 2025-07-18T03:40:02Z

test/model/content_test.dart

+      MathBlockNode(
+        texSource: 'X_n',
+        nodes: [
+          KatexSpanNode(
+            styles: KatexSpanStyles(),
+            text: null,
+            nodes: [


nit: same story as #1559 (comment) and #1698 (comment)

gnprice · 2025-07-18T03:44:57Z

test/model/content_test.dart

+  static const mathBlockKatexNegativeMargin = ContentExample(
+    'math block, KaTeX negative margin',
+    // https://chat.zulip.org/#narrow/channel/7-test-here/topic/Rajesh/near/2223563
+    '```math\n1 \\! 2\n```',


Let's be sure to include this new, minimal example (from #1559 (comment)) in the widget tests too, so that it can fully do its work.

gnprice · 2025-07-18T03:50:16Z

lib/model/katex.dart

+                      styles: styles.filter(topEm: false, marginLeftEm: false),
+                      text: null,
+                      nodes: _parseChildSpans(otherSpans))])]);
+              } else {
+                innerSpanNode = KatexSpanNode(
+                  styles: styles.filter(topEm: false),


At this point styles has already had topEm filtered out, right? So I believe that filtering is redundant.

gnprice · 2025-07-18T03:58:34Z

test/model/content_test.dart

+  testParseExample(ContentExample.mathBlockKatexRaisebox);
+  testParseExample(ContentExample.mathBlockKatexNegativeMargin);
+  testParseExample(ContentExample.mathBlockKatexLogo);
+  testParseExample(ContentExample.mathBlockNegativeMarginsOnVlistRow);


From the last two commits (the commits newly added in yesterday's revision, mentioned at #1698 (comment)):
f845c6d content test: Add test for negative margins on a vlist row in KaTeX content
0ec2cc5 content: Filter negative margin styles if present on a vlist row

+ testParseExample(ContentExample.mathBlockNegativeMarginsOnVlistRow, + // TODO fix parser to filter negative margins styles from + // vlist row span styles. + skip: true);

- testParseExample(ContentExample.mathBlockNegativeMarginsOnVlistRow, - // TODO fix parser to filter negative margins styles from - // vlist row span styles. - skip: true); + testParseExample(ContentExample.mathBlockNegativeMarginsOnVlistRow);

This version doesn't really give any added information compared to just squashing the two commits together. The normal arrangement is to have the tests appear in the same commit as the changes they're testing, because then the tests can be read in the light of those changes and vice versa, which can help for understanding both of them.

There's a variation of this split which would be useful, though:

The first commit adds the test, but doesn't skip it; instead, the expectation reflects the old wrong behavior, and a TODO comment points out where it's wrong.

Then the second commit makes the fix, and updates the test to expect the right behavior.

If the updates to the test are short and readable — simpler to read than the whole test itself — then this can be a useful way of highlighting exactly which aspect of the test expectation is the thing getting fixed by the change.

If I'm understanding this change correctly, then this would be a good example of that: the bit that's wrong would be a single line with a marginLeftEm.

Relatedly:

content: Filter negative margin styles if present on a vlist row This fixes a bug where if negative margin on a vlist row is present the widget side code would hit an assert. Displaying the error (red screen) in debug mode, but in release mode the negative padding would be applied twice, once by `_KatexNegativeMargin` and another by `Padding` widget.

Since one of the symptoms is that the widget code hits an assert, let's have a widget test that would exercise that assert. :-)

gnprice added the maintainer review PR ready for review by Zulip maintainers label Jun 10, 2025

rajveermalviya force-pushed the pr-tex-content-3 branch from c33f5c2 to 6cfcc0d Compare June 11, 2025 17:34

rajveermalviya marked this pull request as ready for review June 11, 2025 17:34

rajveermalviya force-pushed the pr-tex-content-3 branch 2 times, most recently from 843207d to d1f5d5c Compare June 11, 2025 19:19

gnprice reviewed Jun 12, 2025

View reviewed changes

gnprice mentioned this pull request Jun 17, 2025

TeX in the message list #46

Closed

rajveermalviya force-pushed the pr-tex-content-3 branch 2 times, most recently from 92674c9 to 39d2937 Compare June 17, 2025 09:47

This was referenced Jun 17, 2025

tools/content: Support surveying unimplemented KaTeX features #1600

Merged

KaTeX (4/n): Ignore classes that don't have CSS definition #1601

Merged

rajveermalviya force-pushed the pr-tex-content-3 branch from 39d2937 to bf886a6 Compare June 19, 2025 11:08

rajveermalviya force-pushed the pr-tex-content-3 branch from bf886a6 to e8cd58a Compare July 3, 2025 20:47

gnprice added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Jul 4, 2025

rajveermalviya mentioned this pull request Jul 4, 2025

KaTeX(7/n): Handle 'op-limits' KaTeX CSS #1669

Draft

gnprice reviewed Jul 9, 2025

View reviewed changes

rajveermalviya force-pushed the pr-tex-content-3 branch 2 times, most recently from bd56117 to dfabd4d Compare July 9, 2025 14:58

rajveermalviya requested a review from gnprice July 9, 2025 15:03

rajveermalviya force-pushed the pr-tex-content-3 branch from dfabd4d to 2bae0c2 Compare July 9, 2025 22:51

gnprice force-pushed the pr-tex-content-3 branch from 2bae0c2 to 2e60440 Compare July 10, 2025 02:55

gnprice mentioned this pull request Jul 10, 2025

KaTeX (2/n): Support for vertical offsets in spans #1698

Open

gnprice reviewed Jul 16, 2025

View reviewed changes

rajveermalviya force-pushed the pr-tex-content-3 branch from 2e60440 to d4ccfc8 Compare July 17, 2025 00:50

rajveermalviya added 2 commits July 17, 2025 20:30

content: Handle vertical offset spans in KaTeX content

b350875

Implement handling most common types of `vlist` spans.

content: Error message for unexpected CSS class in vlist inner span

0ec184e

rajveermalviya added 2 commits July 17, 2025 20:30

content: Implement debugDescribeChildren for KatexVlistRowNode

b0ca894

Turns out that anything under KatexVlistRowNode wasn't being tested by content tests, fix that by implementing this method. Fortunately there were no fixes needed in the tests.

content: Make sure there aren't any unexpected styles on .vlist span

280bf3c

Also add a comment explaining the reason of ignoring the `height` inline styles value.

rajveermalviya force-pushed the pr-tex-content-3 branch from d4ccfc8 to 04f2e04 Compare July 17, 2025 16:23

rajveermalviya added 3 commits July 17, 2025 22:20

content: Support negative margins on KaTeX spans

39c53bd

Negative margin spans on web render to the offset being applied to the specific span and all the adjacent spans, so mimic the same behaviour here.

content test: Add test for negative margins on a vlist row in KaTeX c…

f845c6d

…ontent Skipped currently, because the parser incorrectly doesn't filter the negative margin styles on the vlist row span after generating a `KatexNegativeMarginNode` to specifically handle negative margins.

rajveermalviya force-pushed the pr-tex-content-3 branch from 04f2e04 to 0ec2cc5 Compare July 17, 2025 16:55

gnprice reviewed Jul 18, 2025

View reviewed changes

		// Like [RenderPadding] but supports negative values.
		class RenderNegativePadding extends RenderShiftedBox {

		extension on EdgeInsetsGeometry {
		bool get isNegative => !isNonNegative;

		final marginRight = switch (styles.marginRightEm) {
		double marginRightEm when marginRightEm >= 0 => marginRightEm * em,

KaTeX (3/n): Support negative margins for spans #1559

Are you sure you want to change the base?

KaTeX (3/n): Support negative margins for spans #1559

Conversation

rajveermalviya commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnprice commented Jun 10, 2025

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajveermalviya commented Jun 20, 2025

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajveermalviya commented Jul 9, 2025

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rajveermalviya commented Jun 10, 2025 •

edited

Loading