Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions iosMath/lib/MTMathList.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ typedef NS_ENUM(NSUInteger, MTMathAtomType)
kMTMathAtomText = 19,
/// A box atom (phantom/smash/lap family). Script-capable (< kMTMathAtomBoundary).
kMTMathAtomBox = 20,
/// A brace group {…} in math mode — an Ord subformula whose nucleus is a
/// sub-mlist (== TeX Ord noad with sub_mlist / KaTeX "ordgroup").
/// Script-capable (< kMTMathAtomBoundary); spaced as Ordinary.
kMTMathAtomOrdGroup = 21,

// Atoms after this point do not support subscripts or superscripts

Expand Down Expand Up @@ -646,6 +650,24 @@ typedef NS_ENUM(NSUInteger, MTBoxHAlign) {

@end

/** A brace group `{…}` in math mode — an Ord subformula whose nucleus is a
sub-mlist. Script-capable; spaced as Ordinary. The honest analog of TeX's
Ord noad with a `sub_mlist` nucleus and KaTeX's `ordgroup` node. The usual
nucleus is empty; the grouped content lives in `innerList`, and the
sub/superScript fields drive scripting of the whole group. */
@interface MTMathGroup : MTMathAtom

/// Creates an empty Ord group (type = kMTMathAtomOrdGroup).
- (instancetype) init NS_DESIGNATED_INITIALIZER;

/// Throws unless type == kMTMathAtomOrdGroup.
- (instancetype) initWithType:(MTMathAtomType)type value:(NSString *)value;

/// The grouped math content.
@property (nonatomic, nonnull) MTMathList* innerList;

@end

/** An atom representing an table element. This atom is not like other
atoms and is not present in TeX. We use it to represent the `\halign` command
in TeX with some simplifications. This is used for matrices, equation
Expand Down
65 changes: 65 additions & 0 deletions iosMath/lib/MTMathList.m
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ static BOOL isNotBinaryOperator(MTMathAtom* prevNode)
return @"Text";
case kMTMathAtomBox:
return @"Box";
case kMTMathAtomOrdGroup:
return @"Ord Group";
case kMTMathAtomBoundary:
return @"Boundary";
case kMTMathAtomSpace:
Expand Down Expand Up @@ -158,6 +160,9 @@ + (instancetype)atomWithType:(MTMathAtomType)type value:(NSString *)value
case kMTMathAtomBox:
return [[MTMathBox alloc] init];

case kMTMathAtomOrdGroup:
return [[MTMathGroup alloc] init];

case kMTMathAtomSpace:
return [[MTMathSpace alloc] initWithSpace:0];

Expand Down Expand Up @@ -1075,6 +1080,66 @@ - (instancetype)finalized

@end

#pragma mark - MTMathGroup

@implementation MTMathGroup

- (instancetype)init
{
self = [super initWithType:kMTMathAtomOrdGroup value:@""];
if (self) {
_innerList = [MTMathList new];
}
return self;
}

- (instancetype)initWithType:(MTMathAtomType)type value:(NSString *)value
{
if (type == kMTMathAtomOrdGroup) {
return [self init];
}
@throw [NSException exceptionWithName:@"InvalidMethod"
reason:@"[MTMathGroup initWithType:value:] cannot be called. Use [MTMathGroup init] instead."
userInfo:nil];
}

// Standalone string (description / error messages): braces + this atom's scripts.
- (NSString *)stringValue
{
NSMutableString* str = [NSMutableString stringWithFormat:@"{%@}",
[MTMathListBuilder mathListToString:self.innerList]];
if (self.superScript) {
[str appendFormat:@"^{%@}", self.superScript.stringValue];
}
if (self.subScript) {
[str appendFormat:@"_{%@}", self.subScript.stringValue];
}
return str;
}

// List serializer: emit ONLY {inner}. mathListToString: appends this atom's own
// ^{…}/_{…} afterwards, so emitting scripts here would double them.
- (void)appendLaTeXToString:(NSMutableString *)str
{
[str appendFormat:@"{%@}", [MTMathListBuilder mathListToString:self.innerList]];
}

- (id)copyWithZone:(NSZone *)zone
{
MTMathGroup* group = [super copyWithZone:zone];
group.innerList = [self.innerList copyWithZone:zone];
return group;
}

- (instancetype)finalized
{
MTMathGroup* newGroup = [super finalized];
newGroup.innerList = newGroup.innerList.finalized;
return newGroup;
}

@end

#pragma mark - MTMathTable

@interface MTMathTable ()
Expand Down
56 changes: 51 additions & 5 deletions iosMath/lib/MTMathListBuilder.m
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ @implementation MTMathListBuilder {
MTFontStyle _currentFontStyle;
BOOL _spacesAllowed;
NSInteger _recursionDepth;
// Set to YES by stopCommand when a TeX group-transformation command (\over,
// \atop, \choose, \brack, \brace) fires inside a {…} group. Checked in the
// {…} branch to decide whether to wrap as MTMathGroup. Cleared at the top of
// every buildInternal call so the check is always fresh.
BOOL _groupWasTransformedByStopCommand;
}

- (instancetype)initWithString:(NSString *)str
Expand Down Expand Up @@ -171,6 +176,7 @@ - (MTMathList*)buildInternal:(BOOL) oneCharOnly stopChar:(unichar) stop
return nil;
}
_recursionDepth++;
_groupWasTransformedByStopCommand = NO;
@try {
MTMathList* list = [MTMathList new];
NSAssert(!(oneCharOnly && (stop > 0)), @"Cannot set both oneCharOnly and stopChar.");
Expand Down Expand Up @@ -224,12 +230,46 @@ - (MTMathList*)buildInternal:(BOOL) oneCharOnly stopChar:(unichar) stop
} else if (ch == '{') {
// this puts us in a recursive routine, and sets oneCharOnly to false and no stop character
MTMathList* sublist = [self buildInternal:false stopChar:'}'];
prevAtom = [sublist.atoms lastObject];
[list append:sublist];
if (oneCharOnly) {
return list;
if (!sublist) {
// inner error already set (e.g. missing closing brace); propagate.
return nil;
}
continue;
// Read-and-clear: a \over/\atop-\class transform fired by an INNER
// group must not leak into THIS (enclosing) group's decision. The flag
// is set in stopCommand: and reset at the top of each buildInternal:,
// but the inner recursion runs between our reset and this read, so
// without clearing here a transformed inner group would wrongly
// suppress wrapping of the outer group (dropping it + leaking any
// \scriptstyle inside it — a #177 regression).
BOOL transformed = _groupWasTransformedByStopCommand;
_groupWasTransformedByStopCommand = NO;
if (oneCharOnly || transformed) {
// Field brace (^{…}, _{…}, \frac{…}, command argument): the {…}
// *is* the field. Flatten and return it as the field — unchanged.
// Also: a group-transforming command (\over, \atop, \choose,
// \brack, \brace) fired inside this group. The resulting fraction
// replaces the group in the parent list (TeX behavior) — do NOT
// wrap in MTMathGroup. Fall through to continue after appending.
// Update prevAtom to the last appended atom so a following ^ / _ /
// prime attaches to the fraction (or field atom), not a spurious
// empty Ord — mirrors the pre-grouping behavior and the shared
// append path below (prevAtom = atom).
prevAtom = [sublist.atoms lastObject];
[list append:sublist];
if (oneCharOnly) {
return list;
}
continue;
}
Comment on lines +244 to +263

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

There are two critical issues in this block:

  1. prevAtom is not updated: In the transformed path, sublist is appended to list and the loop continues, but prevAtom is never updated to the last atom of sublist. This causes any subsequent scripts (like ^2) or operators to attach to the wrong atom (or create a new empty atom) instead of the transformed fraction.
  2. _groupWasTransformedByStopCommand leak: Since _groupWasTransformedByStopCommand is a shared instance variable, when a nested group is transformed (e.g., {a {b \over c}}), the flag remains YES even after returning to the parent group. This causes grandparent groups to also skip wrapping in MTMathGroup. Resetting it to NO immediately after reading prevents this leak.
            BOOL transformed = _groupWasTransformedByStopCommand;
            _groupWasTransformedByStopCommand = NO;
            if (oneCharOnly || transformed) {
                // Field brace (^{…}, _{…}, \frac{…}, command argument): the {…}
                // *is* the field. Flatten and return it as the field — unchanged.
                // Also: a group-transforming command (\over, \atop, \choose,
                // \brack, \brace) fired inside this group. The resulting fraction
                // replaces the group in the parent list (TeX behavior) — do NOT
                // wrap in MTMathGroup. Fall through to continue after appending.
                prevAtom = [sublist.atoms lastObject];
                [list append:sublist];
                if (oneCharOnly) {
                    return list;
                }
                continue;
            }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified both points against the current head (adbe7f9).

1. prevAtom not updated — real bug, fixed in adbe7f9. Confirmed by reproducing it: {a \over b}^2 serialized as \frac{a}{b}{}^{2} — the ^2 attached to a spurious empty Ord instead of the fraction, because the transformed branch appended sublist without setting prevAtom (the ^/_/' branches all key off prevAtom). Restored prevAtom = [sublist.atoms lastObject] in that path, matching the pre-grouping behavior and the shared append path below. Regression test testScriptAfterOverTransformedGroupAttachesToFraction added.

2. _groupWasTransformedByStopCommand leak — already fixed in f99517a. This review was filed against b7a705a (3rd commit). The 4th commit f99517a ("Fix stale _groupWasTransformedByStopCommand leaking into enclosing group") landed afterward and does exactly the read-and-clear suggested here — BOOL transformed = _groupWasTransformedByStopCommand; _groupWasTransformedByStopCommand = NO; at MTMathListBuilder.m:244–245, read and cleared in the {…} branch before any fallthrough. The {{a \over b}\scriptstyle c}z leak variant is covered by testBraceGroupingAroundOverTransform. No further change.

// Grouping brace in the main list: wrap as an Ord subformula so style
// nodes are scoped, scripts target the whole group, and Bin/Ord
// reclassification stops at the brace boundary
// (== TeX Ord-noad-with-sub_mlist / KaTeX ordgroup).
MTMathGroup* group = [[MTMathGroup alloc] init];
group.innerList = sublist;
atom = group;
// fall through to the shared append path below: it sets prevAtom = group
// (so {x}^2 scripts the group) and finalize assigns the indexRange.
} else if (ch == '}') {
NSAssert(!oneCharOnly, @"This should have been handled before");
NSAssert(stop == 0, @"This should have been handled before");
Expand Down Expand Up @@ -1178,6 +1218,12 @@ - (MTMathList*) stopCommand:(NSString*) command list:(MTMathList*) list stopChar
}
MTMathList* fracList = [MTMathList new];
[fracList addAtom:frac];
// Signal to the {…} branch that this group was transformed by a TeX
// group-transformation command (\over / \atop / \choose / \brack / \brace).
// The fraction should be inserted into the parent list directly (not wrapped
// in MTMathGroup), mirroring TeX's behavior where these commands replace the
// enclosing group with a generalized fraction.
_groupWasTransformedByStopCommand = YES;
return fracList;
} else if ([command isEqualToString:@"\\"] || [command isEqualToString:@"cr"]) {
if (_currentEnv) {
Expand Down
25 changes: 25 additions & 0 deletions iosMath/render/internal/MTTypesetter.m
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,31 @@ - (void) createDisplayAtoms:(NSArray*) preprocessed
break;
}

case kMTMathAtomOrdGroup: {
// A brace group {…}: an Ord subformula. Lay out its sub-mlist with a
// fresh typesetter — that recursion scopes any interior style node
// (the #177 fix) — then place the child inline like \color and run
// scripts on the whole group.
if (_currentLine.length > 0) {
[self addDisplayLine];
}
// Spaced as Ordinary: reclassify before any inter-element lookup
// (mirrors the Box case), so it never reaches the default assert.
[self addInterElementSpace:prevNode currentType:kMTMathAtomOrdinary];
atom.type = kMTMathAtomOrdinary;

MTMathGroup* groupAtom = (MTMathGroup*) atom;
MTMathListDisplay* child = [MTTypesetter createLineForMathList:groupAtom.innerList font:_font style:_style];
child.position = _currentPosition;
_currentPosition.x += child.width;
[_displayAtoms addObject:child];

if (atom.subScript || atom.superScript) {
[self makeScripts:atom display:child index:atom.indexRange.location delta:0];
}
break;
}

case kMTMathAtomText: {
// Flush any pending math run so the text block stands alone.
if (_currentLine.length > 0) {
Expand Down
Loading
Loading