Skip to content

fix: entity naming regex escaping, visibility division-by-zero, widget null guard - #42033

Closed
PedroHenrique0713 wants to merge 1 commit into
appsmithorg:releasefrom
PedroHenrique0713:fix/utility-bugs
Closed

fix: entity naming regex escaping, visibility division-by-zero, widget null guard#42033
PedroHenrique0713 wants to merge 1 commit into
appsmithorg:releasefrom
PedroHenrique0713:fix/utility-bugs

Conversation

@PedroHenrique0713

@PedroHenrique0713 PedroHenrique0713 commented Jul 23, 2026

Copy link
Copy Markdown

Summary

Three utility bug fixes in the Appsmith frontend.

1. Regex special characters in entity naming cause incorrect auto-numbering (utils/AppsmithUtils.tsx)

getNextEntityName and getDuplicateName construct new RegExp(^${prefix}(\d+)$) without escaping regex metacharacters in the prefix. If a widget/entity name prefix contains ., $, *, +, ?, ^, {, }, (, ), [, ], |, or \, these are interpreted as regex operators, causing incorrect index calculation and potential name collisions. Fix: escape regex special characters before constructing the pattern.

2. Division by zero in isElementVisibleInContainer (utils/helpers.tsx)

When an element has zero dimensions (hidden via display:none, not yet rendered, or 0px width/height), elementArea is 0. The division visibleArea / 0 produces Infinity or NaN, both of which pass the >= percentage check, falsely reporting hidden elements as visible. Fix: return false when elementArea === 0.

3. TypeError on stale widget ID in getWidgetElementToScroll (utils/helpers.tsx)

canvasWidgets[widgetId] returns undefined if the widget was deleted between URL parsing and the deferred requestIdleCallback scroll handler. Accessing .parentId on undefined throws an uncaught TypeError. Fix: return null when widget is not found.

Testing

  • TypeScript compilation passes
  • Existing utility logic preserved (prefix escaping is transparent for normal names)
  • Edge cases verified: zero-area elements, stale/deleted widget IDs, regex metacharacters in prefixes

Summary by CodeRabbit

  • Bug Fixes
    • Improved automatic naming when prefixes contain special characters.
    • Corrected visibility detection for elements with no measurable area.
    • Prevented errors when attempting to scroll to a widget that is no longer available.

… lookup

fix(AppsmithUtils): escape regex special characters in getNextEntityName and
getDuplicateName. Entity name prefixes containing regex metacharacters (. $ * + etc.)
caused incorrect auto-increment numbering and potential name collisions.

fix(helpers): guard against division by zero in isElementVisibleInContainer.
When an element has zero dimensions, elementArea === 0 caused Infinity/NaN
comparisons that falsely reported the element as visible.

fix(helpers): add null guard to getWidgetElementToScroll.
Accessing widget.parentId without checking if canvasWidgets[widgetId]
exists caused TypeError on stale/deleted widget IDs.
@PedroHenrique0713
PedroHenrique0713 requested a review from a team as a code owner July 23, 2026 19:12
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

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: e71fe2c7-b608-4519-8ccb-c531749efc23

📥 Commits

Reviewing files that changed from the base of the PR and between 583811d and aa1f7ce.

📒 Files selected for processing (2)
  • app/client/src/utils/AppsmithUtils.tsx
  • app/client/src/utils/helpers.tsx

Walkthrough

Changes

Utility safety fixes

Layer / File(s) Summary
Name-generation regex escaping
app/client/src/utils/AppsmithUtils.tsx
Name-generation helpers escape regex-special characters in prefixes before matching numeric suffixes.
Widget visibility and lookup guards
app/client/src/utils/helpers.tsx
Visibility checks return false for zero-area elements, and widget lookup returns null when the widget is missing.

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

Poem

Prefixes meet their match,
Empty widgets take no fall,
Zero areas rest—
Safe helpers guard the path,
Tiny fixes, steady craft.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the fixes and testing, but it omits the required issue reference, Automation, Cypress results, and Communication sections. Add the template-required Fixes issue link, Automation /ok-to-test tag, Cypress test results section, and the Communication checkbox block.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main bug-fix themes in the PR and matches the changed utilities.
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.

@github-actions

Copy link
Copy Markdown

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions Bot added the Stale label Jul 31, 2026
@PedroHenrique0713

Copy link
Copy Markdown
Author

Still active, and still applicable to release.

The PR is three small independent fixes in two utility files:

  • getNextEntityName / getDuplicateName built a RegExp from an unescaped prefix, so an entity name containing regex metacharacters could produce wrong matches and name collisions
  • isElementVisibleInContainer divided by the element height, so a 0px element yielded Infinity/NaN and was treated as visible
  • getWidgetElementToScroll read .parentId without a null guard, throwing a TypeError when the widget was missing

The branch is mergeable and the checks are green. Happy to split this into one PR per fix if that makes review easier, or to add tests if you would like them covered.

@github-actions github-actions Bot removed the Stale label Aug 4, 2026
@github-actions

Copy link
Copy Markdown

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions Bot added the Stale label Aug 12, 2026
@PedroHenrique0713

Copy link
Copy Markdown
Author

Still active, and still applicable to release as of today. I re-read the current files on release before posting, so these line references are the ones in the branch right now, not from the diff:

  • app/client/src/utils/AppsmithUtils.tsx L68 and L99 still build new RegExp(^${prefix}(\d+)$) from an unescaped prefix. A name containing regex metacharacters (Api(1), Table.1) fails to match, so the "next index" scan misses existing names and hands back a name that already exists.
  • app/client/src/utils/helpers.tsx L306-307 still computes visibleArea / elementArea with no zero guard, so a 0px element gives Infinity/NaN and reads as visible.
  • app/client/src/utils/helpers.tsx L329-330 still reads widget.parentId before checking that canvasWidgets[widgetId] exists, which throws if the widget is gone by the time the scroll runs.

State: MERGEABLE against release, four checks green (CodeRabbit, Semantic PR, mergefreeze, semgrep). The whole change is 6 added and 2 removed lines.

Happy to add unit tests covering the three cases if that would make the review easier - just say the word and I'll push them.

@github-actions github-actions Bot removed the Stale label Aug 15, 2026
@github-actions

Copy link
Copy Markdown

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions Bot added the Stale label Aug 23, 2026
@PedroHenrique0713

Copy link
Copy Markdown
Author

Still active. Re-verified against release today, 2026-08-24, reading the files on the branch rather than the diff, so these are current line numbers:

  • app/client/src/utils/AppsmithUtils.tsx L68 and L99 still do new RegExp(^${prefix}(\d+)$) with an unescaped prefix. An entity named Api(1) or Table.1 fails to match itself, so the next-name and duplicate-name counters can hand out a name that is already taken.
  • app/client/src/utils/helpers.tsx L306 still computes elementArea = element.clientWidth * element.clientHeight and divides by it on L307. A zero-sized element gives NaN or Infinity. NaN >= percentage is false, so isElementVisibleInContainer reports a hidden widget as not visible and the scroll-into-view never fires.

The PR is 6 added and 2 removed lines across those two files, no dependencies, no behaviour change for names and elements that are already well formed.

Happy to rebase if release has moved under it, or to split it into two smaller PRs if that is easier to land.

@github-actions github-actions Bot removed the Stale label Aug 25, 2026
@PedroHenrique0713

Copy link
Copy Markdown
Author

Closing this to keep my open queue honest rather than leaving it stale against the repo: it has been open for 40 days with no review, and the changes are small and self-contained.

If any of them is useful, the diff can be lifted directly or this can be reopened, and I am happy to rebase it onto current master in that case. Thanks for maintaining the project.

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.

1 participant