Skip to content

fix: resolve 2 bugs in Editron - #604

Closed
saurabhhhcodes wants to merge 1 commit into
piyushdotcomm:mainfrom
saurabhhhcodes:fix/Editron-91140
Closed

fix: resolve 2 bugs in Editron#604
saurabhhhcodes wants to merge 1 commit into
piyushdotcomm:mainfrom
saurabhhhcodes:fix/Editron-91140

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Aug 4, 2026

Copy link
Copy Markdown

Description

This PR fixes real bugs found in the codebase:

  • Replaced innerHTML assignment with textContent: prevents HTML injection / XSS and is faster since it does not parse markup.
  • Replaced innerHTML assignment with textContent: prevents HTML injection / XSS and is faster since it does not parse markup.

Type of Change

  • Bug fix (non-breaking change fixing an issue)

How Has This Been Tested?

  • Local manual testing

Checklist

  • My code follows the style guidelines
  • I have performed a self-review

Related Issue

Ref: #603

Summary by CodeRabbit

  • Bug Fixes
    • Improved counter display updates for safer and more reliable text rendering.
    • Applied the fix consistently across the tutorial example and starter template.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

👋 Thanks for opening a PR, @saurabhhhcodes!

Your PR has entered the 🚦 PR Review Pipeline.

Standard PR detected — your PR will follow the standard review pipeline.


What happens next

Stage Reviewer Checks
Stage 1 — Automated Validation 🤖 Bot DCO · Format · AI/Slop · Duplicate
Stage 2 — Human Review 👥 Maintainer Code + Quality Review
Stage 3 — PA / Maintainer Review 🔑 Project Admin Final Merge Decision

A pipeline status comment will appear below and update automatically as your PR progresses.


While you wait

  • Sign all commits (git commit -s)
  • Link your issue (Closes #123)
  • Use a feature branch (not main)
  • Avoid unrelated changes

This comment is posted only once.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

🔁 Possible Duplicate PR Detected

Hi @saurabhhhcodes, a PR with a very similar title already exists:

➡️ #599

Please check if your changes overlap with the existing PR.

This is just a heads-up. The PR will not be auto-closed.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f4ee8f4-98b9-4197-adca-9288f4df116d

📥 Commits

Reviewing files that changed from the base of the PR and between 4ffe26f and 348e17c.

📒 Files selected for processing (2)
  • editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_solution/counter.js
  • editron-starters/tutorialkit/src/templates/default/counter.js

Walkthrough

Both setCounter implementations now update the counter display with textContent instead of innerHTML.

Changes

Counter rendering

Layer / File(s) Summary
Plain-text counter updates
editron-starters/tutorialkit/src/content/tutorial/.../_solution/counter.js, editron-starters/tutorialkit/src/templates/default/counter.js
Both setCounter implementations now assign the counter value through textContent.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

Suggested reviewers: piyushdotcomm

Poem

A rabbit checks the counter’s light,
Plain text makes each number right.
No HTML burrows through the display,
The starter code hops on its way.
🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: fixing two Editron bugs.
Description check ✅ Passed The description explains the fix, identifies it as a bug fix, references issue #603, and records manual testing despite template differences.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix unsafe DOM updates in Editron counter templates

🐞 Bug fix 🕐 Less than 5 minutes

Grey Divider

AI Description

• Replace innerHTML with textContent to prevent HTML injection/XSS.
• Apply the fix to both the tutorial solution and default template counters.
Diagram

graph TD
A["src/templates/default/counter.js"] --> C(["setupCounter()"]) 
B["src/content/.../1-welcome/_solution/counter.js"] --> C 
C --> D(["setCounter(count)"]) --> E["element.textContent = 'count is …'"]
C --> F["click event listener"] --> D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep innerHTML but escape/sanitize output
  • ➕ Supports intentional markup rendering if needed later
  • ➕ Can be applied generically across multiple templates
  • ➖ Adds dependency/maintenance burden (or error-prone custom escaping)
  • ➖ Still slower than textContent due to HTML parsing
  • ➖ Overkill when rendering plain text only

Recommendation: Use textContent (current approach) since the content is plain text and this eliminates XSS risk and HTML parsing overhead. Consider sanitization only if the UI genuinely needs to render trusted markup.

Files changed (2) +2 / -2

Bug fix (2) +2 / -2
counter.jsUse textContent for counter rendering in tutorial solution +1/-1

Use textContent for counter rendering in tutorial solution

• Replaces 'element.innerHTML' with 'element.textContent' when updating the counter label. This prevents HTML injection and avoids unnecessary markup parsing.

editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_solution/counter.js

counter.jsUse textContent for counter rendering in default template +1/-1

Use textContent for counter rendering in default template

• Replaces 'element.innerHTML' with 'element.textContent' for the starter template counter label. This aligns with safe DOM update practices and removes XSS risk from this path.

editron-starters/tutorialkit/src/templates/default/counter.js

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 22 rules

Grey Divider


Remediation recommended

1. Lesson solution mismatch 🐞 Bug ⚙ Maintainability
Description
The Welcome lesson content and starter file still instruct updating innerHTML, but the lesson
solution now uses textContent, so the written guidance/code sample no longer matches the provided
solution. This can confuse learners and can cause “expected solution” drift if the platform compares
against _solution output/code.
Code

editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_solution/counter.js[6]

+    element.textContent = `count is ${counter}`;
Relevance

●●● Strong

Team often accepts fixes aligning stale/mismatched copy/docs with current behavior to avoid
confusion/drift.

PR-#102
PR-#157

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The lesson’s written instructions and displayed snippet explicitly reference innerHTML, and the
starter file also uses innerHTML, but the solution file now uses textContent, so these artifacts
are no longer aligned.

editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/content.md[19-34]
editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_files/counter.js[1-10]
editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_solution/counter.js[1-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The lesson text and starter code still reference `innerHTML`, while the updated solution uses `textContent`, creating inconsistent instructional materials.

## Issue Context
This PR changed the solution implementation to use `textContent`.

## Fix Focus Areas
- editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/content.md[19-34]
- editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_files/counter.js[1-10]
- editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_solution/counter.js[1-12]

## What to change
- Update `content.md` prose and the embedded code block to use `textContent` instead of `innerHTML` (and adjust wording accordingly).
- Consider updating `_files/counter.js` to also use `textContent` if the goal is to teach the safer pattern consistently (or alternatively revert `_solution/counter.js` back to `innerHTML` if that is intentional for the lesson).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

const setCounter = (count) => {
counter = count;
element.innerHTML = `count is ${counter}`;
element.textContent = `count is ${counter}`;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Lesson solution mismatch 🐞 Bug ⚙ Maintainability

The Welcome lesson content and starter file still instruct updating innerHTML, but the lesson
solution now uses textContent, so the written guidance/code sample no longer matches the provided
solution. This can confuse learners and can cause “expected solution” drift if the platform compares
against _solution output/code.
Agent Prompt
## Issue description
The lesson text and starter code still reference `innerHTML`, while the updated solution uses `textContent`, creating inconsistent instructional materials.

## Issue Context
This PR changed the solution implementation to use `textContent`.

## Fix Focus Areas
- editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/content.md[19-34]
- editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_files/counter.js[1-10]
- editron-starters/tutorialkit/src/content/tutorial/1-basics/1-introduction/1-welcome/_solution/counter.js[1-12]

## What to change
- Update `content.md` prose and the embedded code block to use `textContent` instead of `innerHTML` (and adjust wording accordingly).
- Consider updating `_files/counter.js` to also use `textContent` if the goal is to teach the safer pattern consistently (or alternatively revert `_solution/counter.js` back to `innerHTML` if that is intentional for the lesson).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@github-actions github-actions Bot mentioned this pull request Aug 4, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants