Skip to content

fix(utils): add null safety guards to canvasPixelRatio - #7974

Open
karankumar1106 wants to merge 1 commit into
sugarlabs:masterfrom
karankumar1106:refactor/canvas-pixels-ratio-null-safety
Open

fix(utils): add null safety guards to canvasPixelRatio#7974
karankumar1106 wants to merge 1 commit into
sugarlabs:masterfrom
karankumar1106:refactor/canvas-pixels-ratio-null-safety

Conversation

@karankumar1106

Copy link
Copy Markdown
Contributor

Description

Refactors canvasPixelRatio() in js/utils/browser-utils.js to include null-safety guards for missing #myCanvas DOM element or getContext("2d") returning null.

Previously, canvasPixelRatio() executed document.querySelector("#myCanvas").getContext("2d") directly. If #myCanvas was not present in the DOM (e.g. before full DOM load or in headless test/embedded canvas environments), calling .getContext("2d") threw an unhandled TypeError: Cannot read properties of null (reading 'getContext').

Related Issue

N/A

PR Category

  • Bug Fix — Fixes a bug or incorrect behavior
  • Feature — Adds new functionality
  • Performance — Improves performance (load time, memory, rendering, etc.)
  • Tests — Adds or updates test coverage
  • Documentation — Updates to docs, comments, or README
  • Chore / Refactor — Maintenance, cleanup, or refactoring with no behavior change
  • CI/CD — Changes to CI/CD workflows and automation

Changes Made

  • js/utils/browser-utils.js:
    • Updated canvasPixelRatio() with null checks for document.querySelector("#myCanvas") and canvas.getContext("2d"), returning devicePixelRatio as a fallback.
  • js/utils/__tests__/browser-utils.test.js:
    • Added unit test verifying canvasPixelRatio() safely returns devicePixelRatio when #myCanvas is not present in DOM.
    • Added unit test verifying canvasPixelRatio() safely returns devicePixelRatio when canvas.getContext("2d") returns null.

Testing Performed

  • Ran local Jest test suite: npx.cmd jest js/utils/__tests__/browser-utils.test.js --coverage=false
  • Verified all 39 unit tests pass cleanly (39/39 passed).

Checklist

  • I have tested these changes locally and they work as expected.
  • I have added/updated tests that prove the effectiveness of these changes.
  • I have updated the documentation to reflect these changes, if applicable.
  • I have followed the project's coding style guidelines.
  • I have run npm run lint and npx prettier --check . with no errors.
  • I have addressed the code review feedback from the previous submission, if applicable.
  • I have enabled "Allow edits from maintainers" (required for auto-rebase; this only affects the PR branch, not your fork).

Additional Notes for Reviewers

None.

@github-actions github-actions Bot added bug fix Fixes a bug or incorrect behavior tests Adds or updates test coverage chore Maintenance, refactoring, or tooling with no behavior change size/S Small: 10-49 lines changed area/javascript Changes to JS source files area/tests Changes to test files labels Aug 5, 2026
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.06%. Comparing base (5462731) to head (9eda17a).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7974   +/-   ##
=======================================
  Coverage   62.05%   62.06%           
=======================================
  Files         187      187           
  Lines       58377    58382    +5     
=======================================
+ Hits        36227    36232    +5     
  Misses      22150    22150           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@karankumar1106

Copy link
Copy Markdown
Contributor Author

Hi @walterbender Just following up on this PR. When you have time, could you please review it? I'm happy to make any changes if needed. Thanks for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files bug fix Fixes a bug or incorrect behavior chore Maintenance, refactoring, or tooling with no behavior change size/S Small: 10-49 lines changed tests Adds or updates test coverage

Projects

Development

Successfully merging this pull request may close these issues.

1 participant