fix: Strip unused CSS rules by daveallie · Pull Request #1014 · crosspoint-reader/crosspoint-reader

daveallie · 2026-02-19T23:53:56Z

Summary

In a sample book I loaded, it had 900+ CSS rules, and took up 180kB of memory loading the cache in
- Looking at the rules, a lot of them were completely useless as we only ever apply look for 3 kinds of CSS rules:
  - tag
  - tag.class1
  - .class1
Stripping out CSS rules with descendant, nested, attribute matching, sibling matching, pseudo element selection (as we never actually read these from the cache) reduced the rule count down to 200

Additional Context

I've left in .class1.class2 rules for now, even though we technically can never match on them as they're likely to be addressed soonest out of the all the CSS expansion
Because we don't ever delete the CSS cache, users will need to delete the book cache through the menu in order to get this new logic
- A new PR should be done up to address this - tracked here Allow css_rules.cache to be rebuilt when CSS_CACHE_VERSION changes #1015

AI Usage

While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it
helps set the right context for reviewers.

Did you use AI tools to help write this code? No

coderabbitai · 2026-02-19T23:54:17Z

📝 Walkthrough

Walkthrough

Trim behavior expanded to remove trailing spaces/newlines; selector processing now skips selectors containing +, >, [, :, or whitespace; TODOs added for future class-combination support (e.g., .class1.class2); no public API or cache-version functional changes.

Changes

Cohort / File(s)	Summary
Selector Filtering `lib/Epub/Epub/css/CssParser.cpp`	Added pre-checks in `processRuleBlockWithStyle` to skip selectors containing `+`, `>`, `[`, `:`, or whitespace, restricting processing to simple selectors only.
Normalization & Cache Notes `lib/Epub/Epub/css/CssParser.cpp`	Expanded trimming from removing a single trailing space to looping to remove trailing spaces/newlines. Added TODO comments about future combined-class selector support; no functional change to cache version constants.
Resolve Style Comments `lib/Epub/Epub/css/CssParser.cpp`	Inserted comments/TODOs in `resolveStyle` outlining planned support for combined class selectors; no runtime behavior change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

jdk2pq

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: Strip unused CSS rules' accurately reflects the main change in the PR, which filters out unsupported CSS selectors and reduces rule count from 900+ to 200.
Description check	✅ Passed	The description is well-detailed and directly related to the changeset, explaining the motivation, analysis, implementation approach, and migration considerations for stripping unused CSS rules.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cabbfcf and cc45c27.

📒 Files selected for processing (1)

lib/Epub/Epub/css/CssParser.cpp

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: build
GitHub Check: cppcheck

🔇 Additional comments (3)

lib/Epub/Epub/css/CssParser.cpp (3)

76-79: Trailing whitespace trimming is now robust.
Looping until the end prevents leftover trailing whitespace after normalization.

563-577: Comment-only clarification is helpful.
The TODOs make future class-combination support explicit without changing behavior.

598-599: Cache version bump aligns with format changes.
Invalidates older cache entries after selector filtering updates.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/Epub/Epub/css/CssParser.cpp`:
- Around line 346-375: The selector filter currently skips only +, >, [, :, and
whitespace but still accepts unsupported selectors (e.g. those with '#', '~',
'*', '|') which get cached into rulesBySelector_ and waste MAX_RULES slots;
update the filtering logic (the block that checks the variable named key in
CssParser.cpp) to perform an allowlist check that only accepts the simple
patterns supported by the resolver (bare tag names, .class, or tag.class) — for
example: reject any selector containing '#', '~', '*', '|' or any characters
outside valid tag/class name characters, and skip inserting into
rulesBySelector_ when the selector does not match the allowlist so only valid
patterns consume cache entries and MAX_RULES.

lib/Epub/Epub/css/CssParser.cpp

coderabbitai

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc45c27 and ac0171b.

📒 Files selected for processing (1)

lib/Epub/Epub/css/CssParser.cpp

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@lib/Epub/Epub/css/CssParser.cpp`:
- Around line 346-375: The selector filtering in CssParser.cpp currently skips
selectors containing '+', '>', '[', ':', and whitespace but still allows
unsupported patterns like '#', '~', '*', and '|' which waste cache slots; update
the filter around the variable/key checks in the relevant parsing routine (the
block checking key.find(...)) to also detect and continue on selectors
containing '#', '~', '*', and '|' so those selectors are ignored before
caching/resolving.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

lib/Epub/Epub/css/CssParser.cpp (1)
77-79: Inconsistency with normalizedInto() and redundant '\n' check.

The normalization loop (lines 64-73) already converts all CSS whitespace including '\n' to ' ', making the '\n' check redundant here. Additionally, normalizedInto() at lines 100-102 uses a single if checking only for ' ', creating inconsistent behavior between these two functions.

Consider aligning both functions:
Suggested fix
   // Remove trailing space
-  while (!result.empty() && (result.back() == ' ' || result.back() == '\n')) {
+  while (!result.empty() && result.back() == ' ') {
     result.pop_back();
   }
And update normalizedInto() to match:
  while (!out.empty() && out.back() == ' ') {
    out.pop_back();
  }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/Epub/Epub/css/CssParser.cpp` around lines 77 - 79, The trailing-character
trimming in CssParser::normalized() currently checks for both ' ' and '\n' even
though earlier normalization converts all whitespace (including '\n') to spaces;
make it consistent with normalizedInto() by removing the '\n' check and only
trimming trailing spaces, and update normalizedInto() (the function named
normalizedInto) to use the same single-character check (while (!out.empty() &&
out.back() == ' ') { out.pop_back(); }) so both functions behave identically
when removing trailing whitespace.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac0171b and 58eac9d.

📒 Files selected for processing (1)

lib/Epub/Epub/css/CssParser.cpp

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build

🔇 Additional comments (1)

lib/Epub/Epub/css/CssParser.cpp (1)

581-605: LGTM!

The TODO comments appropriately document the known limitation around multi-class selector combinations. The current implementation correctly handles single-class lookups and defers the more complex .class1.class2 matching to future work as stated in the PR objectives.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/Epub/Epub/css/CssParser.cpp`:
- Around line 346-394: The selector-filtering block in CssParser.cpp currently
skips selectors containing '+', '>', '[', ':', '#', '~', '*', and whitespace but
misses the namespace separator '|' so selectors like "svg|rect" are not
filtered; update the checks around the `key` variable (same block that tests
key.find('+') etc.) to also continue when key.find('|') !=
std::string_view::npos, following the same pattern and comment style as the
other excluded characters.

---

Nitpick comments:
In `@lib/Epub/Epub/css/CssParser.cpp`:
- Around line 77-79: The trailing-character trimming in CssParser::normalized()
currently checks for both ' ' and '\n' even though earlier normalization
converts all whitespace (including '\n') to spaces; make it consistent with
normalizedInto() by removing the '\n' check and only trimming trailing spaces,
and update normalizedInto() (the function named normalizedInto) to use the same
single-character check (while (!out.empty() && out.back() == ' ') {
out.pop_back(); }) so both functions behave identically when removing trailing
whitespace.

lib/Epub/Epub/css/CssParser.cpp

## Summary * In a sample book I loaded, it had 900+ CSS rules, and took up 180kB of memory loading the cache in * Looking at the rules, a lot of them were completely useless as we only ever apply look for 3 kinds of CSS rules: * `tag` * `tag.class1` * `.class1` * Stripping out CSS rules with descendant, nested, attribute matching, sibling matching, pseudo element selection (as we never actually read these from the cache) reduced the rule count down to 200 ## Additional Context * I've left in `.class1.class2` rules for now, even though we technically can never match on them as they're likely to be addressed soonest out of the all the CSS expansion * Because we don't ever delete the CSS cache, users will need to delete the book cache through the menu in order to get this new logic * A new PR should be done up to address this - tracked here #1015 --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? No

## Summary * In a sample book I loaded, it had 900+ CSS rules, and took up 180kB of memory loading the cache in * Looking at the rules, a lot of them were completely useless as we only ever apply look for 3 kinds of CSS rules: * `tag` * `tag.class1` * `.class1` * Stripping out CSS rules with descendant, nested, attribute matching, sibling matching, pseudo element selection (as we never actually read these from the cache) reduced the rule count down to 200 ## Additional Context * I've left in `.class1.class2` rules for now, even though we technically can never match on them as they're likely to be addressed soonest out of the all the CSS expansion * Because we don't ever delete the CSS cache, users will need to delete the book cache through the menu in order to get this new logic * A new PR should be done up to address this - tracked here crosspoint-reader#1015 --- ### AI Usage While CrossPoint doesn't have restrictions on AI tools in contributing, please be transparent about their usage as it helps set the right context for reviewers. Did you use AI tools to help write this code? No

daveallie added 2 commits February 20, 2026 10:52

Avoid caching and loading unusable CSS rules

7849bb2

Trim all training whitespace from CSS selector

ac0171b

daveallie requested a review from jdk2pq February 19, 2026 23:53

daveallie changed the title ~~fix: Strip unused css rules~~ fix: Strip unused CSS rules Feb 19, 2026

daveallie force-pushed the fix/strip-unused-css-rules branch from cc45c27 to ac0171b Compare February 19, 2026 23:56

coderabbitai bot reviewed Feb 19, 2026

View reviewed changes

lib/Epub/Epub/css/CssParser.cpp Show resolved Hide resolved

coderabbitai bot reviewed Feb 20, 2026

View reviewed changes

Add more CSS exclusions

58eac9d

coderabbitai bot reviewed Feb 20, 2026

View reviewed changes

lib/Epub/Epub/css/CssParser.cpp Show resolved Hide resolved

daveallie mentioned this pull request Feb 20, 2026

feat: Avoid css crash, css rule selection logic #967

Closed

jdk2pq approved these changes Feb 20, 2026

View reviewed changes

znelson approved these changes Feb 20, 2026

View reviewed changes

daveallie merged commit 356fe9a into master Feb 20, 2026
6 checks passed

coderabbitai bot mentioned this pull request Feb 20, 2026

fix: Destroy CSS Cache file when invalid #1018

Merged

daveallie deleted the fix/strip-unused-css-rules branch February 22, 2026 09:56

daveallie mentioned this pull request Feb 22, 2026

chore(release): 1.1.0 Release Candidate #992

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Strip unused CSS rules#1014

fix: Strip unused CSS rules#1014
daveallie merged 3 commits intomasterfrom
fix/strip-unused-css-rules

daveallie commented Feb 19, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 19, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

daveallie commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Additional Context

AI Usage

Uh oh!

coderabbitai bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

daveallie commented Feb 19, 2026 •

edited

Loading

coderabbitai bot commented Feb 19, 2026 •

edited

Loading