Do not scrub attributes on a removed node

This should improve performance by eliminating unneeded scrubbing of attributes.

Author: flavorjones

lib/rails/html/scrubbers.rb

@@ -72,7 +72,8 @@ def scrub(node)
         return CONTINUE if skip_node?(node)
 
         unless (node.element? || node.comment?) && keep_node?(node)
-          return STOP if scrub_node(node) == STOP
+          scrub_node(node)
+          return STOP
         end
 
         scrub_attributes(node)

test/scrubbers_test.rb

@@ -215,3 +215,39 @@ def test_returns_stop_from_scrub_if_scrub_node_does
     assert_scrub_stopped "<script>remove me</script>"
   end
 end
+
+class PermitScrubberMinimalOperationsTest < ScrubberTest
+  class TestPermitScrubber < Rails::HTML::PermitScrubber
+    def initialize
+      @scrub_attribute_args = []
+      @scrub_attributes_args = []
+
+      super
+
+      self.tags = ["div"]
+      self.attributes = ["class"]
+    end
+
+    def scrub_attributes(node)
+      @scrub_attributes_args << node.name
+
+      super
+    end
+
+    def scrub_attribute(node, attr)
+      @scrub_attribute_args << [node.name, attr.name]
+
+      super
+    end
+  end
+
+  def test_does_not_scrub_attributes_of_a_removed_node
+    @scrubber = TestPermitScrubber.new
+
+    input = "<div class='foo' href='bar'><svg xlink:href='asdf'><set></set></svg></div>"
+    frag = scrub_fragment(input)
+    assert_equal("<div class=\"foo\"></div>", frag)
+
+    assert_equal(["div"], @scrubber.instance_variable_get(:@scrub_attributes_args))
+  end
+end