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
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,16 @@ export class ParagraphNodeView {
* @returns {boolean}
*/
#updateListStyles() {
let { suffix, justification } = this.node.attrs.listRendering;
const listRendering = this.node.attrs.listRendering;
// When listRendering is null (can happen transiently during certain
// transactions, e.g. after a setDocAttribute + paragraph delete), leave
// the existing marker/separator untouched. Forcing a default `suffix` here
// would risk writing tab-style CSS onto a text-node separator created by
// a prior 'space'/'nothing' suffix and scheduled RAF pass.
if (!listRendering) {
return true;
}
let { suffix, justification } = listRendering;
suffix = suffix ?? 'tab';
this.#calculateMarkerStyle(justification);
if (suffix === 'tab') {
Expand Down Expand Up @@ -280,9 +289,15 @@ export class ParagraphNodeView {
}

/**
* @param {{ markerText: string, suffix?: string }} listRendering
* @param {{ markerText: string, suffix?: string } | null} listRendering
*/
#initList(listRendering) {
// See #updateListStyles: when listRendering is null the previous marker/
// separator are left in place; avoid invoking the create helpers with
// undefined values.
if (!listRendering) {
return;
}
this.#createMarker(listRendering.markerText);
this.#createSeparator(listRendering.suffix);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,82 @@ describe('ParagraphNodeView', () => {
expect(nodeView.separator.textContent).toBe('\u00A0');
});

it('does not throw when listRendering is null', () => {
// Regression: #updateListStyles destructured `{ suffix, justification }`
// from `this.node.attrs.listRendering` without a null-check, throwing
// `TypeError: Cannot destructure property 'suffix' of ... as it is null`
// whenever a paragraph node carried `listRendering: null`.
isList.mockReturnValue(true);
const baseAttrs = createNode().attrs;
const { nodeView } = mountNodeView({ attrs: { ...baseAttrs } });

const nextNode = createNode({
attrs: {
...baseAttrs,
listRendering: null,
},
});

expect(() => nodeView.update(nextNode, [])).not.toThrow();
});

it('does not try to style a text-node separator when switching to null listRendering', () => {
// Regression: when transitioning from a 'space'/'nothing' suffix (which
// creates a text-node separator) to `listRendering: null`, the null-guarded
// path must not fall back to the 'tab' branch, since writing
// `this.separator.style.cssText` on a Text node throws.
isList.mockReturnValue(true);
const spaceAttrs = {
...createNode().attrs,
listRendering: { suffix: 'space', justification: 'left', markerText: '1.' },
};
const { nodeView } = mountNodeView({ attrs: spaceAttrs });
// The separator should be a Text node under the 'space' suffix.
expect(nodeView.separator?.nodeType).toBe(Node.TEXT_NODE);
const textSeparator = nodeView.separator;

const nullNode = createNode({
attrs: { ...spaceAttrs, listRendering: null },
});

expect(() => nodeView.update(nullNode, [])).not.toThrow();
// The text-node separator must be left alone (not replaced, not styled).
expect(nodeView.separator).toBe(textSeparator);
});
Comment thread
kendaller marked this conversation as resolved.
Comment thread
kendaller marked this conversation as resolved.

it('does not throw when mounted with listRendering null', () => {
// Regression: the null guards in #initList and #updateListStyles must also
// cover the constructor path — mounting a paragraph whose listRendering is
// already null previously threw before update() ever ran.
isList.mockReturnValue(true);
const nullAttrs = { ...createNode().attrs, listRendering: null };
expect(() => mountNodeView({ attrs: nullAttrs })).not.toThrow();
});

it('recovers marker/separator when listRendering returns from null to tab', () => {
// Regression: the null-guarded path leaves the existing marker/separator in
// place. When listRendering clears and later returns with a different suffix
// (here: space → null → tab), the separator has to swap from a text node
// back to a span element — #createSeparator handles this only if the
// recovery path actually runs, so exercise it end-to-end.
isList.mockReturnValue(true);
const spaceAttrs = {
...createNode().attrs,
listRendering: { suffix: 'space', justification: 'left', markerText: '1.' },
};
const { nodeView } = mountNodeView({ attrs: spaceAttrs });

nodeView.update(createNode({ attrs: { ...spaceAttrs, listRendering: null } }), []);

const tabNode = createNode({
attrs: { ...spaceAttrs, listRendering: { suffix: 'tab', justification: 'left', markerText: '2.' } },
});
nodeView.update(tabNode, []);

expect(nodeView.marker?.textContent).toBe('2.');
expect(nodeView.separator?.tagName?.toLowerCase()).toBe('span');
});

it('uses hanging indent width for right-justified tabs and skips tab helper', () => {
isList.mockReturnValue(true);
const attrs = {
Expand Down
Loading