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.

Sign up for GitHub

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

Jump to bottom

Fix parsing space-less media-features #141

Merged
merged 1 commit into from Sep 1, 2023
Merged

Fix parsing space-less media-features #141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -2,6 +2,8 @@

### Unreleased

* Fix parsing space-less media query features like `@media(width:123px)` [#141](https://github.com/premailer/css_parser/pull/141)

### Version v1.15.0

* Fix parsing background shorthands in ruby 3.2 [#140](https://github.com/premailer/css_parser/pull/140)
Expand Down
13 changes: 11 additions & 2 deletions lib/css_parser/parser.rb
View file
Open in desktop
Expand Up @@ -329,7 +329,7 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
rule_start = nil
offset = nil

block.scan(/\s+|\\{2,}|\\?[{}\s"]|.[^\s"{}\\]*/) do |token|
block.scan(/\s+|\\{2,}|\\?[{}\s"]|[()]|.[^\s"{}()\\]*/) do |token|
# save the regex offset so that we know where in the file we are
offset = Regexp.last_match.offset(0) if options[:capture_offsets]

Expand Down Expand Up @@ -391,7 +391,16 @@ def parse_block_into_rule_sets!(block, options = {}) # :nodoc:
current_media_query = String.new
else
token.strip!
current_media_query << token << ' '
# special-case the ( and ) tokens to remove inner-whitespace
# (eg we'd prefer '(width: 500px)' to '( width: 500px )' )
case token
when '('
current_media_query << token
when ')'
current_media_query.sub!(/ ?$/, token)
else
current_media_query << token << ' '
end
end
Comment on lines +394 to 404
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't necessary to fix the bug, but does give nicer media query names. It could be skipped if we're happy with

@media(min-width: 500px) {...}

getting re-written to

@media ( min-width: 500px ) {...}

I'm happy with either, but it's maybe not worth the extra complexity. Without it, I thought the test suite needing the extra spaces in, eg, @cp.find_by_selector('body', :'( min-width: 500px )') was kind of ugly... but I don't know how much real-world usage the media_types argument to find_selector is getting.

elsif in_charset or token =~ /@charset/i
# iterate until we are out of the charset declaration
Expand Down
19 changes: 19 additions & 0 deletions test/test_css_parser_media_types.rb
View file
Open in desktop
Expand Up @@ -46,6 +46,25 @@ def test_finding_by_media_type
assert_equal 'color: blue;', @cp.find_by_selector('body', 'print and resolution > 90dpi'.to_sym).join(' ')
end

def test_with_parenthesized_media_features
@cp.add_block!(<<-CSS)
body { color: black }
@media(prefers-color-scheme: dark) {
body { color: white }
}
@media(min-width: 500px) {
body { color: blue }
}
@media screen and (width > 500px) {
body { color: red }
}
CSS
assert_equal [:all, :'(prefers-color-scheme: dark)', :'(min-width: 500px)', :'screen and (width > 500px)'], @cp.rules_by_media_query.keys
assert_equal 'color: white;', @cp.find_by_selector('body', :'(prefers-color-scheme: dark)').join(' ')
assert_equal 'color: blue;', @cp.find_by_selector('body', :'(min-width: 500px)').join(' ')
assert_equal 'color: red;', @cp.find_by_selector('body', :'screen and (width > 500px)').join(' ')
end

def test_finding_by_multiple_media_types
@cp.add_block!(<<-CSS)
@media print {
Expand Down