Skip to content

Fix rewriting of CSS @import statements that use strings instead of url()#639

Merged
rtibbles merged 1 commit into
learningequality:mainfrom
jaltekruse:fix-css-imports-bare-string-urls
Jul 5, 2026
Merged

Fix rewriting of CSS @import statements that use strings instead of url()#639
rtibbles merged 1 commit into
learningequality:mainfrom
jaltekruse:fix-css-imports-bare-string-urls

Conversation

@jaltekruse

@jaltekruse jaltekruse commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

Summary

While trying to import a textbook written in PreteXt I found a missing case in the code that finds referenced resources inside of CSS files. While most cases require using the url("http://abc.com") syntax, there are a few instances where bare "string" types are acceptable in the CSS spec, the one I hit was in an @import statement.

References

https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/At-rules/@import
url - Is a <string> or a <url> type representing the location of the resource to import.
The URL may be absolute or relative.

Reviewer guidance

I'll add an automated test, just wanted to create this PR as a place to document my work in the open and write some additional notes

@jaltekruse

Copy link
Copy Markdown
Contributor Author

While I'm working on a test, I'm also going to try to look up any other lingering cases where bare strings are accepted inside of CSS for URLs, that don't need a url() to wrap them. So far a quick search only gave me relevant info in the Gemini AI summary from google, I didn't find an independent page/discussion about this topic:

URLs in CSS can appear as plain strings in the following contexts:
@import rule: When importing another CSS stylesheet, the URL can be specified as a plain string.
Code

@import "path/to/another.css";

This is equivalent to using the url() function:
Code

@import url("path/to/another.css");

content property (for generated content): When using the content property with pseudo-elements (::before, ::after), a URL can be included as part of the string content.
Code

.element::before {
  content: "Visit our website: https://example.com";
}

cursor property (for custom cursors): A URL to an image file can be provided as a string value for a custom cursor.
Code

body {
  cursor: url("path/to/custom-cursor.png"), auto;
}

In other contexts where URLs are used in CSS, such as for background-image, list-style-image, border-image-source, or @font-face src, the url() functional notation is typically required to explicitly denote a URL value.

@MisRob

MisRob commented Dec 2, 2025

Copy link
Copy Markdown
Member

Hi @jaltekruse, we will have a look, thanks!

@rtibbles rtibbles self-assigned this Dec 2, 2025
rtibblesbot added a commit to rtibblesbot/ricecooker that referenced this pull request Feb 13, 2026
Introduces ricecooker/utils/url_utils.py with pure-function utilities for
detecting and rewriting external URL references in HTML, CSS, and H5P JSON
content. These will be used by the archive processor (Phase 3) to download
external resources and bundle them into archives for offline use.

Functions: is_external_url, derive_local_filename, extract_urls_from_html,
extract_urls_from_css, extract_urls_from_h5p_json, rewrite_urls_in_html,
rewrite_urls_in_css, rewrite_urls_in_h5p_json.

Includes bug fixes from PRs learningequality#636 (css_node_filter attr check) and learningequality#639
(CSS @import bare string regex) incorporated into the new utilities.

64 new tests in tests/test_url_utils.py — all pure-function, no I/O.

Ref: learningequality#233, supersedes learningequality#303

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rtibblesbot added a commit to rtibblesbot/ricecooker that referenced this pull request Feb 13, 2026
…egexes

- Import _CSS_IMPORT_RE from url_utils instead of defining locally,
  consolidating the @import regex from PR learningequality#639 into the shared module
- Use _CSS_URL_RE.search() for background-image extraction instead of
  a separate inline regex
- Clean up srcset parsing to match url_utils conventions
- Fix linting issues in test_downloader.py from PR learningequality#636 merge
- Make parse_srcset public (renamed from _parse_srcset)
- Relax _CSS_IMPORT_RE to allow optional whitespace (\s*) for compat

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rtibbles rtibbles force-pushed the fix-css-imports-bare-string-urls branch from 6d6badb to e1e269e Compare July 4, 2026 21:57

@rtibbles rtibbles left a comment

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.

I made some test tweaks to simplify tests and get them to pass on Windows. One small correctness fix to allow spaces after @import statements.

…rl()

Match @import with optional whitespace before the string so the standard `@import "styles.css"` form is rewritten, and add a regression test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014AH7GGbLRC3y8KKtSiY5Cu
@rtibbles rtibbles force-pushed the fix-css-imports-bare-string-urls branch from e1e269e to af3926a Compare July 5, 2026 01:13
@rtibbles rtibbles merged commit 3a36225 into learningequality:main Jul 5, 2026
19 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