Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed counter-reset not working in combination with inline styles #460

Merged
merged 1 commit into from
May 6, 2023

Conversation

SimonBackx
Copy link
Contributor

@SimonBackx SimonBackx commented May 2, 2023

An error is thrown in resetCounter when adding an counter-reset inline style to an element in combination with a different style (e.g., in head).

  1) juice-content/counter-reset:
     Uncaught TypeError: Cannot set properties of undefined (setting 'li')
      at resetCounter (lib/inline.js:167:40)
      at addProps (lib/inline.js:198:15)
      at Element.<anonymous> (lib/inline.js:146:11)
      at LoadedCheerio.each (node_modules/cheerio/lib/api/traversing.js:519:26)
      at handleRule (lib/inline.js:120:9)
      at Array.forEach (<anonymous>)
      at inlineDocument (lib/inline.js:36:9)
      at juiceDocument (lib/inline.js:459:3)
      at module.exports (lib/cheerio.js:61:22)
      at onInline (index.js:63:7)
      at /Users/.../juice/node_modules/web-resource-inliner/src/html.js:281:13

This error is caused by the fact that the addProps method is called when an element doesn't have styleProps (in the if (!el.styleProps) { block), before a default value for counterProps is set. This is fixed by assigning a default value for counterProps before calling addProps.

A test case is also added to reproduce the issue and to prevent future regressions.

An error is thrown in `resetCounter` when adding an `counter-reset` inline style to an element in combination with a different style (e.g., in head).

This error is caused by the fact that the `addProps` method is called when an element doesn't have `styleProps` (in the `if (!el.styleProps) {` block), before a default value for `counterProps` is set. This is fixed by assigning a default value for `counterProps` before calling `addProps`.

A test case is also added to reproduce the issue and to prevent future regressions.
Copy link
Collaborator

@jrit jrit left a comment

Choose a reason for hiding this comment

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

cool, thank you

@jrit jrit merged commit a9d920f into Automattic:master May 6, 2023
3 checks passed
@naz
Copy link

naz commented May 15, 2023

Hey @jrit , would you be able to release a new version with this change please? 🙏

@titanism
Copy link

Bump!

@SimonBackx SimonBackx deleted the fixed-counter-reset branch May 22, 2023 07:38
@SimonBackx
Copy link
Contributor Author

@jrit Sorry to bump this again! Do you mind releasing a new patch version for this? 😊

@jrit
Copy link
Collaborator

jrit commented Jul 4, 2023

published!

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.

None yet

4 participants