Skip to content

Bug fix: when calling text() with x as null - #1702

Merged
blikblum merged 5 commits into
foliojs:masterfrom
culiutudousi:patch-1
Mar 17, 2026
Merged

blikblum merged 5 commits into
foliojs:masterfrom
culiutudousi:patch-1

Conversation

@culiutudousi

@culiutudousi culiutudousi commented Mar 16, 2026 •

Copy link
Copy Markdown
Contributor

Bug fix: when calling text() with input x as null

When calling text() with x as null like:

  .text('balabala', null, null, { width: 256 })

It will raise error as TypeError: Cannot read properties of null (reading 'rotation').

It happens in function _initOptions that when x is null:

  1. it will pass the if check of typeof x === 'object' at line 392
  2. then leads options to be overwritten as null at line 393
  3. then it will raise error when calling options.rotation at line 436

Actually any of the change line could fix this bug, but I changed both 2 places to make it safer for future.

This bug exists after 0.27.0 when the feature of rotation added with code result.rotation = Number(options?.rotation ?? 0) % 360;.

Checklist:

  • Unit Tests - N/A no new test added because no api changes, only easy fix
  • Documentation - N/A
  • Update CHANGELOG.md
  • Ready to be merged

while this is the first time I contribute in this project, please let me know if I missed any thing

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a crash in the text rendering pipeline when text() is called with x explicitly set to null, which previously could result in options becoming null and throwing when reading rotation.

Changes:

  • Guard the “treat x as options object” overload to avoid treating null as an options object.
  • Make rotation access null-safe via optional chaining.
  • Add an Unreleased changelog entry documenting the fix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
lib/mixins/text.js Prevents null from being treated as an options object and avoids null deref when reading rotation.
CHANGELOG.md Documents the fix under “Unreleased”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread lib/mixins/text.js Outdated
Comment thread lib/mixins/text.js
Comment on lines 391 to 394
_initOptions(x = {}, y, options = {}) {
if (typeof x === 'object') {
if (x && typeof x === 'object') {
options = x;
x = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot add a unit test

Comment thread CHANGELOG.md

- Bump node version requirement to 20+
- Bump minimum supported browsers to Firefox 115, iOS/Safari 16
- Fix text with input x as null
@blikblum blikblum closed this Mar 16, 2026
@blikblum blikblum reopened this Mar 16, 2026
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@blikblum

Copy link
Copy Markdown
Member

@culiutudousi please add a simple regression unit test

@culiutudousi

Copy link
Copy Markdown
Contributor Author

@culiutudousi please add a simple regression unit test

Hi @blikblum thanks for your review, unit test added

@blikblum

Copy link
Copy Markdown
Member

There is a prettier issue.

@culiutudousi

Copy link
Copy Markdown
Contributor Author

thanks @blikblum , fixed now

@blikblum
blikblum merged commit 78b1b87 into foliojs:master Mar 17, 2026
3 checks passed
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.

3 participants