From 91fd43886766dca759e2280da2e8ca12aac11cbf Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 18 Jun 2024 15:45:17 +0200 Subject: [PATCH] Remove sanitize_media_query and media queries are strings Media queries can be anything so it don't make much sense to have it as symbol. The media "print" seam like could be nice as a media query but it can be combined with "max-width: 1px", to "pring and max-width: 1px)" which make less sense as a symbol. --- lib/css_parser.rb | 7 ----- lib/css_parser/parser.rb | 42 +++++++++++++++-------------- lib/css_parser/parser_fx.rb | 1 - test/test_css_parser_loading.rb | 6 ++--- test/test_css_parser_media_types.rb | 34 +++++++++++------------ 5 files changed, 42 insertions(+), 48 deletions(-) diff --git a/lib/css_parser.rb b/lib/css_parser.rb index 019d088..10177f9 100644 --- a/lib/css_parser.rb +++ b/lib/css_parser.rb @@ -155,11 +155,4 @@ def self.convert_uris(css, base_uri) "url('#{uri}')" end end - - def self.sanitize_media_query(raw) - mq = raw.to_s.gsub(/\s+/, ' ') - mq.strip! - mq = 'all' if mq.empty? - mq.to_sym - end end diff --git a/lib/css_parser/parser.rb b/lib/css_parser/parser.rb index 06a81d6..5c23254 100644 --- a/lib/css_parser/parser.rb +++ b/lib/css_parser/parser.rb @@ -10,6 +10,13 @@ module CssParser # [import] Follow @import rules. Boolean, default is true. # [io_exceptions] Throw an exception if a link can not be found. Boolean, default is true. class Parser + module Util + def self.ensure_media_types(media_types) + Array(media_types) + .tap { raise ArgumentError unless _1.all? { |type| type.is_a?(String) || type == :all } } + end + end + USER_AGENT = "Ruby CSS Parser/#{CssParser::VERSION} (https://github.com/premailer/css_parser)".freeze def initialize(options = {}) @@ -34,7 +41,7 @@ def initialize(options = {}) # Get declarations by selector. # - # +media_types+ are optional, and can be a symbol or an array of symbols. + # +media_types+ are optional, and can be a symbol or an array of media queries (:all or string). # The default value is :all. # # ==== Examples @@ -78,9 +85,9 @@ def find_rule_sets(selectors, media_types = :all) # In order to follow +@import+ rules you must supply either a # +:base_dir+ or +:base_uri+ option. # - # Use the +:media_types+ option to set the media type(s) for this block. Takes an array of symbols. + # Use the +:media_types+ option to set the media type(s) for this block. Takes an media queries (:all or string). # - # Use the +:only_media_types+ option to selectively follow +@import+ rules. Takes an array of symbols. + # Use the +:only_media_types+ option to selectively follow +@import+ rules. Takes an media queries (:all or string). # # ==== Example # css = <<-EOT @@ -94,19 +101,16 @@ def find_rule_sets(selectors, media_types = :all) # parser = CssParser::Parser.new # parser.add_block!(css) def add_block!(block, options = {}) - options = {base_uri: nil, base_dir: nil, charset: nil, media_types: :all, only_media_types: :all}.merge(options) - options[:media_types] = [options[:media_types]].flatten.collect { |mt| CssParser.sanitize_media_query(mt) } - options[:only_media_types] = [options[:only_media_types]].flatten.collect { |mt| CssParser.sanitize_media_query(mt) } + options = {base_uri: nil, base_dir: nil, charset: nil, media_types: [:all], only_media_types: [:all]}.merge(options) + options[:media_types] = Util.ensure_media_types(options[:media_types]) + options[:only_media_types] = Util.ensure_media_types(options[:only_media_types]) # TODO: Would be nice to skip this step too if options[:base_uri] and @options[:absolute_paths] block = CssParser.convert_uris(block, options[:base_uri]) end - current_media_queries = [:all] - if options[:media_types] - current_media_queries = options[:media_types].flatten.collect { |mt| CssParser.sanitize_media_query(mt) } - end + current_media_queries = Util.ensure_media_types(options[:media_types] || [:all]) Crass.parse(block).each do |node| case node @@ -212,7 +216,7 @@ def add_block!(block, options = {}) # and +media_types+. Optional pass +filename+ , +offset+ for source # reference too. # - # +media_types+ can be a symbol or an array of symbols. default to :all + # +media_types+ can be a symbol or an array of media queries (:all or string). default to :all # optional fields for source location for source location # +filename+ can be a string or uri pointing to the file or url location. # +offset+ should be Range object representing the start and end byte locations where the rule was found in the file. @@ -229,23 +233,22 @@ def add_rule!(selectors: nil, block: nil, filename: nil, offset: nil, media_type # Add a CssParser RuleSet object. # - # +media_types+ can be a symbol or an array of symbols. + # +media_types+ can be a symbol or an media queries (:all or string). def add_rule_set!(ruleset, media_types = :all) raise ArgumentError unless ruleset.is_a?(CssParser::RuleSet) - media_types = [media_types] unless media_types.is_a?(Array) - media_types = media_types.flat_map { |mt| CssParser.sanitize_media_query(mt) } + media_types = Util.ensure_media_types(media_types) @rules << {media_types: media_types, rules: ruleset} end # Remove a CssParser RuleSet object. # - # +media_types+ can be a symbol or an array of symbols. + # +media_types+ can be a symbol or an media queries (:all or string). def remove_rule_set!(ruleset, media_types = :all) raise ArgumentError unless ruleset.is_a?(CssParser::RuleSet) - media_types = [media_types].flatten.collect { |mt| CssParser.sanitize_media_query(mt) } + media_types = Util.ensure_media_types(media_types) @rules.reject! do |rule| rule[:media_types] == media_types && rule[:rules].to_s == ruleset.to_s @@ -254,10 +257,9 @@ def remove_rule_set!(ruleset, media_types = :all) # Iterate through RuleSet objects. # - # +media_types+ can be a symbol or an array of symbols. + # +media_types+ can be a symbol or an array of media queries (:all or string). def each_rule_set(media_types = :all) # :yields: rule_set, media_types - media_types = [:all] if media_types.nil? - media_types = [media_types].flatten.collect { |mt| CssParser.sanitize_media_query(mt) } + media_types = Util.ensure_media_types(media_types) @rules.each do |block| if media_types.include?(:all) or block[:media_types].any? { |mt| media_types.include?(mt) } @@ -289,7 +291,7 @@ def to_h(which_media = :all) # Iterate through CSS selectors. # - # +media_types+ can be a symbol or an array of symbols. + # +media_types+ can be a symbol or an array of media queries (:all or string). # See RuleSet#each_selector for +options+. def each_selector(all_media_types = :all, options = {}) # :yields: selectors, declarations, specificity, media_types return to_enum(__method__, all_media_types, options) unless block_given? diff --git a/lib/css_parser/parser_fx.rb b/lib/css_parser/parser_fx.rb index 2d17278..965c0a6 100644 --- a/lib/css_parser/parser_fx.rb +++ b/lib/css_parser/parser_fx.rb @@ -58,7 +58,6 @@ def self.split_media_query_by_or_condition(media_query_selector) end # rubocop:disable Style/MultilineBlockChain .map { Crass::Parser.stringify(_1).strip } .reject(&:empty?) - .map(&:to_sym) end end end diff --git a/test/test_css_parser_loading.rb b/test/test_css_parser_loading.rb index 4898e29..ba32ab8 100644 --- a/test/test_css_parser_loading.rb +++ b/test/test_css_parser_loading.rb @@ -180,9 +180,9 @@ def test_importing_with_media_types @cp.load_uri!("#{@uri_base}/import-with-media-types.css") - # from simple.css with :screen media type - assert_equal 'margin: 0px;', @cp.find_by_selector('p', :screen).join(' ') - assert_equal '', @cp.find_by_selector('p', :tty).join(' ') + # from simple.css with screen media type + assert_equal 'margin: 0px;', @cp.find_by_selector('p', "screen").join(' ') + assert_equal '', @cp.find_by_selector('p', "tty").join(' ') end def test_local_circular_reference_exception diff --git a/test/test_css_parser_media_types.rb b/test/test_css_parser_media_types.rb index 87e00e9..1ee798a 100644 --- a/test/test_css_parser_media_types.rb +++ b/test/test_css_parser_media_types.rb @@ -41,9 +41,9 @@ def test_finding_by_media_type } CSS - assert_equal 'font-size: 10pt; line-height: 1.2;', @cp.find_by_selector('body', :print).join(' ') - assert_equal 'font-size: 13px; line-height: 1.2; color: blue;', @cp.find_by_selector('body', :screen).join(' ') - assert_equal 'color: blue;', @cp.find_by_selector('body', :'print and resolution > 90dpi').join(' ') + assert_equal 'font-size: 10pt; line-height: 1.2;', @cp.find_by_selector('body', "print").join(' ') + assert_equal 'font-size: 13px; line-height: 1.2; color: blue;', @cp.find_by_selector('body', "screen").join(' ') + assert_equal 'color: blue;', @cp.find_by_selector('body', 'print and resolution > 90dpi').join(' ') end def test_with_parenthesized_media_features @@ -59,10 +59,10 @@ def test_with_parenthesized_media_features 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(' ') + 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 @@ -78,16 +78,16 @@ def test_finding_by_multiple_media_types } CSS - assert_equal 'font-size: 13px; line-height: 1.2;', @cp.find_by_selector('body', [:screen, :handheld]).join(' ') + assert_equal 'font-size: 13px; line-height: 1.2;', @cp.find_by_selector('body', ["screen", "handheld"]).join(' ') end def test_adding_block_with_media_types - @cp.add_block!(<<-CSS, media_types: [:screen]) + @cp.add_block!(<<-CSS, media_types: ["screen"]) body { font-size: 10pt } CSS - assert_equal 'font-size: 10pt;', @cp.find_by_selector('body', :screen).join(' ') - assert @cp.find_by_selector('body', :handheld).empty? + assert_equal 'font-size: 10pt;', @cp.find_by_selector('body', "screen").join(' ') + assert @cp.find_by_selector('body', "handheld").empty? end def test_adding_block_with_media_types_followed_by_general_rule @@ -109,7 +109,7 @@ def test_adding_block_and_limiting_media_types1 base_dir = Pathname.new(__dir__).join('fixtures') - @cp.add_block!(css, only_media_types: :screen, base_dir: base_dir) + @cp.add_block!(css, only_media_types: "screen", base_dir: base_dir) assert @cp.find_by_selector('div').empty? end @@ -130,14 +130,14 @@ def test_adding_block_and_limiting_media_types CSS base_dir = Pathname.new(__dir__).join('fixtures') - @cp.add_block!(css, only_media_types: :print, base_dir: base_dir) + @cp.add_block!(css, only_media_types: "print", base_dir: base_dir) assert_equal '', @cp.find_by_selector('div').join(' ') end def test_adding_rule_set_with_media_type - @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: [:handheld, :tty]) - @cp.add_rule!(selectors: 'body', block: 'color: blue;', media_types: :screen) - assert_equal 'color: black;', @cp.find_by_selector('body', :handheld).join(' ') + @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: ["handheld", "tty"]) + @cp.add_rule!(selectors: 'body', block: 'color: blue;', media_types: "screen") + assert_equal 'color: black;', @cp.find_by_selector('body', "handheld").join(' ') end def test_adding_rule_set_with_media_query @@ -147,7 +147,7 @@ def test_adding_rule_set_with_media_query end def test_selecting_with_all_media_types - @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: [:handheld, :tty]) + @cp.add_rule!(selectors: 'body', block: 'color: black;', media_types: ["handheld", "tty"]) assert_equal 'color: black;', @cp.find_by_selector('body', :all).join(' ') end