Use Monaco Editor and save code in submitpage.php when edit the content#989
Merged
Conversation
Contributor
Reviewer's GuideReplace the legacy CodeMirror-based submission editor with a Monaco-backed abstraction, use it on submitpage.php with autosave and fallbacks, and adjust several read-only code views and headers while bumping versions to 3.5.2. Sequence diagram for Monaco-backed submission editor initialization and autosavesequenceDiagram
actor User
participant Browser
participant main
participant createMonacoEditor
participant ensureMonaco
participant monaco_editor as monaco.editor
participant localStorage
User->>Browser: Open /submitpage.php
Browser->>main: execute()
main->>main: Build _submitPageHeader DOM
main->>main: Define getSubmitStorageKey()
main->>createMonacoEditor: createMonacoEditor('MonacoEditor', editorOptions)
activate createMonacoEditor
createMonacoEditor->>ensureMonaco: ensureMonaco()
activate ensureMonaco
ensureMonaco->>Browser: load loader.js via MonacoCDN
ensureMonaco->>Browser: require(['vs/editor/editor.main'])
ensureMonaco-->>createMonacoEditor: monaco available
deactivate ensureMonaco
createMonacoEditor->>monaco_editor: monaco.editor.create(innerHost, options)
createMonacoEditor->>localStorage: getItem(localStorageKey)
localStorage-->>createMonacoEditor: saved code or null
createMonacoEditor-->>main: CodeMirrorElement (Monaco adapter)
deactivate createMonacoEditor
main->>monaco_editor: addCommand(Ctrl+Enter, Submit.click)
main->>monaco_editor: addCommand(Ctrl+Space, triggerSuggest)
main->>Browser: remove loadEditor placeholder
loop onDidChangeModelContent
monaco_editor-->>createMonacoEditor: content changed
createMonacoEditor->>localStorage: setItem(localStorageKey, editor.getValue())
end
alt Monaco initialization fails
createMonacoEditor--x main: throw Error
main->>Browser: hide MonacoEditor
main->>Browser: show CodeInput textarea
main->>localStorage: getItem(getSubmitStorageKey())
main->>Browser: attach keydown Ctrl+Enter -> Submit.click
loop textarea input
Browser->>localStorage: setItem(fallbackKey, textarea.value)
end
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
hendragon-bot
Bot
added
the
user-script
Deploying xmoj-script-dev-channel with
|
| Latest commit: |
317d4af
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5fb59fb4.xmoj-script-dev-channel.pages.dev |
| Branch Preview URL: | https://def-wa2025-monaco-editor-and.xmoj-script-dev-channel.pages.dev |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 security issues, 1 other issue, and left some high level feedback:
Security issues:
- User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
document.querySelector("body > div > div.mt-3").innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link)
General comments:
- On submitpage.php the hidden #CodeInput textarea is no longer bound to the editor as it was with CodeMirror.fromTextArea; make sure you copy the Monaco editor content back into #CodeInput (e.g., in the submit handler) so the backend still receives the code.
- In the freopen snippet error block you always append an empty div (codeHost) to #ErrorMessage and, in the fallback path, also append a
directly to #ErrorMessage; consider either rendering the fallback
inside codeHost or skipping codeHost creation when Monaco is unavailable to avoid redundant markup.
- The CodeMirror shim exposes _monacoEditor and callers reach into that to add commands; consider exposing a small public API (e.g., addCommand/triggerSuggest) on the adapter instead of using the private _monacoEditor field to keep the abstraction boundary cleaner.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- On submitpage.php the hidden #CodeInput textarea is no longer bound to the editor as it was with CodeMirror.fromTextArea; make sure you copy the Monaco editor content back into #CodeInput (e.g., in the submit handler) so the backend still receives the code.
- In the freopen snippet error block you always append an empty div (codeHost) to #ErrorMessage and, in the fallback path, also append a <pre> directly to #ErrorMessage; consider either rendering the fallback <pre> inside codeHost or skipping codeHost creation when Monaco is unavailable to avoid redundant markup.
- The CodeMirror shim exposes _monacoEditor and callers reach into that to add commands; consider exposing a small public API (e.g., addCommand/triggerSuggest) on the adapter instead of using the private _monacoEditor field to keep the abstraction boundary cleaner.
## Individual Comments
### Comment 1
<location path="XMOJ.user.js" line_range="374-383" />
<code_context>
+ shim.MergeView = function(container, options) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** MergeView hardcodes the language to C++, ignoring any mode/language passed in options.
The shim currently instantiates both models with language `'cpp'` and ignores `options.mode` (or any language option), which changes behavior compared to the CodeMirror-based MergeView for non-C++ content. Please derive the Monaco language from the provided mode (similar to `shim`/`createMonacoEditor`, mapping known CodeMirror modes and defaulting to C++) so other languages still get appropriate syntax handling.
</issue_to_address>
### Comment 2
<location path="XMOJ.user.js" line_range="3633-3638" />
<code_context>
document.querySelector("body > div > div.mt-3").innerHTML = `<center class="mb-3">` + `<h3>提交代码</h3>` + (SearchParams.get("id") != null ? `题目<span class="blue">${Number(SearchParams.get("id"))}</span>` : `比赛<span class="blue">${Number(SearchParams.get("cid")) + `</span> 题目<span class="blue">` + String.fromCharCode(65 + parseInt(SearchParams.get("pid")))}</span>`) + `</center>
<div id="MonacoEditor" style="width:100%; height:400px;"></div>
<textarea id="CodeInput" style="display:none"></textarea>
<center class="mt-3">
<input id="enable_O2" name="enable_O2" type="checkbox"><label for="enable_O2">打开O2开关</label>
<br>
<input id="Submit" class="btn btn-info mt-2" type="button" value="提交">
<div id="ErrorElement" class="mt-2" style="display: none; text-align: left; padding: 10px;">
<div id="ErrorMessage" style="white-space: pre; background-color: rgba(0, 0, 0, 0.1); padding: 10px; border-radius: 5px;"></div>
<button id="PassCheck" class="btn btn-outline-secondary mt-2" style="display: none">强制提交</button>
</div>
</center>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 3
<location path="XMOJ.user.js" line_range="3633-3638" />
<code_context>
document.querySelector("body > div > div.mt-3").innerHTML = `<center class="mb-3">` + `<h3>提交代码</h3>` + (SearchParams.get("id") != null ? `题目<span class="blue">${Number(SearchParams.get("id"))}</span>` : `比赛<span class="blue">${Number(SearchParams.get("cid")) + `</span> 题目<span class="blue">` + String.fromCharCode(65 + parseInt(SearchParams.get("pid")))}</span>`) + `</center>
<div id="MonacoEditor" style="width:100%; height:400px;"></div>
<textarea id="CodeInput" style="display:none"></textarea>
<center class="mt-3">
<input id="enable_O2" name="enable_O2" type="checkbox"><label for="enable_O2">打开O2开关</label>
<br>
<input id="Submit" class="btn btn-info mt-2" type="button" value="提交">
<div id="ErrorElement" class="mt-2" style="display: none; text-align: left; padding: 10px;">
<div id="ErrorMessage" style="white-space: pre; background-color: rgba(0, 0, 0, 0.1); padding: 10px; border-radius: 5px;"></div>
<button id="PassCheck" class="btn btn-outline-secondary mt-2" style="display: none">强制提交</button>
</div>
</center>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `document.querySelector("body > div > div.mt-3").innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Member
|
@copilot fix all |
…ck editor, package.json version
…nt, fallback interface stubs
This comment was marked as off-topic.
This comment was marked as off-topic.
Signed-off-by: Shan Wenxiao <seanoj_noreply@yeah.net>
Copilot
AI
changed the title
Use Monaco Editor and save code in submitpage.php when edit the content.
Fix always-false May 31, 2026
typeof h === 'number' check in Monaco adapter setSize
PythonSmall-Q
changed the title
typeof h === 'number' check in Monaco adapter setSize
Member
|
@sourcery-ai review |
Removed the creation of a custom style element for Monaco editor. Added error handling for disposing transient Monaco editors in various functions. Signed-off-by: zsTree <wa2025666@gmail.com>
Signed-off-by: zsTree <wa2025666@gmail.com>
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
createMonacoEditor,applyAutoFitreferences theeditorconst before it’s initialized (viatypeof editor !== 'undefined'), which will hit the temporal dead zone and can throw aReferenceError; consider passing the editor instance intoapplyAutoFitor closing over it only after initialization instead of touching it inside the pre‑init calls. - The
CodeMirror.fromTextAreashim replaces the original<textarea>with a div without copying over attributes likeid,name, or classes, which may break selectors or CSS that rely on the original element identity; consider preserving key attributes on the replacement container.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `createMonacoEditor`, `applyAutoFit` references the `editor` const before it’s initialized (via `typeof editor !== 'undefined'`), which will hit the temporal dead zone and can throw a `ReferenceError`; consider passing the editor instance into `applyAutoFit` or closing over it only after initialization instead of touching it inside the pre‑init calls.
- The `CodeMirror.fromTextArea` shim replaces the original `<textarea>` with a div without copying over attributes like `id`, `name`, or classes, which may break selectors or CSS that rely on the original element identity; consider preserving key attributes on the replacement container.
## Individual Comments
### Comment 1
<location path="XMOJ.user.js" line_range="5244" />
<code_context>
- }).setSize("100%", "auto");
+ }).setSize("100%", "730px");
}
+ document.getElementById("overlay").remove();
});
} else if (location.pathname == "/mail.php") {
</code_context>
<issue_to_address>
**issue (bug_risk):** Removing the `overlay` element without a null check can throw if it is missing.
If `document.getElementById('overlay')` returns `null`, calling `.remove()` will throw and stop the handler. Consider guarding the call, e.g.:
```js
const overlay = document.getElementById('overlay');
if (overlay && overlay.remove) overlay.remove();
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: zsTree <wa2025666@gmail.com>
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The logic for computing available viewport height from the header (in
createMonacoEditor, the diff editor shim, and the textarea fallback) is duplicated with slightly different implementations; consider extracting a shared helper to keep sizing behavior consistent and easier to maintain. - The Monaco loader currently hardcodes a specific CDN URL and version; it would be more flexible to centralize this configuration (e.g., at the top-level or behind a small config object) so it can be updated or overridden without touching multiple call sites.
- In the CodeMirror shim,
placeholderAdaptermethods are reassigned asynchronously after Monaco loads, which can be subtle and race-prone; consider returning a Promise-resolved adapter or exposing an explicitreadyPromise to make consumer code more robust and predictable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic for computing available viewport height from the header (in `createMonacoEditor`, the diff editor shim, and the textarea fallback) is duplicated with slightly different implementations; consider extracting a shared helper to keep sizing behavior consistent and easier to maintain.
- The Monaco loader currently hardcodes a specific CDN URL and version; it would be more flexible to centralize this configuration (e.g., at the top-level or behind a small config object) so it can be updated or overridden without touching multiple call sites.
- In the CodeMirror shim, `placeholderAdapter` methods are reassigned asynchronously after Monaco loads, which can be subtle and race-prone; consider returning a Promise-resolved adapter or exposing an explicit `ready` Promise to make consumer code more robust and predictable.
## Individual Comments
### Comment 1
<location path="XMOJ.user.js" line_range="5255" />
<code_context>
- }).setSize("100%", "auto");
+ }).setSize("100%", "730px");
}
+ document.getElementById("overlay").remove();
});
} else if (location.pathname == "/mail.php") {
</code_context>
<issue_to_address>
**issue:** Make overlay removal resilient if the element is missing
Accessing `#overlay` unconditionally can throw if it isn’t rendered on this page or has already been removed, which can break the `problem_std.php` flow. Consider guarding the call:
```js
const overlay = document.getElementById('overlay');
if (overlay && overlay.remove) overlay.remove();
```
This preserves current behavior when the overlay exists while preventing runtime errors when it doesn’t.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
PythonSmall-Q
requested changes
Jun 7, 2026
PythonSmall-Q
left a comment
PythonSmall-Q
left a comment
Member
There was a problem hiding this comment.
2026-06-07.154701.mp4
重交页面逻辑错误，不应该保留
多用cdnjs
Member
|
compared with jsdelivr we personally prefer cdnjs, and we have primarily switched to cdnjs
maybe you can ask @boomzero to see what he thinks
At 2026-06-11 00:43:11, "zsTree" ***@***.***> wrote:
@def-WA2025 commented on this pull request.
In XMOJ.user.js:
@@ -240,6 +240,319 @@ let GetUserBadge = async (Username) => {
}
}
};
+async function ensureMonaco() {
+ if (typeof monaco !== 'undefined') return;
+ const loaderUrl = ***@***.***/min/vs/loader.js';
我等下再改吧
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Member
Author
|
https://s4.zstatic.net/npm/monaco-editor@0.55.1/min/vs/loader.js I think it faster than cloudflare cdnjs |
Signed-off-by: zsTree <wa2025666@gmail.com>
Removed unused localStorage fallback logic for submission key. Signed-off-by: zsTree <wa2025666@gmail.com>
Member
Author
|
I used Cloudflare cdnjs now |
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There are many broad
try { ... } catch (e) {}blocks that fully swallow errors (especially around Monaco loading and layout); consider at least logging or gating these behind a debug flag so failures are diagnosable without overwhelming the console in normal use. - The
ensureMonacoloader assumes a globalrequire/AMD environment and hardcodes the Monaco CDN/version; it may be more robust to guard against conflictingrequireimplementations and to centralize the CDN/version into a single configuration spot or feature flag so it can be updated or overridden more easily. - The auto-fit height computation for the Monaco hosts relies on locating a header/nav element and can return
nullif the DOM structure is unexpected; it may be safer to provide a consistent minimum height fallback (not just in theMath.maxcall) when the header cannot be found orgetBoundingClientRectfails, to avoid very small or zero-height editors on atypical layouts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are many broad `try { ... } catch (e) {}` blocks that fully swallow errors (especially around Monaco loading and layout); consider at least logging or gating these behind a debug flag so failures are diagnosable without overwhelming the console in normal use.
- The `ensureMonaco` loader assumes a global `require`/AMD environment and hardcodes the Monaco CDN/version; it may be more robust to guard against conflicting `require` implementations and to centralize the CDN/version into a single configuration spot or feature flag so it can be updated or overridden more easily.
- The auto-fit height computation for the Monaco hosts relies on locating a header/nav element and can return `null` if the DOM structure is unexpected; it may be safer to provide a consistent minimum height fallback (not just in the `Math.max` call) when the header cannot be found or `getBoundingClientRect` fails, to avoid very small or zero-height editors on atypical layouts.
## Individual Comments
### Comment 1
<location path="XMOJ.user.js" line_range="5241-5250" />
<code_context>
+ if (!ParsedDocument.getElementsByClassName("jumbotron")[0].innerHTML.includes('No such Problem!')) {
</code_context>
<issue_to_address>
**issue:** Similarly guard against missing `.jumbotron` element on `problem_std.php`
This repeats the CE info pattern: `getElementsByClassName("jumbotron")[0].innerHTML` is used without confirming the element exists, so a missing/renamed `.jumbotron` will throw and break the standard solution view. Please first store the element, check it’s non-null, then access `.innerHTML` so minor HTML changes don’t cause runtime errors.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
PythonSmall-Q
approved these changes
Jun 12, 2026
PythonSmall-Q
left a comment
PythonSmall-Q
left a comment
Member
There was a problem hiding this comment.
if you wish, you may add console output of debug data
Enhanced error handling by adding console error logging and user alerts in DebugMode for various operations. Signed-off-by: zsTree <wa2025666@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR aim to accomplish?:
Use Monaco Editor and save code in submitpage.php when edit the content.
How does this PR accomplish the above?:
Editor -> Monaco
Save code in localStorage.
By submitting this pull request, I confirm the following:
git rebase)Summary by Sourcery
Replace the legacy CodeMirror integration with a Monaco-based editor abstraction and apply it to the submission page, including autosave and graceful fallbacks.
New Features:
Bug Fixes:
Enhancements:
Build:
Summary by Sourcery
Replace the legacy CodeMirror integration with a Monaco-based editor abstraction and apply it to the submission page, including autosave and safer error/standard-solution displays.
New Features:
Bug Fixes:
Enhancements:
Build: