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

rajveermalviya · 2025-07-09T22:57:15Z

Stacked on top of: #1452

Web	Flutter

Related: #46

gnprice

Thanks for all your work on this! Now that #1452 is merged, I'm reaching these commits for detailed review :-)

Comments below. Posting this now because I'm going AFK, but I've read most of the branch so far: partway through the tests, and read all the other changes.

gnprice · 2025-07-10T22:11:51Z

lib/model/katex.dart

+          dom.Element(localName: 'span', className: 'vlist-r', nodes: [
+            dom.Element(localName: 'span', className: 'vlist', nodes: [
+              dom.Element(localName: 'span', className: '', nodes: []),
+            ]),
+          ]),
+        ]) {


Interesting — do we not need any information from this second row of the table?

It looks like an example of how that second row looks in the HTML is (from ContentExample.mathBlockKatexVertical2 added here):

 

What's the effect of that height value when the whole vlist-t gets rendered in a browser?

The value of height on span.vlist-r span.vlist specifies the height of the whole span.vlist-t. And if there are two span.vlist-r then the addition of height value of both span.vlist-r span.vlist specifies the height of the whole span.vlist-t.

From the web dev console, disabling height on either span.vlist-r span.vlist doesn't seem to affect the rendering.

Hmm, OK. A bit odd that there's that information there and it's not getting used; but maybe it's another example where it either used to get used, or gets used within KaTeX's internal processing.

Let's definitely mark that with a comment, though, because otherwise it looks wrong to a reader who's comparing this parsing code to the HTML. In fact we should do that for both of the .vlist cases — this one for the second .vlist-r's child, and below for the first/main .vlist-r's child.

gnprice · 2025-07-10T22:13:47Z

lib/model/katex.dart

+      if (vlistT.nodes.isEmpty) throw _KatexHtmlParseError();
+      if (vlistT.attributes.containsKey('style')) throw _KatexHtmlParseError();
+
+      final hasTwoVlistR = vlistT.className == 'vlist-t vlist-t2';


There's also a CSS rule for .vlist-t2 in the KaTeX CSS:

.vlist-t2 { margin-right: -2px; }

What's the net effect of that? On its face it looks like something we should have to take account of.

This will need support for negative margins, so will add a commit in the next PR for this. (It will also need changes for supporting non-em values in KatexSpanStyles.)

Cool, sounds good. Please file a brief follow-up issue for that, then; and then this code can have a TODO comment mentioning that issue.

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

lib/model/katex.dart

+
+          return KatexVlistNode(
+            rows: rows,
+            debugHtmlNode: kDebugMode ? vlistT : null,


nit:

Suggested change

debugHtmlNode: kDebugMode ? vlistT : null,

debugHtmlNode: debugHtmlNode,

This is equivalent, right? Seems simpler for the reader — makes it the same as most of our debugHtmlNode arguments.

gnprice · 2025-07-10T22:19:23Z

lib/model/katex.dart

+          final rows = <KatexVlistRowNode>[];
+
+          for (final innerSpan in vlist.nodes) {


Hmm, what's meant by calling these "rows"?

In CSS terms, the element that's a "row" here is the .vlist-r — that's the one with display: table-row. But that's the grandparent of these elements — these are the children of the .vlist, which is one cell of the table (having display: table-cell).

IIRC, I called them "rows" because they seem like rows in a column with different vertical offset.

OK, we can go with that.

gnprice · 2025-07-10T22:22:35Z

lib/model/katex.dart

+              if (innerSpan.className != '') {
+                throw _KatexHtmlParseError('unexpected CSS class for '
+                  'vlist inner span: ${innerSpan.className}');
+              }


I guess this points at a different, sometimes-applicable downside of if-case syntax (other than the fact that it puts the failure case way at the bottom on the other side of the main code, which we can hope to someday have a "guard let" feature to address): it doesn't make room for emitting specific errors for specific types of failures to match.

gnprice · 2025-07-10T22:38:08Z

test/model/content_test.dart

+  static const mathBlockKatexVertical2 = ContentExample(
+    'math block katex vertical 2',


Can you give these examples names that reflect what interesting situations they're each exercising that isn't covered by the other examples?

For example, this one introduces vlist-t2, which is definitely an interesting case that's good to be sure to cover.

gnprice · 2025-07-10T22:39:21Z

test/model/content_test.dart

+  static const mathBlockKatexVertical3 = ContentExample(
+    'math block katex vertical 3',


(And I'm not sure what this example demonstrates that isn't covered by the "vertical 1" example above. So that would be good to make explicit.)

Removed this example as it was technically same as "vertical 1", just different characters.

gnprice · 2025-07-10T22:43:58Z

(As we discussed on #1452 earlier, let's avoid major revisions to these commits until this PR and #1559 are merged — that will help reduce the work of tracking what changed between the last few releases, which have already included versions of these, and the version we eventually merge. Instead let's stick to small edits for the moment where possible, and make larger changes as follow-up PRs.)

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.

rajveermalviya · 2025-07-17T01:10:27Z

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

Also, I've added commits for 2 bug fixes:

ca00158 content: Implement debugDescribeChildren for KatexVlistRowNode
(in #1559) 2e0c092 content test: Add test for negative margins on a vlist row in KaTeX content
(in #1559) d4ccfc8 content: Filter negative margin styles if present on a vlist row

rajveermalviya · 2025-07-17T01:12:24Z

CI failure is unrelated, will investigate later.
Logs:

2025-07-17T00:50:46.9768068Z Resolving dependencies...
2025-07-17T00:50:49.0495710Z The current Flutter SDK version is 0.0.0-unknown.
2025-07-17T00:50:49.0496135Z 
2025-07-17T00:50:49.0496587Z Because zulip requires Flutter SDK version >=3.33.0-1.0.pre.832, version solving failed.
2025-07-17T00:50:49.0497167Z 
2025-07-17T00:50:49.0497175Z 
2025-07-17T00:50:49.0497468Z You can try the following suggestion to make the pubspec resolve:
2025-07-17T00:50:49.0498127Z * Try using the Flutter SDK version: 3.35.0-0.0.pre. 
2025-07-17T00:50:49.1161302Z Failed to update packages.

gnprice

Thanks for the revision! Comments below.

gnprice · 2025-07-17T04:27:49Z

lib/model/katex.dart

+          final rows = <KatexVlistRowNode>[];
+
+          for (final innerSpan in vlist.nodes) {


OK, we can go with that.

gnprice · 2025-07-17T04:30:46Z

lib/model/katex.dart

+      if (vlistT.nodes.isEmpty) throw _KatexHtmlParseError();
+      if (vlistT.attributes.containsKey('style')) throw _KatexHtmlParseError();
+
+      final hasTwoVlistR = vlistT.className == 'vlist-t vlist-t2';


Cool, sounds good. Please file a brief follow-up issue for that, then; and then this code can have a TODO comment mentioning that issue.

gnprice · 2025-07-17T04:44:23Z

lib/model/content.dart

+  List<DiagnosticsNode> debugDescribeChildren() {
+    return [node.toDiagnosticsNode()];


content: Implement debugDescribeChildren for KatexVlistRowNode Turns out that anything under KatexVlistRowNode wasn't being tested by content tests, fix that by implementing this method.

Hmm indeed, good to catch this!

This meant it wasn't appearing in either the actual or expected tree, so it just got missed.

I went and scanned through all the other node classes in this file, and I think this is the only such omission.

gnprice · 2025-07-17T05:16:34Z

lib/model/katex.dart

+          dom.Element(localName: 'span', className: 'vlist-r', nodes: [
+            dom.Element(localName: 'span', className: 'vlist', nodes: [
+              dom.Element(localName: 'span', className: '', nodes: []),
+            ]),
+          ]),
+        ]) {


Hmm, OK. A bit odd that there's that information there and it's not getting used; but maybe it's another example where it either used to get used, or gets used within KaTeX's internal processing.

Let's definitely mark that with a comment, though, because otherwise it looks wrong to a reader who's comparing this parsing code to the HTML. In fact we should do that for both of the .vlist cases — this one for the second .vlist-r's child, and below for the first/main .vlist-r's child.

gnprice · 2025-07-17T05:22:09Z

test/model/content_test.dart

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


Let's make these examples more compact in the same way I described today at #1559 (comment) . I think that will help making it easier to visually scan for the interesting parts.

gnprice · 2025-07-17T05:48:25Z

CI failure is unrelated, will investigate later. Logs:

2025-07-17T00:50:46.9768068Z Resolving dependencies...
2025-07-17T00:50:49.0495710Z The current Flutter SDK version is 0.0.0-unknown.
[…]

I think the problem is that the last tag is more than 1000 commits behind origin/main (or, equally, master):

$ flutter --version
Flutter 3.33.0-1.0.pre-1005 • channel main • https://github.com/flutter/flutter.git
Framework • revision 9c626d9f9a (3 hours ago) • 2025-07-16 23:03:27 -0400
Engine • hash de6ba3d359194bd9ddc0c917c2d1f4c531a63ed0 (revision 9c626d9f9a) (2 hours ago)
• 2025-07-17 03:03:27.000Z
Tools • Dart 3.10.0 (build 3.10.0-6.0.dev) • DevTools 2.48.0

The quick fix is to bump the --depth in ci.yml so that it fetches, say, 2000 commits instead of 1000.

Then we should probably also follow up to get the upstream Flutter release team to start putting tags in main again, so that version numbers in the main/master channel are accurate. (There's been a 3.34 and a 3.35 at this point, so the version number shouldn't be saying "3.33".) That's an issue whenever using the main (or master) channel, even when one makes a full clone without using --depth.

This was referenced Jul 9, 2025

KaTeX (1.75/n): Support horizontal offset and pre-req for vertical offsets #1452

Merged

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

Open

gnprice force-pushed the pr-tex-content-2-new branch from d8b0539 to de8b678 Compare July 10, 2025 02:54

gnprice reviewed Jul 10, 2025

View reviewed changes

rajveermalviya added 2 commits July 17, 2025 03:15

content: Handle vertical offset spans in KaTeX content

3b93cf5

Implement handling most common types of `vlist` spans.

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

8c6b215

rajveermalviya force-pushed the pr-tex-content-2-new branch from de8b678 to 8c6b215 Compare July 16, 2025 22:02

content: Implement debugDescribeChildren for KatexVlistRowNode

ca00158

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.

rajveermalviya requested a review from gnprice July 17, 2025 01:10

gnprice reviewed Jul 17, 2025

View reviewed changes

	debugHtmlNode: kDebugMode ? vlistT : null,
	debugHtmlNode: debugHtmlNode,

		final rows = <KatexVlistRowNode>[];

		for (final innerSpan in vlist.nodes) {

		static const mathBlockKatexVertical2 = ContentExample(
		'math block katex vertical 2',

		static const mathBlockKatexVertical3 = ContentExample(
		'math block katex vertical 3',

		List<DiagnosticsNode> debugDescribeChildren() {
		return [node.toDiagnosticsNode()];

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

Are you sure you want to change the base?

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

Conversation

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!

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!

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!

gnprice commented Jul 10, 2025

Uh oh!

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

Uh oh!

Uh oh!

rajveermalviya commented Jul 17, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice commented Jul 17, 2025

Uh oh!

Uh oh!

rajveermalviya commented Jul 17, 2025 •

edited

Loading