Skip to content
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.

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

Fix sanitizeScriptTags when content is null #3316

Merged

Conversation

G-Ambatte
Copy link
Collaborator

This PR resolves #3315.

This PR modifies the sanitizeScriptTags function to properly handle the null case, which will preventing website crashing to a white screen when the content window is empty.

@G-Ambatte G-Ambatte self-assigned this Feb 22, 2024
@G-Ambatte G-Ambatte added the P1 - high priority Obvious bug or popular features label Feb 22, 2024
@calculuschild
Copy link
Member

calculuschild commented Feb 22, 2024

Just noting here for future reference: The error is caused by the addition of this line:

renderedPages[props.currentEditorPage] = renderPage(rawPages[props.currentEditorPage], props.currentEditorPage);

This line is to make sure when rendering pages, always render the currently-edited page first (which may not be in view on the brewRenderer preview). This ensures that any changes that have cross-page effects (e.g. variables or links) will be parsed first, so the latest values will propagate correctly to the pages in view.

Unfortunately, updating the "current editor page" happens slightly out of sync here:

Codemirror sees a change -> sends this change up to the top at handleTextChange() in editPage.jsx (or newPage or homePage) and gets the currently edited page from editor.jsx -> propagates new text down to editor.jsx and brewRenderer.jsx.

As you can see, editor.jsx is being asked to calculate the latest cursor position before it it has received the latest text.

currentEditorPage : this.refs.editor.getCurrentPage() - 1 //Offset index since Marked starts pages at 0

So while this PR does fix the crashing (I will merge this for now), it might be worth moving getCurrentPage() to come directly from Codemirror in codeEditor.jsx instead which will be more accurate.

@calculuschild calculuschild merged commit 94d9e1b into naturalcrit:master Feb 22, 2024
1 check passed
@G-Ambatte G-Ambatte deleted the fixSantizeScriptTags-#3315 branch March 27, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 - high priority Obvious bug or popular features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sanitize Script Tags crashes when content is null/empty
2 participants