Open
Conversation
Contributor
Reviewer's GuideThis PR internationalizes key user-facing messages and placeholders across the Scrum Helper popup, GitHub/GitLab helpers, and report generation flows by routing them through chrome.i18n with safe English fallbacks, and adds the corresponding translation entries for all supported locales. Sequence diagram for localized GitHub user fetch error handlingsequenceDiagram
actor User
participant Popup
participant ScrumHelper
participant GitHubAPI
participant ChromeI18n as chrome_i18n
User->>Popup: Click generate report
Popup->>ScrumHelper: allIncluded(outputTarget)
ScrumHelper->>GitHubAPI: fetch(userUrl)
GitHubAPI-->>ScrumHelper: 404 Not Found
ScrumHelper->>ChromeI18n: getMessage(githubUserNotFoundError, [platformUsernameLocal])
ChromeI18n-->>ScrumHelper: localizedMessage or null
alt localizedMessage available
ScrumHelper->>ScrumHelper: errorMsg = localizedMessage
else no localizedMessage
ScrumHelper->>ScrumHelper: errorMsg = English fallback string
end
ScrumHelper->>Popup: throw Error(errorMsg)
Popup-->>User: Show localized error in UI
Sequence diagram for localized invalid token handling in popupsequenceDiagram
actor User
participant Popup
participant ScrumHelper
participant ChromeI18n as chrome_i18n
User->>Popup: Trigger action requiring GitHub token
Popup->>ScrumHelper: allIncluded(outputTarget = popup)
ScrumHelper->>ScrumHelper: detect invalid or expired token
ScrumHelper->>ChromeI18n: getMessage(invalidTokenError)
ChromeI18n-->>ScrumHelper: localizedMessage or null
alt localizedMessage available
ScrumHelper->>ScrumHelper: errMsg = localizedMessage
else no localizedMessage
ScrumHelper->>ScrumHelper: errMsg = English fallback
end
alt scrumReport element exists
ScrumHelper->>Popup: Set innerHTML with errMsg
ScrumHelper->>Popup: Re-enable generateReport button
else no scrumReport element
ScrumHelper->>User: alert(errMsg)
end
Flow diagram for chrome.i18n message resolution with English fallbackflowchart TD
A[Need user-facing message] --> B[Call chrome.i18n.getMessage with key and optional args]
B --> C{chrome and i18n<br>available?}
C -- No --> F[Use hardcoded English fallback string]
C -- Yes --> D{getMessage<br>returned value?}
D -- Has value --> E[Use localized message]
D -- Null or empty --> F[Use hardcoded English fallback string]
E --> G[Render message in UI or throw Error]
F --> G[Render message in UI or throw Error]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
bot
added
javascript
![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 14 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
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
scrumReport.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
reportDiv.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link)
General comments:
- In
showInvalidTokenMessage,errMsgis assigned without aconst/letdeclaration, which will create/overwrite a global variable and can throw in strict mode—declare it locally (const errMsg = ...) to avoid that. - There are many repeated snippets building the same styled error
<div>with i18n fallbacks; consider extracting a small helper (e.g.,getErrorMessage(key, fallback, args)orrenderError(container, key, fallback, args)) to reduce duplication and make future changes to error styling/messages safer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `showInvalidTokenMessage`, `errMsg` is assigned without a `const`/`let` declaration, which will create/overwrite a global variable and can throw in strict mode—declare it locally (`const errMsg = ...`) to avoid that.
- There are many repeated snippets building the same styled error `<div>` with i18n fallbacks; consider extracting a small helper (e.g., `getErrorMessage(key, fallback, args)` or `renderError(container, key, fallback, args)`) to reduce duplication and make future changes to error styling/messages safer.
## Individual Comments
### Comment 1
<location path="src/scripts/scrumHelper.js" line_range="941" />
<code_context>
verifyCacheStatus();
function showInvalidTokenMessage() {
+ errMsg = chrome?.i18n.getMessage('invalidTokenError') || 'Invalid or expired GitHub token. Please check your token in the Scrum Helper settings and try again.';
if (outputTarget === 'popup') {
const reportDiv = document.getElementById('scrumReport');
</code_context>
<issue_to_address>
**issue (bug_risk):** Declare `errMsg` locally to avoid leaking a global variable.
This assignment creates/overwrites a global `errMsg` in non-strict mode, which can cause subtle conflicts elsewhere. Declare it locally, e.g. `const errMsg = ...` inside `showInvalidTokenMessage`.
</issue_to_address>
### Comment 2
<location path="src/scripts/scrumHelper.js" line_range="179-180" />
<code_context>
scrumReport.innerHTML =
'<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">' + (chrome?.i18n.getMessage('usernameRequiredError') || 'Please enter your username to generate a report.') + '</div>';
</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="src/scripts/scrumHelper.js" line_range="179-180" />
<code_context>
scrumReport.innerHTML =
'<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">' + (chrome?.i18n.getMessage('usernameRequiredError') || 'Please enter your username to generate a report.') + '</div>';
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 4
<location path="src/scripts/scrumHelper.js" line_range="265" />
<code_context>
scrumReport.innerHTML = `<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${err.message || chrome?.i18n.getMessage('gitlabFetchingError') || 'An error occurred while fetching GitLab data.'}</div>`;
</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 5
<location path="src/scripts/scrumHelper.js" line_range="265" />
<code_context>
scrumReport.innerHTML = `<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${err.message || chrome?.i18n.getMessage('gitlabFetchingError') || 'An error occurred while fetching GitLab data.'}</div>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 6
<location path="src/scripts/scrumHelper.js" line_range="313" />
<code_context>
scrumReport.innerHTML = `<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${err.message || chrome?.i18n.getMessage('gitlabFetchingError') || 'An error occurred while fetching GitLab data.'}</div>`;
</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 7
<location path="src/scripts/scrumHelper.js" line_range="313" />
<code_context>
scrumReport.innerHTML = `<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${err.message || chrome?.i18n.getMessage('gitlabFetchingError') || 'An error occurred while fetching GitLab data.'}</div>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 8
<location path="src/scripts/scrumHelper.js" line_range="325-326" />
<code_context>
scrumReport.innerHTML =
`<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${chrome?.i18n.getMessage('usernameRequiredError') || 'Please enter your username to generate a report.'}</div>`;
</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 9
<location path="src/scripts/scrumHelper.js" line_range="325-326" />
<code_context>
scrumReport.innerHTML =
`<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${chrome?.i18n.getMessage('usernameRequiredError') || 'Please enter your username to generate a report.'}</div>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 10
<location path="src/scripts/scrumHelper.js" line_range="340-341" />
<code_context>
scrumReport.innerHTML =
`<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${chrome?.i18n.getMessage('unknownPlatformError') || 'Unknown platform selected.'}</div>`;
</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 11
<location path="src/scripts/scrumHelper.js" line_range="340-341" />
<code_context>
scrumReport.innerHTML =
`<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${chrome?.i18n.getMessage('unknownPlatformError') || 'Unknown platform selected.'}</div>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 12
<location path="src/scripts/scrumHelper.js" line_range="766" />
<code_context>
scrumReport.innerHTML = `<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${errorMsg}</div>`;
</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 13
<location path="src/scripts/scrumHelper.js" line_range="766" />
<code_context>
scrumReport.innerHTML = `<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${errorMsg}</div>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `scrumReport.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 14
<location path="src/scripts/scrumHelper.js" line_range="945-946" />
<code_context>
reportDiv.innerHTML =
`<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${errMsg}</div>`;
</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 15
<location path="src/scripts/scrumHelper.js" line_range="945-946" />
<code_context>
reportDiv.innerHTML =
`<div class="error-message" style="color: #dc2626; font-weight: bold; padding: 10px;">${errMsg}</div>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `reportDiv.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.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves internationalization across the extension by translating previously hard-coded UI placeholders and user-facing error/status messages (addressing #422), while keeping English fallbacks when translations are unavailable.
Changes:
- Replaced several hard-coded user-facing strings in the popup and report generation flows with
chrome.i18n.getMessage(...)lookups (with English fallback). - Updated
popup.htmlinputs to usedata-i18n-placeholderso placeholders are set via the existing i18n mechanism. - Added many new i18n message keys across locales for common errors/status messages.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scripts/scrumHelper.js | Uses i18n-backed error messages during report generation and token validation. |
| src/scripts/popup.js | Uses i18n-backed status/error strings in popup UI (display mode notice, repo filtering/loading, etc.). |
| src/scripts/gitlabHelper.js | Throws i18n-backed errors for GitLab fetch failures. |
| src/popup.html | Switches certain input placeholders to data-i18n-placeholder for translation. |
| src/_locales/en/messages.json | Adds new English message keys (and improves tooltip metadata). |
| src/_locales/de/messages.json | Adds German translations for new message keys. |
| src/_locales/es/messages.json | Adds Spanish translations for new message keys. |
| src/_locales/fr/messages.json | Adds French translations for new message keys. |
| src/_locales/he/messages.json | Adds Hebrew translations for new message keys. |
| src/_locales/hi/messages.json | Adds Hindi translations for new message keys. |
| src/_locales/id/messages.json | Adds Indonesian translations for new message keys. |
| src/_locales/it/messages.json | Adds Italian translations for new message keys. |
| src/_locales/ja/messages.json | Adds Japanese translations for new message keys. |
| src/_locales/ml/messages.json | Adds Malayalam translations for new message keys. |
| src/_locales/my/messages.json | Adds Burmese translations for new message keys. |
| src/_locales/nb/messages.json | Adds Norwegian Bokmål translations for new message keys. |
| src/_locales/pt/messages.json | Adds Portuguese translations for new message keys. |
| src/_locales/pt_BR/messages.json | Adds Brazilian Portuguese translations for new message keys. |
| src/_locales/ru/messages.json | Adds Russian translations for new message keys. |
| src/_locales/te/messages.json | Adds Telugu translations for new message keys. |
| src/_locales/uk/messages.json | Adds Ukrainian translations for new message keys. |
| src/_locales/vi/messages.json | Adds Vietnamese translations for new message keys. |
| src/_locales/zh_CN/messages.json | Adds Simplified Chinese translations for new message keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Author
|
Hey @vedansh-5, thanks for the feedback! I completely missed the new Copilot suggestions, sorry about that. Hope it looks good to you now |
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.
📌 Fixes
Fixes #422
📝 Summary of Changes
✅ Checklist
Summary by Sourcery
Localize user-facing error and status messages and hook them into the browser i18n system with fallbacks.
Bug Fixes:
Enhancements: