Skip to content

Commit 51a493f

Browse files
arnogarnog
committed
fix: fixed #2686 
1 parent ae67934 commit 51a493f

File tree

3 files changed

+238
-3
lines changed

3 files changed

+238
-3
lines changed

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@
6363
`\frac{{\colorbox{red}{344}}}{3}`.
6464
- **#2665** Clicking on a mathfield now only fires focus events once, instead of
6565
generating duplicate focus/blur/focusin/focusout events.
66+
- **#2686** When deleting a range that includes special mathematical atoms like
67+
`\sqrt`, `\frac`, or `\enclose`, the special atom structure is now properly
68+
removed or cleaned up. Previously, deleting content would leave behind empty
69+
special atom structures. Now, if a special atom becomes empty after deletion,
70+
it is removed entirely. Additionally, when a deletion crosses the boundary of
71+
a special atom (e.g., selecting content both outside and inside a square root),
72+
any remaining content inside the special atom is hoisted out, and the special
73+
atom structure is removed.
6674
- **#2698** Dispatched virtual keyboard events now include the standard `origin`
6775
property when `mathVirtualKeyboardPolicy = "sandboxed"`. Previously, when
6876
MathLive operated within an iframe in sandboxed mode, the MessageEvent

‎src/editor-model/delete.ts‎

Lines changed: 174 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,7 @@ export function deleteRange(
491491
const result = model.getAtoms(range);
492492
if (result.length > 0 && result[0].parent) {
493493
//
494-
// multiline environment (`\displaylines`, `multline`, `split`, `gather`, etc...)
494+
// multiline environment (`\\displaylines`, `multline`, `split`, `gather`, etc...)
495495
//
496496
let parent: Atom | undefined = result[0];
497497
while (parent && !(parent instanceof ArrayAtom)) parent = parent.parent;
@@ -547,7 +547,7 @@ export function deleteRange(
547547
const lastSelected = result[result.length - 1];
548548

549549
// If we're deleting all the children, also delete the parent
550-
// (for example for surd/\sqrt)
550+
// (for example for surd/\\sqrt)
551551
if (firstSelected === firstChild && lastSelected === lastChild) {
552552
const parent = result[0].parent!;
553553
if (parent.parent && parent.type !== 'prompt')
@@ -582,7 +582,178 @@ export function deleteRange(
582582
}
583583
return model.deferNotifications(
584584
{ content: true, selection: true, type },
585-
() => model.deleteAtoms(range)
585+
() => {
586+
// Track special atom parents and their relationship to the selection
587+
const specialAtomInfo = new Map<Atom, {
588+
containsStart: boolean;
589+
containsEnd: boolean;
590+
}>();
591+
592+
if (result.length > 0) {
593+
// Check if the start and end of the selection are in different contexts
594+
const startAtom = result[0];
595+
const endAtom = result[result.length - 1];
596+
597+
// Find all special atom ancestors for both start and end
598+
const startAncestors = new Set<Atom>();
599+
let p = startAtom.parent;
600+
while (p) {
601+
if (
602+
(p.type === 'surd' ||
603+
p.type === 'box' ||
604+
p.type === 'enclose' ||
605+
p.type === 'leftright' ||
606+
p.type === 'genfrac' ||
607+
p.type === 'overunder') &&
608+
p.parent
609+
) {
610+
startAncestors.add(p);
611+
}
612+
p = p.parent;
613+
}
614+
615+
const endAncestors = new Set<Atom>();
616+
p = endAtom.parent;
617+
while (p) {
618+
if (
619+
(p.type === 'surd' ||
620+
p.type === 'box' ||
621+
p.type === 'enclose' ||
622+
p.type === 'leftright' ||
623+
p.type === 'genfrac' ||
624+
p.type === 'overunder') &&
625+
p.parent
626+
) {
627+
endAncestors.add(p);
628+
}
629+
p = p.parent;
630+
}
631+
632+
// Collect all special atoms that are ancestors of any atom in the selection
633+
for (const atom of result) {
634+
p = atom.parent;
635+
while (p) {
636+
if (
637+
(p.type === 'surd' ||
638+
p.type === 'box' ||
639+
p.type === 'enclose' ||
640+
p.type === 'leftright' ||
641+
p.type === 'genfrac' ||
642+
p.type === 'overunder') &&
643+
p.parent
644+
) {
645+
if (!specialAtomInfo.has(p)) {
646+
specialAtomInfo.set(p, {
647+
containsStart: startAncestors.has(p),
648+
containsEnd: endAncestors.has(p),
649+
});
650+
}
651+
break; // Only track the innermost special atom
652+
}
653+
p = p.parent;
654+
}
655+
}
656+
}
657+
658+
// Delete the atoms
659+
const startPos = Math.min(...range);
660+
model.deleteAtoms(range);
661+
662+
// After deletion, check if any special atoms should be removed or hoisted
663+
for (const [specialAtom, info] of specialAtomInfo) {
664+
// Check if the atom still exists (it might have been deleted)
665+
if (!specialAtom.parent) continue;
666+
667+
// Check if this was a boundary-crossing deletion
668+
// (selection started outside and ended inside, or vice versa)
669+
const isBoundaryCrossing = info.containsStart !== info.containsEnd;
670+
671+
// For surd/box/enclose/leftright: handle both empty and boundary-crossing cases
672+
if (
673+
specialAtom.type === 'surd' ||
674+
specialAtom.type === 'box' ||
675+
specialAtom.type === 'enclose' ||
676+
specialAtom.type === 'leftright'
677+
) {
678+
const body = specialAtom.branch('body');
679+
const bodyEmpty = !body || body.length === 0 ||
680+
(body.length === 1 && body[0].type === 'placeholder');
681+
682+
if (bodyEmpty) {
683+
// Body is empty, remove the special atom
684+
const pos = model.offsetOf(specialAtom.leftSibling);
685+
specialAtom.parent.removeChild(specialAtom);
686+
model.position = Math.max(0, pos);
687+
} else if (isBoundaryCrossing && !info.containsStart && info.containsEnd) {
688+
// Selection started outside and ended inside - hoist remaining content
689+
const pos = model.offsetOf(specialAtom.leftSibling);
690+
const content = specialAtom.removeBranch('body');
691+
if (content && content.length > 0) {
692+
specialAtom.parent.addChildrenAfter(content, specialAtom);
693+
}
694+
specialAtom.parent.removeChild(specialAtom);
695+
model.position = Math.max(0, pos);
696+
}
697+
}
698+
699+
// For genfrac: if both branches are empty/placeholder, remove it
700+
// If one branch is empty, hoist the other
701+
// Also handle boundary-crossing deletions
702+
if (specialAtom.type === 'genfrac') {
703+
const above = specialAtom.branch('above');
704+
const below = specialAtom.branch('below');
705+
706+
const aboveEmpty = !above || above.length === 0 ||
707+
(above.length === 1 && above[0].type === 'placeholder');
708+
const belowEmpty = !below || below.length === 0 ||
709+
(below.length === 1 && below[0].type === 'placeholder');
710+
711+
if (aboveEmpty && belowEmpty) {
712+
// Both empty, remove the fraction
713+
const pos = model.offsetOf(specialAtom.leftSibling);
714+
specialAtom.parent.removeChild(specialAtom);
715+
model.position = Math.max(0, pos);
716+
} else if (aboveEmpty && !belowEmpty) {
717+
// Hoist the denominator
718+
const pos = model.offsetOf(specialAtom.leftSibling);
719+
const content = specialAtom.removeBranch('below');
720+
if (content && content.length > 0) {
721+
specialAtom.parent.addChildrenAfter(content, specialAtom);
722+
}
723+
specialAtom.parent.removeChild(specialAtom);
724+
model.position = Math.max(0, pos);
725+
} else if (!aboveEmpty && belowEmpty) {
726+
// Hoist the numerator
727+
const pos = model.offsetOf(specialAtom.leftSibling);
728+
const content = specialAtom.removeBranch('above');
729+
if (content && content.length > 0) {
730+
specialAtom.parent.addChildrenAfter(content, specialAtom);
731+
}
732+
specialAtom.parent.removeChild(specialAtom);
733+
model.position = Math.max(0, pos);
734+
} else if (isBoundaryCrossing && !info.containsStart && info.containsEnd) {
735+
// Selection started outside and ended inside - hoist whatever remains
736+
// For fractions, we need to check which branch has content and hoist it
737+
if (!aboveEmpty && !belowEmpty) {
738+
// Both have content - this is a more complex case
739+
// For now, just hoist the numerator
740+
const pos = model.offsetOf(specialAtom.leftSibling);
741+
const content = specialAtom.removeBranch('above');
742+
if (content && content.length > 0) {
743+
specialAtom.parent.addChildrenAfter(content, specialAtom);
744+
}
745+
specialAtom.parent.removeChild(specialAtom);
746+
model.position = Math.max(0, pos);
747+
}
748+
}
749+
}
750+
}
751+
752+
// Set position to where deletion started if not already set
753+
if (specialAtomInfo.size === 0) {
754+
model.position = startPos;
755+
}
756+
}
586757
);
587758
}
588759

‎test/playwright-tests/physical-keyboard.spec.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,62 @@ test('backspace on empty displaylines (issue #2739)', async ({ page }) => {
537537
expect(['', '\\displaylines{}', '\\displaylines{\\\\ }']).toContain(latex);
538538
});
539539

540+
test('delete range with sqrt - all content (issue #2686)', async ({ page }) => {
541+
await page.goto('/dist/playwright-test-page/');
542+
543+
// Test deleting a range that includes all sqrt content
544+
const result = await page.locator('#mf-1').evaluate((mfe: MathfieldElement) => {
545+
mfe.value = '\\sqrt{abc}+x';
546+
// Select all content inside sqrt by setting selection programmatically
547+
// The selection should be from the first atom inside sqrt to the last
548+
mfe.selection = { ranges: [[1, 4]] }; // Select "abc" inside the sqrt
549+
// Delete the selection
550+
mfe.executeCommand('deleteBackward');
551+
return mfe.value;
552+
});
553+
554+
// When all content of sqrt is deleted, the sqrt should be removed
555+
expect(result).toBe('+x');
556+
});
557+
558+
test('delete range with empty sqrt after deletion (issue #2686)', async ({ page }) => {
559+
await page.goto('/dist/playwright-test-page/');
560+
561+
// Test that when all content in sqrt is deleted, the sqrt is removed
562+
const result = await page.locator('#mf-1').evaluate((mfe: MathfieldElement) => {
563+
mfe.value = '\\sqrt{a}+b';
564+
// Select just "a" from inside the sqrt - this should leave sqrt empty
565+
mfe.selection = { ranges: [[1, 2]] };
566+
mfe.executeCommand('deleteBackward');
567+
return mfe.value;
568+
});
569+
570+
// After deleting "a", sqrt should be removed (empty), leaving just "+b"
571+
expect(result).toBe('+b');
572+
});
573+
574+
test('delete range crossing sqrt boundary - hoist remaining content (issue #2686)', async ({ page }) => {
575+
await page.goto('/dist/playwright-test-page/');
576+
577+
// Test the scenario: 1√23 -> select "1" and "2" -> delete -> should get "3"
578+
const result = await page.locator('#mf-1').evaluate((mfe: MathfieldElement) => {
579+
mfe.value = '1\\sqrt{23}';
580+
// Debug: let's see the positions
581+
// Position 0: before "1"
582+
// Position 1: after "1"
583+
// Position 2: inside sqrt, before "2"
584+
// Position 3: inside sqrt, between "2" and "3"
585+
// Position 4: inside sqrt, after "3"
586+
mfe.selection = { ranges: [[0, 3]] }; // Select "1" and "2"
587+
mfe.executeCommand('deleteBackward');
588+
return mfe.value;
589+
});
590+
591+
// After deleting "1" and "2", should hoist the "3" that remains in sqrt
592+
expect(result).toBe('3');
593+
});
594+
595+
540596
async function tab(page) {
541597
await page.keyboard.press('Tab');
542598
// Wait some time for focus to change

0 commit comments

Comments
 (0)