diff --git a/CHANGELOG.md b/CHANGELOG.md
index 58ae8ff..ca593b9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,11 @@
+## next / unreleased
+
+* Slightly improve performance.
+
+ Assuming elements are more common than comments, make one less method call per node.
+
+ *Mike Dalessio*
+
## 1.4.1 / 2021-08-18
* Fix regression in v1.4.0 that did not pass comment nodes to the scrubber.
diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index 385d357..09cfe95 100644
--- a/lib/rails/html/scrubbers.rb
+++ b/lib/rails/html/scrubbers.rb
@@ -68,7 +68,7 @@ def scrub(node)
end
return CONTINUE if skip_node?(node)
- unless (node.comment? || node.element?) && keep_node?(node)
+ unless (node.element? || node.comment?) && keep_node?(node)
return STOP if scrub_node(node) == STOP
end
diff --git a/test/scrubbers_test.rb b/test/scrubbers_test.rb
index e6612e9..a825404 100644
--- a/test/scrubbers_test.rb
+++ b/test/scrubbers_test.rb
@@ -41,6 +41,16 @@ def test_default_scrub_behavior
assert_scrubbed 'hello', 'hello'
end
+ def test_default_scrub_removes_comments
+ assert_scrubbed('
one
three',
+ 'one
three')
+ end
+
+ def test_default_scrub_removes_processing_instructions
+ assert_scrubbed('one
three',
+ 'one
three')
+ end
+
def test_default_attributes_removal_behavior
assert_scrubbed 'hello
', 'hello
'
end
@@ -56,6 +66,12 @@ def test_leaves_only_supplied_tags
assert_scrubbed html, 'leave me now'
end
+ def test_leaves_comments_when_supplied_as_tag
+ @scrubber.tags = %w(div comment)
+ assert_scrubbed('one
three',
+ 'one
three')
+ end
+
def test_leaves_only_supplied_tags_nested
html = 'leave me now'
@scrubber.tags = %w(tag)
@@ -112,50 +128,6 @@ def test_attributes_accessor_validation
end
end
-class PermitScrubberSubclassTest < ScrubberTest
- def setup
- @scrubber = Class.new(::Rails::Html::PermitScrubber) do
- attr :nodes_seen
-
- def initialize
- super()
- @nodes_seen = []
- end
-
- def keep_node?(node)
- @nodes_seen << node.name
- super(node)
- end
- end.new
- end
-
- def test_elements_are_checked
- html = %Q("
")
- Loofah.scrub_fragment(html, @scrubber)
- assert_includes(@scrubber.nodes_seen, "div")
- assert_includes(@scrubber.nodes_seen, "a")
- assert_includes(@scrubber.nodes_seen, "tr")
- end
-
- def test_comments_are_checked
- # this passes in v1.3.0 but fails in v1.4.0
- html = %Q("
")
- Loofah.scrub_fragment(html, @scrubber)
- assert_includes(@scrubber.nodes_seen, "div")
- assert_includes(@scrubber.nodes_seen, "comment")
- assert_includes(@scrubber.nodes_seen, "tr")
- end
-
- def test_craftily_named_processing_instructions_are_not_checked
- # this fails in v1.3.0 but passes in v1.4.0
- html = %Q("
")
- Loofah.scrub_fragment(html, @scrubber)
- assert_includes(@scrubber.nodes_seen, "div")
- refute_includes(@scrubber.nodes_seen, "a")
- assert_includes(@scrubber.nodes_seen, "tr")
- end
-end
-
class TargetScrubberTest < ScrubberTest
def setup
@scrubber = Rails::Html::TargetScrubber.new