Conversation
Remove unnecessary backslash escapes: double quotes in template literals and over-escaped characters in regex character classes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
These languages are pending human review and will be added back once reviewed. Removes dead loadSavedLanguage/applySavedLanguageTranslations functions and the corresponding branches in setLanguage. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Rename second pin_h_option (Gesture: ScreenDown) to pin_g_option in
all locale files; update sensing.js dropdown to use getOption("pin_g")
with value "g" — restoring the previously shadowed "Logo pressed" entry
- Remove duplicate __fonts_FreeSans_Bold_json_option in sv/pl/pt (identical value)
- Remove duplicate loading_success_ui in fr (keep the more complete translation)
- Remove duplicate about_heading_ui in fr (identical value)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- api/xr.js: import translate from translation.js - blocks/blocks.js: import categoryColours from toolbox.js - main/input.js: import Blockly; promote announceToScreenReader to exported module-level function (was nested inside setupInput) - main/blockhandling.js: import announceToScreenReader from input.js - ui/gizmos.js: replace undefined areGizmosEnabled guard with direct call (function never existed; typeof guard always fell back to true); use window.selectedColor consistently Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request consolidates multiple updates across translation files, block definitions, and utility modules. Changes include renaming a gesture option key from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
main/translation.js (2)
111-113:⚠️ Potential issue | 🟠 MajorPotential runtime error when selecting unsupported languages.
If a user selects a language not in the
translationsobject (e.g., "fr" which is commented out),translations[currentLanguage]will beundefined, andObject.keys(undefined)will throw aTypeError.Consider adding a guard:
🐛 Proposed fix to guard against undefined translations
- // Apply custom translations for the selected language - Object.keys(translations[currentLanguage]).forEach((key) => { - Blockly.Msg[key] = translations[currentLanguage][key]; - }); + // Apply custom translations for the selected language + const currentTranslations = translations[currentLanguage]; + if (currentTranslations) { + Object.keys(currentTranslations).forEach((key) => { + Blockly.Msg[key] = currentTranslations[key]; + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/translation.js` around lines 111 - 113, The loop that assigns Blockly.Msg from translations uses Object.keys(translations[currentLanguage]) which will throw if translations[currentLanguage] is undefined; update the assignment logic in the block where Object.keys(...) is called (the loop referencing translations and currentLanguage) to first guard that translations[currentLanguage] exists (e.g., use a fallback object like translations[currentLanguage] || translations['en'] || {} or skip assignment and warn) and only iterate keys when that object is defined, ensuring Blockly.Msg is not updated from undefined and optionally logging a warning about unsupported currentLanguage.
86-106:⚠️ Potential issue | 🟠 MajorReduce language support to English and Spanish only—UI mismatch creates broken user experience.
The
setLanguagefunction now only activates Spanish (es) translations; all other languages fall back to English. This is confirmed by the commented-out imports (lines 10-25) and the reducedtranslationsobject (lines 31-40). However,index.htmlstill presents language menu options for French, Italian, Swedish, Portuguese, Polish, and German (lines 455-490).Users selecting these unavailable languages will silently receive English without any indication the selection failed—a UX regression. Either restore support for these languages or remove their menu options from the HTML and clarify whether this is intentional given the PR title is "Linting updates".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@main/translation.js` around lines 86 - 106, The language selector currently only applies Spanish ("es") and otherwise forces English in setLanguage (the translations object and the language switch block in main/translation.js), while index.html still shows other language options—fix by either restoring the missing locale imports and entries in translations so setLanguage supports French/Italian/Swedish/Portuguese/Polish/German (re-add locale modules and their keys used by translations), or remove those language menu entries from index.html and update the UI flow: change setLanguage to detect unsupported codes and show a clear user-facing message (toast/alert) when a user picks an unavailable language; reference the setLanguage function, the translations mapping in main/translation.js, and the language menu in index.html when making edits.scripts/check-link-security.mjs (2)
88-88:⚠️ Potential issue | 🟡 MinorRegex pattern cannot handle nested parentheses in
window.openarguments, but no such calls exist in the codebase.The pattern
/window\.open\s*\(([^)]*)\)/guses[^)]*, which would fail to matchwindow.opencalls with nested parentheses (e.g.,window.open(url, name, getFeatures())).Search found only two
window.opencalls in the codebase:
ui/designview.js:7:window.open("https://flockxr.com/", "_blank", "noopener,noreferrer");main/menu.js:379:window.open("https://hub.flockxr.com/", "_blank", "noopener,noreferrer");Both use only string literal arguments with no nested parentheses. The script includes a sophisticated
splitTopLevelArgs()function (lines 34–84) that properly handles nested structures by tracking depth and quotes, so even if future code introduces such calls, consider whether a more robust regex or a proper JavaScript parser would improve reliability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-link-security.mjs` at line 88, The regex openRegex using /window\.open\s*\(([^)]*)\)/g fails on nested parentheses; update the detection to use the existing robust splitTopLevelArgs() approach instead of relying on that fragile regex (or switch to a JS parser) — locate openRegex in scripts/check-link-security.mjs and replace the simple capturing-regex usage with logic that finds "window.open(" and then extracts the argument substring by walking characters and feeding it into splitTopLevelArgs(), ensuring nested parentheses and quoted strings are handled correctly.
16-16:⚠️ Potential issue | 🔴 CriticalRegex pattern misses single-quoted and unquoted
relattributes.The pattern
/\brel\s*=\s*"([^"]*)"/ionly matches double-quoted attributes, missing valid HTML forms likerel='noopener'andrel=noopener. This allows security violations to go undetected.The proposed fix in the review handles single quotes but still misses unquoted attribute values. Use this more complete pattern instead:
🔒 Complete fix for all valid quote styles
- const relMatch = tag.match(/\brel\s*=\s*"([^"]*)"/i); + const relMatch = tag.match(/\brel\s*=\s*(?:"([^"]*)"|'([^']*)'|(\S+))/i); + const relValue = (relMatch?.[1] || relMatch?.[2] || relMatch?.[3])?.toLowerCase();Note: Update line 25 to extract the matched group correctly from the new pattern structure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-link-security.mjs` at line 16, The current tag.match call using relMatch only handles double-quoted rel attributes; update the regex used in tag.match (the expression assigned to relMatch) to accept double-quoted, single-quoted, or unquoted attribute values (i.e. rel=\s*(double-quoted | single-quoted | unquoted token)), then adjust the code that reads the captured group (where you currently extract the rel value from relMatch) to use the correct capture index for the new pattern so single-quoted and unquoted rel attributes are detected as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@main/translation.js`:
- Around line 111-113: The loop that assigns Blockly.Msg from translations uses
Object.keys(translations[currentLanguage]) which will throw if
translations[currentLanguage] is undefined; update the assignment logic in the
block where Object.keys(...) is called (the loop referencing translations and
currentLanguage) to first guard that translations[currentLanguage] exists (e.g.,
use a fallback object like translations[currentLanguage] || translations['en']
|| {} or skip assignment and warn) and only iterate keys when that object is
defined, ensuring Blockly.Msg is not updated from undefined and optionally
logging a warning about unsupported currentLanguage.
- Around line 86-106: The language selector currently only applies Spanish
("es") and otherwise forces English in setLanguage (the translations object and
the language switch block in main/translation.js), while index.html still shows
other language options—fix by either restoring the missing locale imports and
entries in translations so setLanguage supports
French/Italian/Swedish/Portuguese/Polish/German (re-add locale modules and their
keys used by translations), or remove those language menu entries from
index.html and update the UI flow: change setLanguage to detect unsupported
codes and show a clear user-facing message (toast/alert) when a user picks an
unavailable language; reference the setLanguage function, the translations
mapping in main/translation.js, and the language menu in index.html when making
edits.
In `@scripts/check-link-security.mjs`:
- Line 88: The regex openRegex using /window\.open\s*\(([^)]*)\)/g fails on
nested parentheses; update the detection to use the existing robust
splitTopLevelArgs() approach instead of relying on that fragile regex (or switch
to a JS parser) — locate openRegex in scripts/check-link-security.mjs and
replace the simple capturing-regex usage with logic that finds "window.open("
and then extracts the argument substring by walking characters and feeding it
into splitTopLevelArgs(), ensuring nested parentheses and quoted strings are
handled correctly.
- Line 16: The current tag.match call using relMatch only handles double-quoted
rel attributes; update the regex used in tag.match (the expression assigned to
relMatch) to accept double-quoted, single-quoted, or unquoted attribute values
(i.e. rel=\s*(double-quoted | single-quoted | unquoted token)), then adjust the
code that reads the captured group (where you currently extract the rel value
from relMatch) to use the correct capture index for the new pattern so
single-quoted and unquoted rel attributes are detected as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c069b692-9b98-41f0-b1ec-094689b9158a
📒 Files selected for processing (18)
api/xr.jsblocks/blocks.jsblocks/condition.jsblocks/events.jsblocks/sensing.jslocale/en.jslocale/es.jslocale/fr.jslocale/it.jslocale/pl.jslocale/pt.jslocale/sv.jsmain/blockhandling.jsmain/input.jsmain/translation.jsscripts/check-link-security.mjsscripts/utils/parse-jsdoc.mjsui/gizmos.js
| disableGizmos(); | ||
| e.stopPropagation(); // avoid duplicate handlers upstream | ||
| // don't e.preventDefault() globally unless you *need* to stop other Esc behavior |
There was a problem hiding this comment.
Unconditional capture-phase Escape propagation stop breaks other key handlers
On Line 68, e.stopPropagation() now runs for every plain Escape, and because this listener runs in capture phase (Line 79), other Escape handlers won’t receive the event. That can break modal/dialog/feature-specific Escape behavior outside gizmos.
💡 Proposed fix
try {
- disableGizmos();
- e.stopPropagation(); // avoid duplicate handlers upstream
+ const hadActiveGizmo =
+ Boolean(gizmoManager?.positionGizmoEnabled) ||
+ Boolean(gizmoManager?.rotationGizmoEnabled) ||
+ Boolean(gizmoManager?.scaleGizmoEnabled) ||
+ Boolean(gizmoManager?.boundingBoxGizmoEnabled) ||
+ Boolean(gizmoManager?.selectGizmoEnabled) ||
+ colorPickingKeyboardMode;
+
+ disableGizmos();
+ if (hadActiveGizmo) {
+ e.stopPropagation(); // avoid duplicate handlers only when we actually handled gizmo state
+ }
// don't e.preventDefault() globally unless you *need* to stop other Esc behavior
} catch {Also applies to: 79-79
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Localization