From b773c007b7270c1abd3f747027a5cc09dbf9b625 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 20:35:12 -0700 Subject: [PATCH 01/24] katex [nfc]: Show line numbers only on unknown hard-fails, not others This makes the output of the survey script more stable as our KaTeX parser gets refactored and otherwise edited. --- lib/model/katex.dart | 6 +++--- tools/content/unimplemented_katex_test.dart | 15 +++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 42b3277cee..d1655aad32 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -18,11 +18,11 @@ import 'settings.dart'; /// a specific node. class KatexParserHardFailReason { const KatexParserHardFailReason({ - required this.error, + required this.message, required this.stackTrace, }); - final String error; + final String? message; final StackTrace stackTrace; } @@ -132,7 +132,7 @@ MathParserResult? parseMath(dom.Element element, { required bool block }) { } on _KatexHtmlParseError catch (e, st) { assert(debugLog('$e\n$st')); hardFailReason = KatexParserHardFailReason( - error: e.message ?? 'unknown', + message: e.message, stackTrace: st); } diff --git a/tools/content/unimplemented_katex_test.dart b/tools/content/unimplemented_katex_test.dart index 80b0f482a7..aca9d82af4 100644 --- a/tools/content/unimplemented_katex_test.dart +++ b/tools/content/unimplemented_katex_test.dart @@ -59,8 +59,9 @@ void main() async { int failureCount = 0; if (hardFailReason != null) { - final firstLine = hardFailReason.stackTrace.toString().split('\n').first; - final reason = 'hard fail: ${hardFailReason.error} "$firstLine"'; + final message = hardFailReason.message + ?? 'unknown reason at ${_inmostFrame(hardFailReason.stackTrace)}'; + final reason = 'hard fail: $message'; (failedMessageIdsByReason[reason] ??= {}).add(messageId); (failedMathNodesByReason[reason] ??= {}).add(value); failureCount++; @@ -156,6 +157,16 @@ void main() async { }); } +/// The innermost frame of the given stack trace, +/// e.g. the line where an exception was thrown. +/// +/// Inevitably this is a bit heuristic, given the lack of any API guarantees +/// on the structure of [StackTrace]. +String _inmostFrame(StackTrace stackTrace) { + final firstLine = stackTrace.toString().split('\n').first; + return firstLine.replaceFirst(RegExp(r'^#\d+\s+'), ''); +} + const String _corpusDirPath = String.fromEnvironment('corpusDir'); const bool _verbose = int.fromEnvironment('verbose', defaultValue: 0) != 0; From 20294ed7c11a18e1f51afe60416fbf13d582a5cf Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 20:40:09 -0700 Subject: [PATCH 02/24] katex [nfc]: Add messages for remaining hard-fail cases seen in corpus --- lib/model/katex.dart | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index d1655aad32..c9b7cdfd58 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -386,7 +386,9 @@ class _KatexParser { // Currently, we expect `top` to only be inside a vlist, and // we handle that case separately above. - if (inlineStyles.topEm != null) throw _KatexHtmlParseError(); + if (inlineStyles.topEm != null) { + throw _KatexHtmlParseError('unsupported inline CSS property: top'); + } } // Aggregate the CSS styles that apply, in the same order as the CSS @@ -586,7 +588,7 @@ class _KatexParser { 'size4' => 'KaTeX_Size4', 'mult' => // TODO handle nested spans with `.delim-size{1,4}` class. - throw _KatexHtmlParseError(), + throw _KatexHtmlParseError('unimplemented CSS class pair: .delimsizing.mult'), _ => throw _KatexHtmlParseError(), }; @@ -705,7 +707,7 @@ class _KatexParser { unsupportedInlineCssProperties.add(property); _hasError = true; } else { - throw _KatexHtmlParseError(); + throw _KatexHtmlParseError('unexpected shape of inline CSS'); } } From 545ecfa75547b0505239e659031978aa1e34b08e Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 20:59:40 -0700 Subject: [PATCH 03/24] tools/content [nfc]: Tie-break on reason text when number of failures equal This helps make the output more stable from run to run, so that it's easier to spot changes (or confirm the absence of changes) when editing the code. --- tools/content/unimplemented_katex_test.dart | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/content/unimplemented_katex_test.dart b/tools/content/unimplemented_katex_test.dart index aca9d82af4..bb89052f24 100644 --- a/tools/content/unimplemented_katex_test.dart +++ b/tools/content/unimplemented_katex_test.dart @@ -103,9 +103,13 @@ void main() async { buf.writeln('There were $totalMathInlineNodes math inline nodes out of which $failedMathInlineNodes failed.'); buf.writeln(); - for (final MapEntry(key: reason, value: messageIds) in failedMessageIdsByReason.entries.sorted( - (a, b) => b.value.length.compareTo(a.value.length), - )) { + for (final MapEntry(key: reason, value: messageIds) + in failedMessageIdsByReason.entries.sorted((a, b) { + // Sort by number of failed messages descending, then by reason. + final r = - a.value.length.compareTo(b.value.length); + if (r != 0) return r; + return a.key.compareTo(b.key); + })) { final failedMathNodes = failedMathNodesByReason[reason]!.toList(); failedMathNodes.shuffle(); final oldestId = messageIds.reduce(min); From 1fd2f027217238dd6c98a1239b096731b4e4bd7a Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 18:50:09 -0700 Subject: [PATCH 04/24] katex [nfc]: Cut stray import of widgets library This was here only for a reference in a doc. In general we try to avoid imports of widgets from model code; it's an inversion of layers. --- lib/model/katex.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index c9b7cdfd58..0305543459 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -2,7 +2,6 @@ import 'package:collection/collection.dart'; import 'package:csslib/parser.dart' as css_parser; import 'package:csslib/visitor.dart' as css_visitor; import 'package:flutter/foundation.dart'; -import 'package:flutter/widgets.dart'; import 'package:html/dom.dart' as dom; import '../log.dart'; From a914be549655697c5f946801bf34805b8a3752f2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 18:53:49 -0700 Subject: [PATCH 05/24] binding [nfc]: Cut stray import of a widgets library This was here only for references in docs. Better to avoid the layer-inverting import. --- lib/model/binding.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/model/binding.dart b/lib/model/binding.dart index 4d1a0adaac..2ad21d939b 100644 --- a/lib/model/binding.dart +++ b/lib/model/binding.dart @@ -13,7 +13,6 @@ import 'package:wakelock_plus/wakelock_plus.dart' as wakelock_plus; import '../host/android_notifications.dart'; import '../host/notifications.dart' as notif_pigeon; import '../log.dart'; -import '../widgets/store.dart'; import 'store.dart'; export 'package:file_picker/file_picker.dart' show FilePickerResult, FileType, PlatformFile; From 5ab997b461035cb203dc027b95cfc3f8fc7cdbb3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 00:02:51 -0700 Subject: [PATCH 06/24] katex [nfc]: Fix a variable name to specify its units, namely em --- lib/model/katex.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 0305543459..ceb7082703 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -329,7 +329,7 @@ class _KatexParser { != const KatexSpanStyles()) { throw _KatexHtmlParseError(); } - final pstrutHeight = pstrutStyles.heightEm ?? 0; + final pstrutHeightEm = pstrutStyles.heightEm ?? 0; final KatexSpanNode innerSpanNode; @@ -356,7 +356,7 @@ class _KatexParser { } rows.add(KatexVlistRowNode( - verticalOffsetEm: topEm + pstrutHeight, + verticalOffsetEm: topEm + pstrutHeightEm, debugHtmlNode: kDebugMode ? innerSpan : null, node: innerSpanNode)); } else { From e0d80448978cab88637c533070a36ba9f113b4fa Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 13:32:24 -0700 Subject: [PATCH 07/24] katex: Require height on pstrut spans If this were missing, it's not clear to me that zero would be an appropriate default. In any case, in an empirical corpus, it's always present. So just require that. --- lib/model/katex.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index ceb7082703..c6e33f17e5 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -329,7 +329,8 @@ class _KatexParser { != const KatexSpanStyles()) { throw _KatexHtmlParseError(); } - final pstrutHeightEm = pstrutStyles.heightEm ?? 0; + final pstrutHeightEm = pstrutStyles.heightEm; + if (pstrutHeightEm == null) throw _KatexHtmlParseError(); final KatexSpanNode innerSpanNode; From 8909476b8a8afad3e7826923651ad280cedd6fef Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 17 Jul 2025 23:32:49 -0700 Subject: [PATCH 08/24] katex [nfc]: Separate _parseInlineStyles from constructing a KatexSpanStyles Also document the existing _parseSpanInlineStyles method, and describe our plan for eliminating most of its call sites. --- lib/model/katex.dart | 119 ++++++++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 52 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index c6e33f17e5..a49d081308 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -660,64 +660,79 @@ class _KatexParser { debugHtmlNode: debugHtmlNode); } + /// Parse the inline CSS styles from the given element, + /// and look for the styles we know how to interpret for a generic KaTeX span. + /// + /// TODO: This has a number of call sites that aren't acting on a generic + /// KaTeX span, but instead on spans in particular roles where we have + /// much more specific expectations on the inline styles. + /// For those, switch to [_parseInlineStyles] and inspect the styles directly. KatexSpanStyles? _parseSpanInlineStyles(dom.Element element) { + final declarations = _parseInlineStyles(element); + if (declarations == null) return null; + + double? heightEm; + double? verticalAlignEm; + double? topEm; + double? marginRightEm; + double? marginLeftEm; + + for (final declaration in declarations) { + if (declaration case css_visitor.Declaration( + :final property, + expression: css_visitor.Expressions( + expressions: [css_visitor.Expression() && final expression]), + )) { + switch (property) { + case 'height': + heightEm = _getEm(expression); + if (heightEm != null) continue; + + case 'vertical-align': + verticalAlignEm = _getEm(expression); + if (verticalAlignEm != null) continue; + + case 'top': + topEm = _getEm(expression); + if (topEm != null) continue; + + case 'margin-right': + marginRightEm = _getEm(expression); + if (marginRightEm != null) continue; + + case 'margin-left': + marginLeftEm = _getEm(expression); + if (marginLeftEm != null) continue; + } + + // TODO handle more CSS properties + assert(debugLog('KaTeX: Unsupported CSS expression:' + ' ${expression.toDebugString()}')); + unsupportedInlineCssProperties.add(property); + _hasError = true; + } else { + throw _KatexHtmlParseError('unexpected shape of inline CSS'); + } + } + + return KatexSpanStyles( + heightEm: heightEm, + topEm: topEm, + verticalAlignEm: verticalAlignEm, + marginRightEm: marginRightEm, + marginLeftEm: marginLeftEm, + ); + } + + /// Parse the inline CSS styles from the given element. + static Iterable? _parseInlineStyles(dom.Element element) { if (element.attributes case {'style': final styleStr}) { // `package:csslib` doesn't seem to have a way to parse inline styles: // https://github.com/dart-lang/tools/issues/1173 // So, work around that by wrapping it in a universal declaration. final stylesheet = css_parser.parse('*{$styleStr}'); - if (stylesheet.topLevels case [css_visitor.RuleSet() && final rule]) { - double? heightEm; - double? verticalAlignEm; - double? topEm; - double? marginRightEm; - double? marginLeftEm; - - for (final declaration in rule.declarationGroup.declarations) { - if (declaration case css_visitor.Declaration( - :final property, - expression: css_visitor.Expressions( - expressions: [css_visitor.Expression() && final expression]), - )) { - switch (property) { - case 'height': - heightEm = _getEm(expression); - if (heightEm != null) continue; - - case 'vertical-align': - verticalAlignEm = _getEm(expression); - if (verticalAlignEm != null) continue; - - case 'top': - topEm = _getEm(expression); - if (topEm != null) continue; - - case 'margin-right': - marginRightEm = _getEm(expression); - if (marginRightEm != null) continue; - - case 'margin-left': - marginLeftEm = _getEm(expression); - if (marginLeftEm != null) continue; - } - - // TODO handle more CSS properties - assert(debugLog('KaTeX: Unsupported CSS expression:' - ' ${expression.toDebugString()}')); - unsupportedInlineCssProperties.add(property); - _hasError = true; - } else { - throw _KatexHtmlParseError('unexpected shape of inline CSS'); - } - } - - return KatexSpanStyles( - heightEm: heightEm, - topEm: topEm, - verticalAlignEm: verticalAlignEm, - marginRightEm: marginRightEm, - marginLeftEm: marginLeftEm, - ); + if (stylesheet.topLevels case [css_visitor.RuleSet() && final ruleSet]) { + return ruleSet.declarationGroup.declarations; } else { throw _KatexHtmlParseError(); } From f935050dee7c00c417c1a27df1edec7707ca6b0b Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 17 Jul 2025 23:44:23 -0700 Subject: [PATCH 09/24] katex [nfc]: Push more parsing into _parseInlineStyles; return Map This has a small effect on the survey script's list of failure reasons: in the rare case that the "unexpected shape of inline CSS" error appears, it now fires before any of the CSS properties in the same inline style are processed. That can mean fewer entries added to unsupportedInlineCssProperties. This difference is only possible, though, on a KaTeX expression that is going to reach the same hard failure either way. So it has no effect on behavior seen by a user. --- lib/model/katex.dart | 83 ++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 37 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index a49d081308..bea2ed40c2 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -677,42 +677,37 @@ class _KatexParser { double? marginRightEm; double? marginLeftEm; - for (final declaration in declarations) { - if (declaration case css_visitor.Declaration( - :final property, - expression: css_visitor.Expressions( - expressions: [css_visitor.Expression() && final expression]), - )) { - switch (property) { - case 'height': - heightEm = _getEm(expression); - if (heightEm != null) continue; - - case 'vertical-align': - verticalAlignEm = _getEm(expression); - if (verticalAlignEm != null) continue; - - case 'top': - topEm = _getEm(expression); - if (topEm != null) continue; - - case 'margin-right': - marginRightEm = _getEm(expression); - if (marginRightEm != null) continue; - - case 'margin-left': - marginLeftEm = _getEm(expression); - if (marginLeftEm != null) continue; - } - - // TODO handle more CSS properties - assert(debugLog('KaTeX: Unsupported CSS expression:' - ' ${expression.toDebugString()}')); - unsupportedInlineCssProperties.add(property); - _hasError = true; - } else { - throw _KatexHtmlParseError('unexpected shape of inline CSS'); + for (final declaration in declarations.entries) { + final property = declaration.key; + final expression = declaration.value; + + switch (property) { + case 'height': + heightEm = _getEm(expression); + if (heightEm != null) continue; + + case 'vertical-align': + verticalAlignEm = _getEm(expression); + if (verticalAlignEm != null) continue; + + case 'top': + topEm = _getEm(expression); + if (topEm != null) continue; + + case 'margin-right': + marginRightEm = _getEm(expression); + if (marginRightEm != null) continue; + + case 'margin-left': + marginLeftEm = _getEm(expression); + if (marginLeftEm != null) continue; } + + // TODO handle more CSS properties + assert(debugLog('KaTeX: Unsupported CSS expression:' + ' ${expression.toDebugString()}')); + unsupportedInlineCssProperties.add(property); + _hasError = true; } return KatexSpanStyles( @@ -725,14 +720,28 @@ class _KatexParser { } /// Parse the inline CSS styles from the given element. - static Iterable? _parseInlineStyles(dom.Element element) { + static Map? _parseInlineStyles(dom.Element element) { if (element.attributes case {'style': final styleStr}) { // `package:csslib` doesn't seem to have a way to parse inline styles: // https://github.com/dart-lang/tools/issues/1173 // So, work around that by wrapping it in a universal declaration. final stylesheet = css_parser.parse('*{$styleStr}'); if (stylesheet.topLevels case [css_visitor.RuleSet() && final ruleSet]) { - return ruleSet.declarationGroup.declarations; + final result = {}; + for (final declaration in ruleSet.declarationGroup.declarations) { + if (declaration case css_visitor.Declaration( + :final property, + expression: css_visitor.Expressions( + expressions: [css_visitor.Expression() && final expression]), + )) { + result.update(property, ifAbsent: () => expression, + (_) => throw _KatexHtmlParseError( + 'duplicate inline CSS property: $property')); + } else { + throw _KatexHtmlParseError('unexpected shape of inline CSS'); + } + } + return result; } else { throw _KatexHtmlParseError(); } From eabcca18d74289f2ef510d517c1f7ac94b61704a Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 14:11:33 -0700 Subject: [PATCH 10/24] katex [nfc]: Factor out _takeStyleEm from _parseSpanInlineStyles Also make the error message for this case a bit more specific. --- lib/model/katex.dart | 67 +++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index bea2ed40c2..41c0749d48 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -671,55 +671,31 @@ class _KatexParser { final declarations = _parseInlineStyles(element); if (declarations == null) return null; - double? heightEm; - double? verticalAlignEm; - double? topEm; - double? marginRightEm; - double? marginLeftEm; + final result = KatexSpanStyles( + heightEm: _takeStyleEm(declarations, 'height'), + topEm: _takeStyleEm(declarations, 'top'), + verticalAlignEm: _takeStyleEm(declarations, 'vertical-align'), + marginRightEm: _takeStyleEm(declarations, 'margin-right'), + marginLeftEm: _takeStyleEm(declarations, 'margin-left'), + // TODO handle more CSS properties + ); for (final declaration in declarations.entries) { final property = declaration.key; final expression = declaration.value; - switch (property) { - case 'height': - heightEm = _getEm(expression); - if (heightEm != null) continue; - - case 'vertical-align': - verticalAlignEm = _getEm(expression); - if (verticalAlignEm != null) continue; - - case 'top': - topEm = _getEm(expression); - if (topEm != null) continue; - - case 'margin-right': - marginRightEm = _getEm(expression); - if (marginRightEm != null) continue; - - case 'margin-left': - marginLeftEm = _getEm(expression); - if (marginLeftEm != null) continue; - } - - // TODO handle more CSS properties assert(debugLog('KaTeX: Unsupported CSS expression:' ' ${expression.toDebugString()}')); unsupportedInlineCssProperties.add(property); _hasError = true; } - return KatexSpanStyles( - heightEm: heightEm, - topEm: topEm, - verticalAlignEm: verticalAlignEm, - marginRightEm: marginRightEm, - marginLeftEm: marginLeftEm, - ); + return result; } /// Parse the inline CSS styles from the given element. + /// + /// To interpret the resulting map, consider [_takeStyleEm]. static Map? _parseInlineStyles(dom.Element element) { if (element.attributes case {'style': final styleStr}) { // `package:csslib` doesn't seem to have a way to parse inline styles: @@ -749,12 +725,27 @@ class _KatexParser { return null; } - /// Returns the CSS `em` unit value if the given [expression] is actually an - /// `em` unit expression, else returns null. - double? _getEm(css_visitor.Expression expression) { + /// Remove the given property from the given style map, + /// and parse as a length in ems. + /// + /// If the property is present but is not a length in ems, + /// record an error and return null. + /// + /// If the property is absent, return null with no error. + /// + /// If the map is null, treat it as empty. + /// + /// To produce the map this method expects, see [_parseInlineStyles]. + double? _takeStyleEm(Map? styles, String property) { + final expression = styles?.remove(property); + if (expression == null) return null; if (expression is css_visitor.EmTerm && expression.value is num) { return (expression.value as num).toDouble(); } + assert(debugLog('KaTeX: Unsupported value for CSS property $property,' + ' expected a length in em: ${expression.toDebugString()}')); + unsupportedInlineCssProperties.add(property); + _hasError = true; return null; } } From 95ae2937fdb3f3a07e5d9bf6ab88cd5706f30b28 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 14:16:12 -0700 Subject: [PATCH 11/24] katex: Fix a misleading log line: unexpected CSS property, not value Until the previous commit, this bit of code was handling both the case where the value was unexpected (for which this message was accurate) and the case where the property itself was unexpected (for which it wasn't). Now the first case is handled elsewhere, so fix the remaining case. --- lib/model/katex.dart | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 41c0749d48..4f9aca6c06 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -680,12 +680,8 @@ class _KatexParser { // TODO handle more CSS properties ); - for (final declaration in declarations.entries) { - final property = declaration.key; - final expression = declaration.value; - - assert(debugLog('KaTeX: Unsupported CSS expression:' - ' ${expression.toDebugString()}')); + for (final property in declarations.keys) { + assert(debugLog('KaTeX: Unexpected inline CSS property: $property')); unsupportedInlineCssProperties.add(property); _hasError = true; } From b9c7bc4e87842fae565106ed67f0cf2e9dea6323 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 17 Jul 2025 23:53:45 -0700 Subject: [PATCH 12/24] katex [nfc]: Skip building whole KatexSpanStyles for struts' two properties We know at this spot that there are just two specific CSS properties we expect to see, and we'll end up handling them directly rather than through a KatexSpanStyles object. So parse them directly, rather than build a whole KatexSpanStyles object (and then another one with `.filter()`). --- lib/model/katex.dart | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 4f9aca6c06..a1bb293c37 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -230,18 +230,12 @@ class _KatexParser { if (element.className == 'strut') { if (element.nodes.isNotEmpty) throw _KatexHtmlParseError(); - final styles = _parseSpanInlineStyles(element); + final styles = _parseInlineStyles(element); if (styles == null) throw _KatexHtmlParseError(); - - final heightEm = styles.heightEm; + final heightEm = _takeStyleEm(styles, 'height'); if (heightEm == null) throw _KatexHtmlParseError(); - final verticalAlignEm = styles.verticalAlignEm; - - // Ensure only `height` and `vertical-align` inline styles are present. - if (styles.filter(heightEm: false, verticalAlignEm: false) - != const KatexSpanStyles()) { - throw _KatexHtmlParseError(); - } + final verticalAlignEm = _takeStyleEm(styles, 'vertical-align'); + if (styles.isNotEmpty) throw _KatexHtmlParseError(); return KatexStrutNode( heightEm: heightEm, From 8ac8a439b6d0bb35dd3767e12c91e52d6d437827 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 00:11:27 -0700 Subject: [PATCH 13/24] katex [nfc]: Cut vertical-align from generic style properties All the remaining call sites of _parseSpanInlineStyles would throw anyway if this property were actually found. We only expect it in a specific context, namely a strut. --- lib/model/katex.dart | 17 ++++------------- lib/widgets/content.dart | 4 ---- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index a1bb293c37..560ded094b 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -312,7 +312,6 @@ class _KatexParser { var styles = _parseSpanInlineStyles(innerSpan); if (styles == null) throw _KatexHtmlParseError(); - if (styles.verticalAlignEm != null) throw _KatexHtmlParseError(); final topEm = styles.topEm ?? 0; styles = styles.filter(topEm: false); @@ -374,10 +373,6 @@ class _KatexParser { final inlineStyles = _parseSpanInlineStyles(element); if (inlineStyles != null) { - // We expect `vertical-align` inline style to be only present on a - // `strut` span, for which we emit `KatexStrutNode` separately. - if (inlineStyles.verticalAlignEm != null) throw _KatexHtmlParseError(); - // Currently, we expect `top` to only be inside a vlist, and // we handle that case separately above. if (inlineStyles.topEm != null) { @@ -668,7 +663,6 @@ class _KatexParser { final result = KatexSpanStyles( heightEm: _takeStyleEm(declarations, 'height'), topEm: _takeStyleEm(declarations, 'top'), - verticalAlignEm: _takeStyleEm(declarations, 'vertical-align'), marginRightEm: _takeStyleEm(declarations, 'margin-right'), marginLeftEm: _takeStyleEm(declarations, 'margin-left'), // TODO handle more CSS properties @@ -758,7 +752,10 @@ enum KatexSpanTextAlign { @immutable class KatexSpanStyles { final double? heightEm; - final double? verticalAlignEm; + + // We expect `vertical-align` inline style to be only present on a + // `strut` span, for which we emit `KatexStrutNode` separately. + // final double? verticalAlignEm; final double? topEm; @@ -773,7 +770,6 @@ class KatexSpanStyles { const KatexSpanStyles({ this.heightEm, - this.verticalAlignEm, this.topEm, this.marginRightEm, this.marginLeftEm, @@ -788,7 +784,6 @@ class KatexSpanStyles { int get hashCode => Object.hash( 'KatexSpanStyles', heightEm, - verticalAlignEm, topEm, marginRightEm, marginLeftEm, @@ -803,7 +798,6 @@ class KatexSpanStyles { bool operator ==(Object other) { return other is KatexSpanStyles && other.heightEm == heightEm && - other.verticalAlignEm == verticalAlignEm && other.topEm == topEm && other.marginRightEm == marginRightEm && other.marginLeftEm == marginLeftEm && @@ -818,7 +812,6 @@ class KatexSpanStyles { String toString() { final args = []; if (heightEm != null) args.add('heightEm: $heightEm'); - if (verticalAlignEm != null) args.add('verticalAlignEm: $verticalAlignEm'); if (topEm != null) args.add('topEm: $topEm'); if (marginRightEm != null) args.add('marginRightEm: $marginRightEm'); if (marginLeftEm != null) args.add('marginLeftEm: $marginLeftEm'); @@ -840,7 +833,6 @@ class KatexSpanStyles { KatexSpanStyles merge(KatexSpanStyles other) { return KatexSpanStyles( heightEm: other.heightEm ?? heightEm, - verticalAlignEm: other.verticalAlignEm ?? verticalAlignEm, topEm: other.topEm ?? topEm, marginRightEm: other.marginRightEm ?? marginRightEm, marginLeftEm: other.marginLeftEm ?? marginLeftEm, @@ -866,7 +858,6 @@ class KatexSpanStyles { }) { return KatexSpanStyles( heightEm: heightEm ? this.heightEm : null, - verticalAlignEm: verticalAlignEm ? this.verticalAlignEm : null, topEm: topEm ? this.topEm : null, marginRightEm: marginRightEm ? this.marginRightEm : null, marginLeftEm: marginLeftEm ? this.marginLeftEm : null, diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index b4ef707355..2263b74f8b 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -922,10 +922,6 @@ class _KatexSpan extends StatelessWidget { } final styles = node.styles; - // We expect vertical-align to be only present with the - // `strut` span, for which parser explicitly emits `KatexStrutNode`. - // So, this should always be null for non `strut` spans. - assert(styles.verticalAlignEm == null); // Currently, we expect `top` to be only present with the // vlist inner row span, and parser handles that explicitly. From d44e7303c2d8016fe9dc2e7c0915b5f5b2612cc2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 00:00:55 -0700 Subject: [PATCH 14/24] katex [nfc]: Check "only has height" directly, without making KatexSpanStyles This lets us skip allocating these objects (two in each case -- the second one comes from `.filter()`). We also get to skip parsing the value of `height`, since we don't intend to use it. --- lib/model/katex.dart | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 560ded094b..024d7c9b4f 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -265,9 +265,8 @@ class _KatexParser { // a "height" inline style which we ignore, because it doesn't seem // to have any effect in rendering on the web. // But also make sure there aren't any other inline styles present. - final vlistStyles = _parseSpanInlineStyles(vlist); - if (vlistStyles != null - && vlistStyles.filter(heightEm: false) != const KatexSpanStyles()) { + final vlistStyles = _parseInlineStyles(vlist); + if (vlistStyles != null && vlistStyles.keys.any((p) => p != 'height')) { throw _KatexHtmlParseError(); } } else { @@ -288,9 +287,8 @@ class _KatexParser { // because it doesn't seem to have any effect in rendering on // the web. // But also make sure there aren't any other inline styles present. - final vlistStyles = _parseSpanInlineStyles(vlist); - if (vlistStyles != null - && vlistStyles.filter(heightEm: false) != const KatexSpanStyles()) { + final vlistStyles = _parseInlineStyles(vlist); + if (vlistStyles != null && vlistStyles.keys.any((p) => p != 'height')) { throw _KatexHtmlParseError(); } From b58379308ffbf7d19f81e92b723ad80ad7b50ff2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 00:04:26 -0700 Subject: [PATCH 15/24] katex [nfc]: Get pstrut height directly, without making KatexSpanStyles --- lib/model/katex.dart | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 024d7c9b4f..64bdb4c835 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -314,14 +314,11 @@ class _KatexParser { styles = styles.filter(topEm: false); - final pstrutStyles = _parseSpanInlineStyles(pstrutSpan); + final pstrutStyles = _parseInlineStyles(pstrutSpan); if (pstrutStyles == null) throw _KatexHtmlParseError(); - if (pstrutStyles.filter(heightEm: false) - != const KatexSpanStyles()) { - throw _KatexHtmlParseError(); - } - final pstrutHeightEm = pstrutStyles.heightEm; + final pstrutHeightEm = _takeStyleEm(pstrutStyles, 'height'); if (pstrutHeightEm == null) throw _KatexHtmlParseError(); + if (pstrutStyles.isNotEmpty) throw _KatexHtmlParseError(); final KatexSpanNode innerSpanNode; From c14bfe61019874063d9e421fd418789b1dc31c4d Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 16:05:40 -0700 Subject: [PATCH 16/24] katex [nfc]: Directly handle expected inline styles on vlist child There's only a handful of specific properties we expect to see on this type of span; so handle those explicitly. In fact, making this list explicit brings to light that there's one property here which doesn't actually appear on KaTeX's vlist children: height. We'll cut that in a separate non-NFC commit. This will also open up ways to simplify the interaction between this and the margin-handling logic below. --- lib/model/katex.dart | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 64bdb4c835..5ef810d092 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -308,11 +308,15 @@ class _KatexParser { 'vlist inner span: ${innerSpan.className}'); } - var styles = _parseSpanInlineStyles(innerSpan); - if (styles == null) throw _KatexHtmlParseError(); - final topEm = styles.topEm ?? 0; - - styles = styles.filter(topEm: false); + final inlineStyles = _parseInlineStyles(innerSpan); + if (inlineStyles == null) throw _KatexHtmlParseError(); + final styles = KatexSpanStyles( + heightEm: _takeStyleEm(inlineStyles, 'height'), + marginLeftEm: _takeStyleEm(inlineStyles, 'margin-left'), + marginRightEm: _takeStyleEm(inlineStyles, 'margin-right'), + ); + final topEm = _takeStyleEm(inlineStyles, 'top'); + if (inlineStyles.isNotEmpty) throw _KatexHtmlParseError(); final pstrutStyles = _parseInlineStyles(pstrutSpan); if (pstrutStyles == null) throw _KatexHtmlParseError(); @@ -345,7 +349,7 @@ class _KatexParser { } rows.add(KatexVlistRowNode( - verticalOffsetEm: topEm + pstrutHeightEm, + verticalOffsetEm: (topEm ?? 0) + pstrutHeightEm, debugHtmlNode: kDebugMode ? innerSpan : null, node: innerSpanNode)); } else { From 73ae26991c86967b29bf465953b1e3ea74f36701 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 17:51:10 -0700 Subject: [PATCH 17/24] katex: Don't expect height on vlist child spans These spans are highly structured; the only properties that go into their inline styles are top, margin-left, and margin-right. --- lib/model/katex.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 5ef810d092..6717b52da6 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -311,7 +311,6 @@ class _KatexParser { final inlineStyles = _parseInlineStyles(innerSpan); if (inlineStyles == null) throw _KatexHtmlParseError(); final styles = KatexSpanStyles( - heightEm: _takeStyleEm(inlineStyles, 'height'), marginLeftEm: _takeStyleEm(inlineStyles, 'margin-left'), marginRightEm: _takeStyleEm(inlineStyles, 'margin-right'), ); From fe383b839fc0c8174f98f74c8cca414aa93ed7a0 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 00:23:34 -0700 Subject: [PATCH 18/24] katex [nfc]: Consolidate logic for computing overall styles of KatexSpanNode This just pulls these three pieces of closely-related logic next to each other. That will make it easier to refactor them further. This causes one change in the survey script's list of failure reasons: when the `delimcenter` class occurs with an inline `top` property, we now record the unsupported class before reaching the hard fail for the unsupported property. This has no user-visible effect, though, because it can only happen when the expression is going to reach that hard failure either way. --- lib/model/katex.dart | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 6717b52da6..29e5c38505 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -369,15 +369,6 @@ class _KatexParser { } } - final inlineStyles = _parseSpanInlineStyles(element); - if (inlineStyles != null) { - // Currently, we expect `top` to only be inside a vlist, and - // we handle that case separately above. - if (inlineStyles.topEm != null) { - throw _KatexHtmlParseError('unsupported inline CSS property: top'); - } - } - // Aggregate the CSS styles that apply, in the same order as the CSS // classes specified for this span, mimicking the behaviour on web. // @@ -621,13 +612,23 @@ class _KatexParser { _hasError = true; } } - final styles = KatexSpanStyles( + final classStyles = KatexSpanStyles( fontFamily: fontFamily, fontSizeEm: fontSizeEm, fontWeight: fontWeight, fontStyle: fontStyle, textAlign: textAlign, ); + final inlineStyles = _parseSpanInlineStyles(element); + if (inlineStyles != null) { + // Currently, we expect `top` to only be inside a vlist, and + // we handle that case separately above. + if (inlineStyles.topEm != null) { + throw _KatexHtmlParseError('unsupported inline CSS property: top'); + } + } + final styles = inlineStyles == null ? classStyles + : classStyles.merge(inlineStyles); String? text; List? spans; @@ -639,9 +640,7 @@ class _KatexParser { if (text == null && spans == null) throw _KatexHtmlParseError(); return KatexSpanNode( - styles: inlineStyles != null - ? styles.merge(inlineStyles) - : styles, + styles: styles, text: text, nodes: spans, debugHtmlNode: debugHtmlNode); From 0f634c9d90911a95409d5576b9f0791fe066c5c6 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 00:30:01 -0700 Subject: [PATCH 19/24] katex [nfc]: Inline remaining/main use of _parseSpanInlineStyles And inline the effect of the `merge` method, eliminating that method too. This way we get to construct just one KatexSpanStyles object, rather than constructing three of them when inline styles are present. --- lib/model/katex.dart | 76 +++++++++++--------------------------------- 1 file changed, 18 insertions(+), 58 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 29e5c38505..255e1eaec4 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -612,23 +612,32 @@ class _KatexParser { _hasError = true; } } - final classStyles = KatexSpanStyles( + + final inlineStyles = _parseInlineStyles(element); + final styles = KatexSpanStyles( fontFamily: fontFamily, fontSizeEm: fontSizeEm, fontWeight: fontWeight, fontStyle: fontStyle, textAlign: textAlign, + heightEm: _takeStyleEm(inlineStyles, 'height'), + topEm: _takeStyleEm(inlineStyles, 'top'), + marginLeftEm: _takeStyleEm(inlineStyles, 'margin-left'), + marginRightEm: _takeStyleEm(inlineStyles, 'margin-right'), + // TODO handle more CSS properties ); - final inlineStyles = _parseSpanInlineStyles(element); - if (inlineStyles != null) { - // Currently, we expect `top` to only be inside a vlist, and - // we handle that case separately above. - if (inlineStyles.topEm != null) { - throw _KatexHtmlParseError('unsupported inline CSS property: top'); + if (inlineStyles != null && inlineStyles.isNotEmpty) { + for (final property in inlineStyles.keys) { + assert(debugLog('KaTeX: Unexpected inline CSS property: $property')); + unsupportedInlineCssProperties.add(property); + _hasError = true; } } - final styles = inlineStyles == null ? classStyles - : classStyles.merge(inlineStyles); + // Currently, we expect `top` to only be inside a vlist, and + // we handle that case separately above. + if (styles.topEm != null) { + throw _KatexHtmlParseError('unsupported inline CSS property: top'); + } String? text; List? spans; @@ -646,34 +655,6 @@ class _KatexParser { debugHtmlNode: debugHtmlNode); } - /// Parse the inline CSS styles from the given element, - /// and look for the styles we know how to interpret for a generic KaTeX span. - /// - /// TODO: This has a number of call sites that aren't acting on a generic - /// KaTeX span, but instead on spans in particular roles where we have - /// much more specific expectations on the inline styles. - /// For those, switch to [_parseInlineStyles] and inspect the styles directly. - KatexSpanStyles? _parseSpanInlineStyles(dom.Element element) { - final declarations = _parseInlineStyles(element); - if (declarations == null) return null; - - final result = KatexSpanStyles( - heightEm: _takeStyleEm(declarations, 'height'), - topEm: _takeStyleEm(declarations, 'top'), - marginRightEm: _takeStyleEm(declarations, 'margin-right'), - marginLeftEm: _takeStyleEm(declarations, 'margin-left'), - // TODO handle more CSS properties - ); - - for (final property in declarations.keys) { - assert(debugLog('KaTeX: Unexpected inline CSS property: $property')); - unsupportedInlineCssProperties.add(property); - _hasError = true; - } - - return result; - } - /// Parse the inline CSS styles from the given element. /// /// To interpret the resulting map, consider [_takeStyleEm]. @@ -820,27 +801,6 @@ class KatexSpanStyles { return '${objectRuntimeType(this, 'KatexSpanStyles')}(${args.join(', ')})'; } - /// Creates a new [KatexSpanStyles] with current and [other]'s styles merged. - /// - /// The styles in [other] take precedence and any missing styles in [other] - /// are filled in with current styles, if present. - /// - /// This similar to the behaviour of [TextStyle.merge], if the given style - /// had `inherit` set to true. - KatexSpanStyles merge(KatexSpanStyles other) { - return KatexSpanStyles( - heightEm: other.heightEm ?? heightEm, - topEm: other.topEm ?? topEm, - marginRightEm: other.marginRightEm ?? marginRightEm, - marginLeftEm: other.marginLeftEm ?? marginLeftEm, - fontFamily: other.fontFamily ?? fontFamily, - fontSizeEm: other.fontSizeEm ?? fontSizeEm, - fontStyle: other.fontStyle ?? fontStyle, - fontWeight: other.fontWeight ?? fontWeight, - textAlign: other.textAlign ?? textAlign, - ); - } - KatexSpanStyles filter({ bool heightEm = true, bool verticalAlignEm = true, From f54d168a2e724b2ed292f88c1e5651ae96b86b0f Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 12:12:42 -0700 Subject: [PATCH 20/24] katex [nfc]: Construct vlist child's styles directly, without filter --- lib/model/katex.dart | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 255e1eaec4..e871943e91 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -310,9 +310,13 @@ class _KatexParser { final inlineStyles = _parseInlineStyles(innerSpan); if (inlineStyles == null) throw _KatexHtmlParseError(); + final marginLeftEm = _takeStyleEm(inlineStyles, 'margin-left'); + final marginLeftIsNegative = marginLeftEm?.isNegative ?? false; + final marginRightEm = _takeStyleEm(inlineStyles, 'margin-right'); + if (marginRightEm?.isNegative ?? false) throw _KatexHtmlParseError(); final styles = KatexSpanStyles( - marginLeftEm: _takeStyleEm(inlineStyles, 'margin-left'), - marginRightEm: _takeStyleEm(inlineStyles, 'margin-right'), + marginLeftEm: marginLeftIsNegative ? null : marginLeftEm, + marginRightEm: marginRightEm, ); final topEm = _takeStyleEm(inlineStyles, 'top'); if (inlineStyles.isNotEmpty) throw _KatexHtmlParseError(); @@ -325,19 +329,14 @@ class _KatexParser { final KatexSpanNode innerSpanNode; - final marginRightEm = styles.marginRightEm; - final marginLeftEm = styles.marginLeftEm; - if (marginRightEm != null && marginRightEm.isNegative) { - throw _KatexHtmlParseError(); - } - if (marginLeftEm != null && marginLeftEm.isNegative) { + if (marginLeftIsNegative) { innerSpanNode = KatexSpanNode( styles: KatexSpanStyles(), text: null, nodes: [KatexNegativeMarginNode( - leftOffsetEm: marginLeftEm, + leftOffsetEm: marginLeftEm!, nodes: [KatexSpanNode( - styles: styles.filter(marginLeftEm: false), + styles: styles, text: null, nodes: _parseChildSpans(otherSpans))])]); } else { From ebce0a4b0fe95fa1cb414721ccc91cdca097beb3 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 15:35:38 -0700 Subject: [PATCH 21/24] katex [nfc]: Dedupe logic for vlist child between margin/no-margin cases --- lib/model/katex.dart | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index e871943e91..b58abe2a97 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -327,29 +327,24 @@ class _KatexParser { if (pstrutHeightEm == null) throw _KatexHtmlParseError(); if (pstrutStyles.isNotEmpty) throw _KatexHtmlParseError(); - final KatexSpanNode innerSpanNode; + KatexSpanNode child = KatexSpanNode( + styles: styles, + text: null, + nodes: _parseChildSpans(otherSpans)); if (marginLeftIsNegative) { - innerSpanNode = KatexSpanNode( + child = KatexSpanNode( styles: KatexSpanStyles(), text: null, nodes: [KatexNegativeMarginNode( leftOffsetEm: marginLeftEm!, - nodes: [KatexSpanNode( - styles: styles, - text: null, - nodes: _parseChildSpans(otherSpans))])]); - } else { - innerSpanNode = KatexSpanNode( - styles: styles, - text: null, - nodes: _parseChildSpans(otherSpans)); + nodes: [child])]); } rows.add(KatexVlistRowNode( verticalOffsetEm: (topEm ?? 0) + pstrutHeightEm, debugHtmlNode: kDebugMode ? innerSpan : null, - node: innerSpanNode)); + node: child)); } else { throw _KatexHtmlParseError(); } From 2a604bacd162ab3246045ff3aad64b7a132cd483 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 17:53:47 -0700 Subject: [PATCH 22/24] katex [nfc]: Note that heightEm might turn out not to be needed --- lib/model/katex.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index b58abe2a97..98fc69a861 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -723,6 +723,10 @@ enum KatexSpanTextAlign { @immutable class KatexSpanStyles { + // TODO(#1674) does height actually appear on generic spans? + // In a corpus, the only occurrences that we don't already handle separately + // (i.e. occurrences other than on struts, vlists, etc) seem to be within + // accents; so after #1674 we might be handling those separately too. final double? heightEm; // We expect `vertical-align` inline style to be only present on a From 39fdded9b75c39b3103b7fd92eae6c8fc77391ce Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 21:36:52 -0700 Subject: [PATCH 23/24] katex [nfc]: Simplify extracting from attrs map in _parseInlineStyles This map pattern syntax looks an awful lot like it's saying that no other keys should be present -- after all, that's what the corresponding syntax in a list pattern would mean. So I initially read this to mean that this code would ignore the inline styles if any other attribute was present on the element; which wouldn't be desirable logic. In fact it's just saying that this one key should be present and match the given pattern. But there are simpler ways to say that; so use one. --- lib/model/katex.dart | 46 ++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index 98fc69a861..bf6fdad37e 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -653,32 +653,32 @@ class _KatexParser { /// /// To interpret the resulting map, consider [_takeStyleEm]. static Map? _parseInlineStyles(dom.Element element) { - if (element.attributes case {'style': final styleStr}) { - // `package:csslib` doesn't seem to have a way to parse inline styles: - // https://github.com/dart-lang/tools/issues/1173 - // So, work around that by wrapping it in a universal declaration. - final stylesheet = css_parser.parse('*{$styleStr}'); - if (stylesheet.topLevels case [css_visitor.RuleSet() && final ruleSet]) { - final result = {}; - for (final declaration in ruleSet.declarationGroup.declarations) { - if (declaration case css_visitor.Declaration( - :final property, - expression: css_visitor.Expressions( - expressions: [css_visitor.Expression() && final expression]), - )) { - result.update(property, ifAbsent: () => expression, - (_) => throw _KatexHtmlParseError( - 'duplicate inline CSS property: $property')); - } else { - throw _KatexHtmlParseError('unexpected shape of inline CSS'); - } + final styleStr = element.attributes['style']; + if (styleStr == null) return null; + + // `package:csslib` doesn't seem to have a way to parse inline styles: + // https://github.com/dart-lang/tools/issues/1173 + // So, work around that by wrapping it in a universal declaration. + final stylesheet = css_parser.parse('*{$styleStr}'); + if (stylesheet.topLevels case [css_visitor.RuleSet() && final ruleSet]) { + final result = {}; + for (final declaration in ruleSet.declarationGroup.declarations) { + if (declaration case css_visitor.Declaration( + :final property, + expression: css_visitor.Expressions( + expressions: [css_visitor.Expression() && final expression]), + )) { + result.update(property, ifAbsent: () => expression, + (_) => throw _KatexHtmlParseError( + 'duplicate inline CSS property: $property')); + } else { + throw _KatexHtmlParseError('unexpected shape of inline CSS'); } - return result; - } else { - throw _KatexHtmlParseError(); } + return result; + } else { + throw _KatexHtmlParseError(); } - return null; } /// Remove the given property from the given style map, From 83a0b6fa15a02196584b870aa186f40460c07c65 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 18 Jul 2025 22:18:39 -0700 Subject: [PATCH 24/24] katex [nfc]: Split out _parseStrut, _parseVlist, _parseGenericSpan Each of these swathes of logic has no interaction with the others. Splitting them into their own methods makes that structure easy for the reader to see. --- lib/model/katex.dart | 264 +++++++++++++++++++++++-------------------- 1 file changed, 141 insertions(+), 123 deletions(-) diff --git a/lib/model/katex.dart b/lib/model/katex.dart index bf6fdad37e..6eab8473c7 100644 --- a/lib/model/katex.dart +++ b/lib/model/katex.dart @@ -219,149 +219,167 @@ class _KatexParser { return resultSpans; } - static final _resetSizeClassRegExp = RegExp(r'^reset-size(\d\d?)$'); - static final _sizeClassRegExp = RegExp(r'^size(\d\d?)$'); - KatexNode _parseSpan(dom.Element element) { + assert(element.localName == 'span'); // TODO maybe check if the sequence of ancestors matter for spans. - final debugHtmlNode = kDebugMode ? element : null; - if (element.className == 'strut') { - if (element.nodes.isNotEmpty) throw _KatexHtmlParseError(); - - final styles = _parseInlineStyles(element); - if (styles == null) throw _KatexHtmlParseError(); - final heightEm = _takeStyleEm(styles, 'height'); - if (heightEm == null) throw _KatexHtmlParseError(); - final verticalAlignEm = _takeStyleEm(styles, 'vertical-align'); - if (styles.isNotEmpty) throw _KatexHtmlParseError(); - - return KatexStrutNode( - heightEm: heightEm, - verticalAlignEm: verticalAlignEm, - debugHtmlNode: debugHtmlNode); + return _parseStrut(element); } if (element.className == 'vlist-t' || element.className == 'vlist-t vlist-t2') { - final vlistT = element; - if (vlistT.nodes.isEmpty) throw _KatexHtmlParseError(); - if (vlistT.attributes.containsKey('style')) throw _KatexHtmlParseError(); - - final hasTwoVlistR = vlistT.className == 'vlist-t vlist-t2'; - if (!hasTwoVlistR && vlistT.nodes.length != 1) throw _KatexHtmlParseError(); - - if (hasTwoVlistR) { - if (vlistT.nodes case [ - _, - dom.Element(localName: 'span', className: 'vlist-r', nodes: [ - dom.Element(localName: 'span', className: 'vlist', nodes: [ - dom.Element(localName: 'span', className: '', nodes: []), - ]) && final vlist, - ]), - ]) { - // In the generated HTML the .vlist in second .vlist-r span will have - // a "height" inline style which we ignore, because it doesn't seem - // to have any effect in rendering on the web. - // But also make sure there aren't any other inline styles present. - final vlistStyles = _parseInlineStyles(vlist); - if (vlistStyles != null && vlistStyles.keys.any((p) => p != 'height')) { - throw _KatexHtmlParseError(); - } - } else { + return _parseVlist(element); + } + + return _parseGenericSpan(element); + } + + KatexNode _parseStrut(dom.Element element) { + assert(element.localName == 'span'); + assert(element.className == 'strut'); + if (element.nodes.isNotEmpty) throw _KatexHtmlParseError(); + + final styles = _parseInlineStyles(element); + if (styles == null) throw _KatexHtmlParseError(); + final heightEm = _takeStyleEm(styles, 'height'); + if (heightEm == null) throw _KatexHtmlParseError(); + final verticalAlignEm = _takeStyleEm(styles, 'vertical-align'); + if (styles.isNotEmpty) throw _KatexHtmlParseError(); + + return KatexStrutNode( + heightEm: heightEm, + verticalAlignEm: verticalAlignEm, + debugHtmlNode: kDebugMode ? element : null); + } + + KatexNode _parseVlist(dom.Element element) { + assert(element.localName == 'span'); + assert(element.className == 'vlist-t' + || element.className == 'vlist-t vlist-t2'); + final vlistT = element; + if (vlistT.nodes.isEmpty) throw _KatexHtmlParseError(); + if (vlistT.attributes.containsKey('style')) throw _KatexHtmlParseError(); + + final hasTwoVlistR = vlistT.className == 'vlist-t vlist-t2'; + if (!hasTwoVlistR && vlistT.nodes.length != 1) throw _KatexHtmlParseError(); + + if (hasTwoVlistR) { + if (vlistT.nodes case [ + _, + dom.Element(localName: 'span', className: 'vlist-r', nodes: [ + dom.Element(localName: 'span', className: 'vlist', nodes: [ + dom.Element(localName: 'span', className: '', nodes: []), + ]) && final vlist, + ]), + ]) { + // In the generated HTML the .vlist in second .vlist-r span will have + // a "height" inline style which we ignore, because it doesn't seem + // to have any effect in rendering on the web. + // But also make sure there aren't any other inline styles present. + final vlistStyles = _parseInlineStyles(vlist); + if (vlistStyles != null && vlistStyles.keys.any((p) => p != 'height')) { throw _KatexHtmlParseError(); } + } else { + throw _KatexHtmlParseError(); } + } - if (vlistT.nodes.first - case dom.Element(localName: 'span', className: 'vlist-r') && - final vlistR) { - if (vlistR.attributes.containsKey('style')) throw _KatexHtmlParseError(); - - if (vlistR.nodes.first - case dom.Element(localName: 'span', className: 'vlist') && - final vlist) { - // Same as above for the second .vlist-r span, .vlist span in first - // .vlist-r span will have "height" inline style which we ignore, - // because it doesn't seem to have any effect in rendering on - // the web. - // But also make sure there aren't any other inline styles present. - final vlistStyles = _parseInlineStyles(vlist); - if (vlistStyles != null && vlistStyles.keys.any((p) => p != 'height')) { - throw _KatexHtmlParseError(); - } + if (vlistT.nodes.first + case dom.Element(localName: 'span', className: 'vlist-r') && + final vlistR) { + if (vlistR.attributes.containsKey('style')) throw _KatexHtmlParseError(); + + if (vlistR.nodes.first + case dom.Element(localName: 'span', className: 'vlist') && + final vlist) { + // Same as above for the second .vlist-r span, .vlist span in first + // .vlist-r span will have "height" inline style which we ignore, + // because it doesn't seem to have any effect in rendering on + // the web. + // But also make sure there aren't any other inline styles present. + final vlistStyles = _parseInlineStyles(vlist); + if (vlistStyles != null && vlistStyles.keys.any((p) => p != 'height')) { + throw _KatexHtmlParseError(); + } + + final rows = []; + + for (final innerSpan in vlist.nodes) { + if (innerSpan case dom.Element( + localName: 'span', + nodes: [ + dom.Element(localName: 'span', className: 'pstrut') && + final pstrutSpan, + ...final otherSpans, + ], + )) { + if (innerSpan.className != '') { + throw _KatexHtmlParseError('unexpected CSS class for ' + 'vlist inner span: ${innerSpan.className}'); + } - final rows = []; - - for (final innerSpan in vlist.nodes) { - if (innerSpan case dom.Element( - localName: 'span', - nodes: [ - dom.Element(localName: 'span', className: 'pstrut') && - final pstrutSpan, - ...final otherSpans, - ], - )) { - if (innerSpan.className != '') { - throw _KatexHtmlParseError('unexpected CSS class for ' - 'vlist inner span: ${innerSpan.className}'); - } - - final inlineStyles = _parseInlineStyles(innerSpan); - if (inlineStyles == null) throw _KatexHtmlParseError(); - final marginLeftEm = _takeStyleEm(inlineStyles, 'margin-left'); - final marginLeftIsNegative = marginLeftEm?.isNegative ?? false; - final marginRightEm = _takeStyleEm(inlineStyles, 'margin-right'); - if (marginRightEm?.isNegative ?? false) throw _KatexHtmlParseError(); - final styles = KatexSpanStyles( - marginLeftEm: marginLeftIsNegative ? null : marginLeftEm, - marginRightEm: marginRightEm, - ); - final topEm = _takeStyleEm(inlineStyles, 'top'); - if (inlineStyles.isNotEmpty) throw _KatexHtmlParseError(); - - final pstrutStyles = _parseInlineStyles(pstrutSpan); - if (pstrutStyles == null) throw _KatexHtmlParseError(); - final pstrutHeightEm = _takeStyleEm(pstrutStyles, 'height'); - if (pstrutHeightEm == null) throw _KatexHtmlParseError(); - if (pstrutStyles.isNotEmpty) throw _KatexHtmlParseError(); - - KatexSpanNode child = KatexSpanNode( - styles: styles, + final inlineStyles = _parseInlineStyles(innerSpan); + if (inlineStyles == null) throw _KatexHtmlParseError(); + final marginLeftEm = _takeStyleEm(inlineStyles, 'margin-left'); + final marginLeftIsNegative = marginLeftEm?.isNegative ?? false; + final marginRightEm = _takeStyleEm(inlineStyles, 'margin-right'); + if (marginRightEm?.isNegative ?? false) throw _KatexHtmlParseError(); + final styles = KatexSpanStyles( + marginLeftEm: marginLeftIsNegative ? null : marginLeftEm, + marginRightEm: marginRightEm, + ); + final topEm = _takeStyleEm(inlineStyles, 'top'); + if (inlineStyles.isNotEmpty) throw _KatexHtmlParseError(); + + final pstrutStyles = _parseInlineStyles(pstrutSpan); + if (pstrutStyles == null) throw _KatexHtmlParseError(); + final pstrutHeightEm = _takeStyleEm(pstrutStyles, 'height'); + if (pstrutHeightEm == null) throw _KatexHtmlParseError(); + if (pstrutStyles.isNotEmpty) throw _KatexHtmlParseError(); + + KatexSpanNode child = KatexSpanNode( + styles: styles, + text: null, + nodes: _parseChildSpans(otherSpans)); + + if (marginLeftIsNegative) { + child = KatexSpanNode( + styles: KatexSpanStyles(), text: null, - nodes: _parseChildSpans(otherSpans)); - - if (marginLeftIsNegative) { - child = KatexSpanNode( - styles: KatexSpanStyles(), - text: null, - nodes: [KatexNegativeMarginNode( - leftOffsetEm: marginLeftEm!, - nodes: [child])]); - } - - rows.add(KatexVlistRowNode( - verticalOffsetEm: (topEm ?? 0) + pstrutHeightEm, - debugHtmlNode: kDebugMode ? innerSpan : null, - node: child)); - } else { - throw _KatexHtmlParseError(); + nodes: [KatexNegativeMarginNode( + leftOffsetEm: marginLeftEm!, + nodes: [child])]); } - } - // TODO(#1716) Handle styling for .vlist-t2 spans - return KatexVlistNode( - rows: rows, - debugHtmlNode: debugHtmlNode, - ); - } else { - throw _KatexHtmlParseError(); + rows.add(KatexVlistRowNode( + verticalOffsetEm: (topEm ?? 0) + pstrutHeightEm, + debugHtmlNode: kDebugMode ? innerSpan : null, + node: child)); + } else { + throw _KatexHtmlParseError(); + } } + + // TODO(#1716) Handle styling for .vlist-t2 spans + return KatexVlistNode( + rows: rows, + debugHtmlNode: kDebugMode ? element : null, + ); } else { throw _KatexHtmlParseError(); } + } else { + throw _KatexHtmlParseError(); } + } + + static final _resetSizeClassRegExp = RegExp(r'^reset-size(\d\d?)$'); + static final _sizeClassRegExp = RegExp(r'^size(\d\d?)$'); + + KatexNode _parseGenericSpan(dom.Element element) { + assert(element.localName == 'span'); // Aggregate the CSS styles that apply, in the same order as the CSS // classes specified for this span, mimicking the behaviour on web. @@ -646,7 +664,7 @@ class _KatexParser { styles: styles, text: text, nodes: spans, - debugHtmlNode: debugHtmlNode); + debugHtmlNode: kDebugMode ? element : null); } /// Parse the inline CSS styles from the given element.