Skip to content

Add web client Markdown rendering support#241

Merged
zarvox32 merged 8 commits intoXGDevGroup:mainfrom
mohammad-aloufi:feature/web-markdown-rendering
Apr 26, 2026
Merged

Add web client Markdown rendering support#241
zarvox32 merged 8 commits intoXGDevGroup:mainfrom
mohammad-aloufi:feature/web-markdown-rendering

Conversation

@mohammad-aloufi
Copy link
Copy Markdown
Contributor

Summary

This PR introduces native-vendored Markdown rendering for the web client to properly display help documents and formatted texts. Previously, the web client ignores the packet because of my previous merged PR which implemented markdown rendering for the desktop client.
The cause is that in the server side in pr #235 I had to add a flag that indicates markdown content and the web client didn't know what to do with it so it broke all kinds of edit boxes.

Now, web client has markdown support, and other edit boxes work again.

Key Changes

  • Dependency Automation: Vendored marked.min.js and purify.min.js directly into clients/web/libs/ to guarantee a self-contained, offline-compatible environment without relying on external CDNs.
  • Secure Markdown Viewer: Introduced the <dialog id="markdown-viewer-dialog"> component and markdown_viewer.js module. It safely parses raw markdown strings and aggressively sanitizes the output with DOMPurify to eliminate XSS vectors.
  • Packet Handling Hooks: Updated the request_input handler within app.js to automatically utilize .show() on the new viewer when content_format="markdown".
  • UI & Accessibility Styling: Added cohesive scoped CSS defaults for .markdown-body elements, rendering tables, code segments, blockquotes, and headings seamlessly across dark and light OS themes. Link targeting was mapped to open in new blank tabs, improving UX.
  • Schema Alignment: Modified server/tools/export_packet_schema.py to target the web client appropriately and regenerated the cross-repository schemas, eliminating strict validation errors for the newer content_format field.

Testing Notes

  • Open a game help document in the web client: It should open markdown rendered.

@zarvox32
Copy link
Copy Markdown
Contributor

Reviewed and tested locally — schema regeneration is stable, packet schema tests pass, marked produces correct HTML on a sample doc, and the flow matches the desktop client (main_window.py:2194-2202). LGTM overall; a few small nits worth cleaning up before merge:

  1. ALLOWED_SCHEMES is not a DOMPurify optionmarkdown_viewer.js:86 passes ALLOWED_SCHEMES: ['http', 'https', 'mailto'], but the supported key is ALLOWED_URI_REGEXP. It's silently ignored. Behavior happens to be correct because DOMPurify's default URI regex already allows those schemes, but the code reads like it enforces something it doesn't. Either drop the key or switch to ALLOWED_URI_REGEXP: /^(https?|mailto):/i.

  2. Incomplete escape in the fallback pathmarkdown_viewer.js:91 does rawMarkdown.replace(/</g, '&lt;'). Since the content lives inside a <pre>, < is the only character that can open a tag, so it's safe in practice — but it's worth also escaping & (before <) for correctness: .replace(/&/g, '&amp;').replace(/</g, '&lt;').

  3. Redundant ADD_ATTR: ['target'] — the post-process loop at markdown_viewer.js:97-101 sets target="_blank" and rel="noopener noreferrer" on every link after sanitization anyway, so allowing target through DOMPurify adds nothing. Safe to remove.

None are blocking — happy to merge as-is if you'd rather land the fix and tidy in a follow-up.

@mohammad-aloufi
Copy link
Copy Markdown
Contributor Author

All mentioned issues have been fixed now

@zarvox32
Copy link
Copy Markdown
Contributor

Analysis & Testing

Tests run locally:

  • pytest server/tests/test_packet_schemas.py — 49/49 pass
  • python tools/export_packet_schema.py — regenerates cleanly, no diff vs. committed files on all three targets
  • Pydantic round-trip of a markdown RequestInputPacket — OK

Findings:

  1. Test gap in server/tests/test_packet_schemas.py::test_packet_schema_files_are_up_to_date — it invokes the CLI with only --server-out and --client-out. --web-out now defaults to the real clients/web/packet_schema.json, so the tool writes to the working tree during the test. It's a no-op today (contents already match) but breaks test isolation. Recommend adding --web-out=tmp_path/web.json plus an assertion parallel to the other two.

  2. Dialog a11y#markdown-viewer-dialog has aria-label="Document Viewer" but the per-document prompt (<h2>) isn't wired via aria-labelledby, so the specific title (e.g. the game help doc name) isn't the dialog's accessible name. Minor; the prompt is still visible and focused content is reachable. Consider role="document" on the content element so screen readers enter reading mode automatically.

  3. Script loads are non-optional (index.html loadScript calls for marked/purify). If either 404s, bootstrap throws before app.js can show the "Markdown renderer failed to load" fallback path in markdown_viewer.js. Unlikely since they're vendored, but the in-viewer fallback is currently unreachable.

  4. Empty-markdown edge case — if content_format === "markdown" but default_value === "", the handler falls back to showInlineInput (editable). Likely intentional (empty doc → nothing to render) but worth confirming against desktop behavior.

Looks good:

  • Vendored libs are current: marked@15.0.12, DOMPurify@3.4.0
  • loadScript ordering guarantees marked/purify load before app.js
  • DOMPurify config (USE_PROFILES.html, schemes restricted to http/https/mailto) is XSS-safe
  • Packet handling mirrors the desktop client pattern from Add markdown rendering for read-only document viewer #235
  • CSS uses existing theme vars so dark/light themes work without extra work
  • Submit behavior (empty editbox response) matches desktop semantics for read-only doc views

Not verified here: live browser render of a help document (needs manual testing per the PR description).

@mohammad-aloufi
Copy link
Copy Markdown
Contributor Author

Done, fixed issues 1 to 3.
For number 4: both the web client and desktop client will show a "No documentation available." message when the clients gets an empty markdown document from the server.

The fixes done should help in making the rules document gets focused with a screen reader better which eliminates the 1 second wait before the document loads up for the screen reader on the web client.

@zarvox32 zarvox32 merged commit 0051cbb into XGDevGroup:main Apr 26, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants