Skip to content

katex: Remove renderKaTex experimental flag #1742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 28, 2025
Merged

katex: Remove renderKaTex experimental flag #1742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c9a7b0
content test [nfc]: Drop skipped duplicate KaTeX font-size ratio test
rajveermalviya Jul 24, 2025
6459af6
content test [nfc]: Reorder KaTeX widget tests, enabled before disabled
rajveermalviya Jul 24, 2025
da5c1f7
katex: Remove renderKaTex experimental flag
rajveermalviya Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 14 additions & 18 deletions lib/model/katex.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,31 +116,27 @@ MathParserResult? parseMath(dom.Element element, { required bool block }) {
// content parsing stage here, thus the `!` here.
final globalStore = ZulipBinding.instance.getGlobalStoreSync()!;
final globalSettings = globalStore.settings;
final flagRenderKatex =
globalSettings.getBool(BoolGlobalSetting.renderKatex);
final flagForceRenderKatex =
globalSettings.getBool(BoolGlobalSetting.forceRenderKatex);

KatexParserHardFailReason? hardFailReason;
KatexParserSoftFailReason? softFailReason;
List<KatexNode>? nodes;
if (flagRenderKatex) {
final parser = _KatexParser();
try {
nodes = parser.parseKatexHtml(katexHtmlElement);
} on _KatexHtmlParseError catch (e, st) {
assert(debugLog('$e\n$st'));
hardFailReason = KatexParserHardFailReason(
message: e.message,
stackTrace: st);
}
final parser = _KatexParser();
try {
nodes = parser.parseKatexHtml(katexHtmlElement);
} on _KatexHtmlParseError catch (e, st) {
assert(debugLog('$e\n$st'));
hardFailReason = KatexParserHardFailReason(
message: e.message,
stackTrace: st);
}

if (parser.hasError && !flagForceRenderKatex) {
nodes = null;
softFailReason = KatexParserSoftFailReason(
unsupportedCssClasses: parser.unsupportedCssClasses,
unsupportedInlineCssProperties: parser.unsupportedInlineCssProperties);
}
if (parser.hasError && !flagForceRenderKatex) {
nodes = null;
softFailReason = KatexParserSoftFailReason(
unsupportedCssClasses: parser.unsupportedCssClasses,
unsupportedInlineCssProperties: parser.unsupportedInlineCssProperties);
}

return MathParserResult(
Expand Down
4 changes: 1 addition & 3 deletions lib/model/settings.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,14 @@ enum BoolGlobalSetting {
/// welcome dialog for upgrading from the legacy app.
upgradeWelcomeDialogShown(GlobalSettingType.internal, false),

/// An experimental flag to toggle rendering KaTeX content in messages.
renderKatex(GlobalSettingType.experimentalFeatureFlag, true),

/// An experimental flag to enable rendering KaTeX even when some
/// errors are encountered.
forceRenderKatex(GlobalSettingType.experimentalFeatureFlag, false),

// Former settings which might exist in the database,
// whose names should therefore not be reused:
// openFirstUnread // v0.0.30
// renderKatex // v0.0.29 - v30.0.261
;

const BoolGlobalSetting(this.type, this.default_);
Expand Down
36 changes: 31 additions & 5 deletions test/model/content_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import 'package:stack_trace/stack_trace.dart';
import 'package:test/scaffolding.dart';
import 'package:zulip/model/code_block.dart';
import 'package:zulip/model/content.dart';
import 'package:zulip/model/settings.dart';
import 'package:zulip/model/katex.dart';

import 'binding.dart';
Expand Down Expand Up @@ -528,6 +527,20 @@ class ContentExample {
]),
]));

// A test message to test the fallback behaviour of KaTeX implementation.
static final mathInlineUnknown = ContentExample.inline(
'inline math',
null, // r"$$ \lambda $$" (hypothetical server variation)
expectedText: r'\lambda',
'<p><span class="katex">'
'<span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>λ</mi></mrow>'
'<annotation encoding="application/x-tex"> \\lambda </annotation></semantics></math></span>'
'<span class="katex-html" aria-hidden="true">'
'<span class="base unknown">' // Server doesn't generate this 'unknown' class.
'<span class="strut" style="height:0.6944em;"></span>'
'<span class="mord mathnormal">λ</span></span></span></span></p>',
MathInlineNode(texSource: r'\lambda', nodes: null));

static const mathBlock = ContentExample(
'math block',
"```math\n\\lambda\n```",
Expand All @@ -547,6 +560,20 @@ class ContentExample {
]),
])]);

// A test message to test the fallback behaviour of KaTeX implementation.
static const mathBlockUnknown = ContentExample(
'math block unknown, fallback to TeX source',
null, // r"```math\n\lambda\n```" (hypothetical server variation)
expectedText: r'\lambda',
'<p><span class="katex-display"><span class="katex">'
'<span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML" display="block"><semantics><mrow><mi>λ</mi></mrow>'
'<annotation encoding="application/x-tex">\\lambda</annotation></semantics></math></span>'
'<span class="katex-html" aria-hidden="true">'
'<span class="base unknown">' // Server doesn't generate this 'unknown' class.
'<span class="strut" style="height:0.6944em;"></span>'
'<span class="mord mathnormal">λ</span></span></span></span></span></p>',
[MathBlockNode(texSource: r'\lambda', nodes: null)]);

static const mathBlocksMultipleInParagraph = ContentExample(
'math blocks, multiple in paragraph',
'```math\na\n\nb\n```',
Expand Down Expand Up @@ -1484,10 +1511,6 @@ void main() async {

TestZulipBinding.ensureInitialized();

// We need this to be able to test the currently experimental KaTeX code.
await testBinding.globalStore.settings.setBool(
BoolGlobalSetting.renderKatex, true);

//
// Inline content.
//
Expand Down Expand Up @@ -1599,6 +1622,7 @@ void main() async {
testParseExample(ContentExample.emojiZulipExtra);

testParseExample(ContentExample.mathInline);
testParseExample(ContentExample.mathInlineUnknown);

group('global times', () {
testParseExample(ContentExample.globalTime);
Expand Down Expand Up @@ -1777,6 +1801,8 @@ void main() async {
// into the context of a Zulip message.
// For tests going deeper inside KaTeX content, see katex_test.dart.
testParseExample(ContentExample.mathBlock);
testParseExample(ContentExample.mathBlockUnknown);

testParseExample(ContentExample.mathBlocksMultipleInParagraph);
testParseExample(ContentExample.mathBlockInQuote);
testParseExample(ContentExample.mathBlocksMultipleInQuote);
Expand Down
4 changes: 0 additions & 4 deletions test/model/katex_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import 'dart:io';

import 'package:zulip/model/settings.dart';
import 'package:checks/checks.dart';
import 'package:stack_trace/stack_trace.dart';
import 'package:test_api/scaffolding.dart';
Expand Down Expand Up @@ -572,9 +571,6 @@ class KatexExample extends ContentExample {
void main() async {
TestZulipBinding.ensureInitialized();

await testBinding.globalStore.settings.setBool(
BoolGlobalSetting.renderKatex, true);

testParseExample(KatexExample.mathBlockKatexSizing);
testParseExample(KatexExample.mathBlockKatexNestedSizing);
testParseExample(KatexExample.mathBlockKatexDelimSizing);
Expand Down
72 changes: 17 additions & 55 deletions test/widgets/content_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import '../example_data.dart' as eg;
import '../flutter_checks.dart';
import '../model/binding.dart';
import '../model/content_test.dart';
import '../model/store_checks.dart';
import '../model/test_store.dart';
import '../test_images.dart';
import '../test_navigation.dart';
Expand Down Expand Up @@ -566,23 +565,14 @@ void main() {

testContentSmoke(ContentExample.mathBlock);

testWidgets('displays KaTeX source; experimental flag disabled', (tester) async {
addTearDown(testBinding.reset);
final globalSettings = testBinding.globalStore.settings;
await globalSettings.setBool(BoolGlobalSetting.renderKatex, false);

testWidgets('displays KaTeX content', (tester) async {
await prepareContent(tester, plainContent(ContentExample.mathBlock.html));
tester.widget(find.text(r'\lambda', findRichText: true));
tester.widget(find.text('λ', findRichText: true));
});

testWidgets('displays KaTeX content; experimental flag enabled', (tester) async {
addTearDown(testBinding.reset);
final globalSettings = testBinding.globalStore.settings;
await globalSettings.setBool(BoolGlobalSetting.renderKatex, true);
check(globalSettings).getBool(BoolGlobalSetting.renderKatex).isTrue();

await prepareContent(tester, plainContent(ContentExample.mathBlock.html));
tester.widget(find.text('λ', findRichText: true));
testWidgets('fallback to displaying KaTeX source if unsupported KaTeX HTML', (tester) async {
await prepareContent(tester, plainContent(ContentExample.mathBlockUnknown.html));
tester.widget(find.text(r'\lambda', findRichText: true));
});
});

Expand Down Expand Up @@ -1000,11 +990,6 @@ void main() {
testContentSmoke(ContentExample.mathInline);

testWidgets('maintains font-size ratio with surrounding text', (tester) async {
addTearDown(testBinding.reset);
final globalSettings = testBinding.globalStore.settings;
await globalSettings.setBool(BoolGlobalSetting.renderKatex, true);
check(globalSettings.getBool(BoolGlobalSetting.renderKatex)).isTrue();

const html = '<span class="katex">'
'<span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>λ</mi></mrow>'
'<annotation encoding="application/x-tex"> \\lambda </annotation></semantics></math></span>'
Expand All @@ -1025,50 +1010,27 @@ void main() {
});
});

testWidgets('maintains font-size ratio with surrounding text, when showing TeX source', (tester) async {
const html = '<span class="katex">'
'<span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>λ</mi></mrow>'
'<annotation encoding="application/x-tex"> \\lambda </annotation></semantics></math></span>'
'<span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:0.6944em;"></span><span class="mord mathnormal">λ</span></span></span></span>';
await checkFontSizeRatio(tester,
targetHtml: html,
targetFontSizeFinder: mkTargetFontSizeFinderFromPattern(r'λ'));
}, skip: true // TODO(#46): adapt this test
// (it needs a more complex targetFontSizeFinder;
// see other uses in this file for examples.)
Comment on lines -1036 to -1038
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

content test [nfc]: Drop duplicate skipped KaTeX font-size ratio test

rather, it's a skipped duplicate such test — there's another KaTeX font-size ratio test, which this is a duplicate of, but there's no other skipped KaTeX font-size ratio test, so it can't be a duplicate within that category.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also please add a sentence to the commit message clarifying that the existing test this duplicates is the one above it, not the one below. I initially read it as the latter (because the descriptions match — my conflict-resolution error, probably, somewhere in the chain of cherry-picking that led to b073c6b / #1270), and was going to comment that it's not actually a duplicate of that 🙂

);

testWidgets('maintains font-size ratio with surrounding text, when showing TeX source', (tester) async {
addTearDown(testBinding.reset);
final globalSettings = testBinding.globalStore.settings;
await globalSettings.setBool(BoolGlobalSetting.renderKatex, false);

const html = '<span class="katex">'
testWidgets('maintains font-size ratio with surrounding text, when falling back to TeX source', (tester) async {
const unsupportedHtml = '<span class="katex">'
'<span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>λ</mi></mrow>'
'<annotation encoding="application/x-tex"> \\lambda </annotation></semantics></math></span>'
'<span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:0.6944em;"></span><span class="mord mathnormal">λ</span></span></span></span>';
'<span class="katex-html" aria-hidden="true">'
'<span class="base unknown">' // Server doesn't generate this 'unknown' class.
'<span class="strut" style="height:0.6944em;"></span>'
'<span class="mord mathnormal">λ</span></span></span></span>';
await checkFontSizeRatio(tester,
targetHtml: html,
targetHtml: unsupportedHtml,
targetFontSizeFinder: mkTargetFontSizeFinderFromPattern(r'\lambda'));
});

testWidgets('displays KaTeX source; experimental flag disabled', (tester) async {
addTearDown(testBinding.reset);
final globalSettings = testBinding.globalStore.settings;
await globalSettings.setBool(BoolGlobalSetting.renderKatex, false);

testWidgets('displays KaTeX content', (tester) async {
await prepareContent(tester, plainContent(ContentExample.mathInline.html));
tester.widget(find.text(r'\lambda', findRichText: true));
tester.widget(find.text('λ', findRichText: true));
});

testWidgets('displays KaTeX content; experimental flag enabled', (tester) async {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: make summary line more specific

content test [nfc]: Reorder KaTeX widget tests

With renderKatex flag enabled (and soon to be default behaviour)
being first.

There's room in that summary line to put some of that information from the body, to give an idea of what the reordering is about. Like:

content test [nfc]: Reorder KaTeX widget tests, enabled before disabled

addTearDown(testBinding.reset);
final globalSettings = testBinding.globalStore.settings;
await globalSettings.setBool(BoolGlobalSetting.renderKatex, true);
check(globalSettings.getBool(BoolGlobalSetting.renderKatex)).isTrue();

await prepareContent(tester, plainContent(ContentExample.mathInline.html));
tester.widget(find.text('λ', findRichText: true));
testWidgets('fallback to displaying KaTeX source if unsupported KaTeX HTML', (tester) async {
await prepareContent(tester, plainContent(ContentExample.mathInlineUnknown.html));
tester.widget(find.text(r'\lambda'));
});
});

Expand Down
7 changes: 0 additions & 7 deletions test/widgets/katex_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import 'package:checks/checks.dart';
import 'package:flutter/services.dart';
import 'package:flutter_checks/flutter_checks.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:zulip/model/settings.dart';
import 'package:zulip/widgets/katex.dart';

import '../model/binding.dart';
import '../model/katex_test.dart';
import '../model/store_checks.dart';
import 'content_test.dart';

void main() {
Expand Down Expand Up @@ -81,11 +79,6 @@ void main() {
testWidgets(testCase.$1.description, (tester) async {
await _loadKatexFonts();

addTearDown(testBinding.reset);
final globalSettings = testBinding.globalStore.settings;
await globalSettings.setBool(BoolGlobalSetting.renderKatex, true);
check(globalSettings).getBool(BoolGlobalSetting.renderKatex).isTrue();

await prepareContent(tester, plainContent(testCase.$1.html));

final baseRect = tester.getRect(find.byType(KatexWidget));
Expand Down
3 changes: 0 additions & 3 deletions tools/content/unimplemented_katex_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,12 @@ import 'package:collection/collection.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:zulip/model/content.dart';
import 'package:zulip/model/settings.dart';

import '../../test/model/binding.dart';
import 'model.dart';

void main() async {
TestZulipBinding.ensureInitialized();
await testBinding.globalStore.settings.setBool(
BoolGlobalSetting.renderKatex, true);

Future<void> checkForKatexFailuresInFile(File file) async {
int totalMessageCount = 0;
Expand Down