fastmail / Squire

The rich text editor for arbitrary HTML.
MIT License
4.75k stars 406 forks source link

Html gets reformatted on Undo #463

Open paulflo150 opened 5 months ago

paulflo150 commented 5 months ago

Make a change to the html and then press CTRL+Z. The original html gets completely reformatted where several <p><br></p> are being inserted.

Is there any way to return to the original html, or how would I go about removing these <p><br></p> nodes?

This is my html: <p><b>Chapter 1</b> <br />Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.</p><p><b>Chapter 2</b> <br />Pretium viverra suspendisse potenti nullam. Rutrum quisque non tellus orci ac auctor augue mauris augue. Sed egestas egestas fringilla phasellus faucibus.</p><p><b>Chapter 3</b> <br />Velit aliquet sagittis id consectetur purus. Sed ullamcorper morbi tincidunt ornare massa eget egestas purus. Elementum eu facilisis sed odio morbi quis commodo odio aenean.</p>

Here is the stacblitz: https://stackblitz.com/edit/web-platform-altctd

https://github.com/fastmail/Squire/assets/8136504/83a369f5-9119-4932-83e4-db7a536cf885

Thanks!

tomohiro-okajima commented 2 weeks ago

First of all, thank you for this amazing project.

I am also encountering a similar issue. After investigating, I believe the issue might be related to setHTML -> cleanupBRs -> fixContainer(here)

Possible Cause: The cleanupBRs function converts the input into invalid HTML. e.g., <p>foo<br>bar</p> to <p><div>foo<br /></div><div>bar<br /></div></p> (A p element cannot contain a div element.)

Consequently, the following occurs:

  1. undoStack <p><input id="squire-selection-start" type="hidden"><input id="squire-selection-end" type="hidden"><div>foo<br></div><div>bar<br></div></p>
  2. undo()->_setRawHTML->'root.innerHTML = html;' <p><input id="squire-selection-start" type="hidden"><input id="squire-selection-end" type="hidden"></p><div>foo<br></div><div>bar<br></div><p></p> (The browser completes the closing p tag.)
  3. _setRawHTML's result <p><input id="squire-selection-start" type="hidden"><input id="squire-selection-end" type="hidden"><br></p><div>foo<br></div><div>bar<br></div><p><br></p>

Proposed Solution: Execute cleanupBRs -> fixContainer only if allowed. e.g.,

const cleanupBRs = ( ... ): void => {
    ...
        } else if (!isInline(parent) && canNodeContainDiv(parent)) {
            fixContainer(parent, root);
        }
    }
};

// Regular expression to match allowed parent nodes for <div> elements
const allowedDivParentNodes =
    /^(?:BODY|ARTICLE|SECTION|NAV|ASIDE|HEADER|FOOTER|ADDRESS|BLOCKQUOTE|LI|DL|DT|DD|FIGURE|FIGCAPTION|MAIN|DIV|A|INS|DEL|OBJECT|VIDEO|AUDIO|MAP|CAPTION|TD|TH|FORM|FIELDSET|DETAILS|DIALOG|NOSCRIPT|TEMPLATE|CANVAS)$/;

// Determine if a node can contain a <div> element
const canNodeContainDiv = (node: Node): boolean => {
    return (
        node instanceof Element &&
        allowedDivParentNodes.test(node.nodeName.toUpperCase())
    );
};

Looking forward to your response. Thanks!