Merge pull request #65 from jamesmartin/handle-malformed-docs

Handle documents that do not contain SVG root elements

CHANGELOG.md

@@ -3,7 +3,8 @@ All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased][unreleased]
-- Nothing
+- Handle malformed documents that don't contain a root SVG element
+  [#60](https://github.com/jamesmartin/inline_svg/pull/65)
 
 ## [1.2.1] - 2017-05-02
 ### Fixed

README.md

@@ -133,10 +133,9 @@ For example, inherit from `InlineSvg::CustomTransformation` and implement the `#
 
 class MyCustomTransform < InlineSvg::CustomTransformation
   def transform(doc)
-    doc = Nokogiri::XML::Document.parse(doc.to_html)
-    svg = doc.at_css 'svg'
-    svg['custom'] = value
-    doc
+    with_svg(doc) do |svg|
+      svg["custom"] = value
+    end
   end
 end
 ```

lib/inline_svg/transform_pipeline/transformations/aria_attributes.rb

@@ -1,34 +1,31 @@
 module InlineSvg::TransformPipeline::Transformations
   class AriaAttributes < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css("svg")
+      with_svg(doc) do |svg|
+        # Add role
+        svg["role"] = "img"
 
-      # Add role
-      svg["role"] = "img"
+        # Build aria-labelledby string
+        aria_elements = []
+        svg.search("title").each do |element|
+          aria_elements << element["id"] = element_id_for("title", element)
+        end
 
-      # Build aria-labelledby string
-      aria_elements = []
-      doc.search("svg title").each do |element|
-        aria_elements << element['id'] = element_id_for("title", element)
-      end
-
-      doc.search("svg desc").each do |element|
-        aria_elements << element['id'] = element_id_for("desc", element)
-      end
+        svg.search("desc").each do |element|
+          aria_elements << element["id"] = element_id_for("desc", element)
+        end
 
-      if aria_elements.any?
-        svg["aria-labelledby"] = aria_elements.join(" ")
+        if aria_elements.any?
+          svg["aria-labelledby"] = aria_elements.join(" ")
+        end
       end
-
-      doc
     end
 
     def element_id_for(base, element)
-      if element['id'].nil?
+      if element["id"].nil?
         InlineSvg::IdGenerator.generate(base, element.text)
       else
-        InlineSvg::IdGenerator.generate(element['id'], element.text)
+        InlineSvg::IdGenerator.generate(element["id"], element.text)
       end
     end
   end

lib/inline_svg/transform_pipeline/transformations/class_attribute.rb

@@ -1,12 +1,11 @@
 module InlineSvg::TransformPipeline::Transformations
   class ClassAttribute < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css "svg"
-      classes = (svg["class"] || "").split(" ")
-      classes << value
-      svg["class"] = classes.join(" ")
-      doc
+      with_svg(doc) do |svg|
+        classes = (svg["class"] || "").split(" ")
+        classes << value
+        svg["class"] = classes.join(" ")
+      end
     end
   end
 end

lib/inline_svg/transform_pipeline/transformations/data_attributes.rb

@@ -1,12 +1,11 @@
 module InlineSvg::TransformPipeline::Transformations
   class DataAttributes < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css 'svg'
-      with_valid_hash_from(self.value).each_pair do |name, data|
-        svg["data-#{dasherize(name)}"] = data
+      with_svg(doc) do |svg|
+        with_valid_hash_from(self.value).each_pair do |name, data|
+          svg["data-#{dasherize(name)}"] = data
+        end
       end
-      doc
     end
 
     private

lib/inline_svg/transform_pipeline/transformations/description.rb

@@ -1,12 +1,13 @@
 module InlineSvg::TransformPipeline::Transformations
   class Description < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      node = Nokogiri::XML::Node.new('desc', doc)
-      node.content = value
-      doc.search('svg desc').each { |node| node.remove }
-      doc.at_css('svg').prepend_child(node)
-      doc
+      with_svg(doc) do |svg|
+        node = Nokogiri::XML::Node.new("desc", doc)
+        node.content = value
+
+        svg.search("desc").each { |node| node.remove }
+        svg.prepend_child(node)
+      end
     end
   end
 end

lib/inline_svg/transform_pipeline/transformations/height.rb

@@ -1,10 +1,9 @@
 module InlineSvg::TransformPipeline::Transformations
   class Height < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css 'svg'
-      svg['height'] = self.value
-      doc
+      with_svg(doc) do |svg|
+        svg["height"] = self.value
+      end
     end
   end
 end

lib/inline_svg/transform_pipeline/transformations/id_attribute.rb

@@ -1,10 +1,9 @@
 module InlineSvg::TransformPipeline::Transformations
   class IdAttribute < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css 'svg'
-      svg['id'] = self.value
-      doc
+      with_svg(doc) do |svg|
+        svg["id"] = self.value
+      end
     end
   end
 end

lib/inline_svg/transform_pipeline/transformations/no_comment.rb

@@ -2,11 +2,11 @@ module InlineSvg::TransformPipeline
   module Transformations
     class NoComment < Transformation
       def transform(doc)
-        doc = Nokogiri::XML::Document.parse(doc.to_html)
-        doc.xpath("//comment()").each do |comment|
-          comment.remove
+        with_svg(doc) do |svg|
+          svg.xpath("//comment()").each do |comment|
+            comment.remove
+          end
         end
-        doc
       end
     end
   end

lib/inline_svg/transform_pipeline/transformations/preserve_aspect_ratio.rb

@@ -1,10 +1,9 @@
 module InlineSvg::TransformPipeline::Transformations
   class PreserveAspectRatio < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css 'svg'
-      svg['preserveAspectRatio'] = self.value
-      doc
+      with_svg(doc) do |svg|
+        svg["preserveAspectRatio"] = self.value
+      end
     end
   end
 end

lib/inline_svg/transform_pipeline/transformations/size.rb

@@ -1,11 +1,10 @@
 module InlineSvg::TransformPipeline::Transformations
   class Size < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css 'svg'
-      svg['width'] = width_of(self.value)
-      svg['height'] = height_of(self.value)
-      doc
+      with_svg(doc) do |svg|
+        svg["width"] = width_of(self.value)
+        svg["height"] = height_of(self.value)
+      end
     end
 
     def width_of(value)

lib/inline_svg/transform_pipeline/transformations/title.rb

@@ -1,12 +1,13 @@
 module InlineSvg::TransformPipeline::Transformations
   class Title < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      node = Nokogiri::XML::Node.new('title', doc)
-      node.content = value
-      doc.search('svg title').each { |node| node.remove }
-      doc.at_css('svg').prepend_child(node)
-      doc
+      with_svg(doc) do |svg|
+        node = Nokogiri::XML::Node.new("title", doc)
+        node.content = value
+
+        svg.search("title").each { |node| node.remove }
+        svg.prepend_child(node)
+      end
     end
   end
 end

lib/inline_svg/transform_pipeline/transformations/transformation.rb

@@ -13,6 +13,17 @@ def initialize(value)
     def transform(*)
       raise "#transform should be implemented by subclasses of Transformation"
     end
+
+    # Parses a document and yields the contained SVG nodeset to the given block
+    # if it exists.
+    #
+    # Returns a Nokogiri::XML::Document.
+    def with_svg(doc)
+      doc = Nokogiri::XML::Document.parse(doc.to_html)
+      svg = doc.at_css "svg"
+      yield svg if svg && block_given?
+      doc
+    end
   end
 
   class NullTransformation < Transformation

lib/inline_svg/transform_pipeline/transformations/width.rb

@@ -1,10 +1,9 @@
 module InlineSvg::TransformPipeline::Transformations
   class Width < Transformation
     def transform(doc)
-      doc = Nokogiri::XML::Document.parse(doc.to_html)
-      svg = doc.at_css 'svg'
-      svg['width'] = self.value
-      doc
+      with_svg(doc) do |svg|
+        svg["width"] = self.value
+      end
     end
   end
 end

spec/transformation_pipeline/transformations/aria_attributes_spec.rb

@@ -1,8 +1,8 @@
-require 'inline_svg/transform_pipeline'
+require "inline_svg/transform_pipeline"
 
 describe InlineSvg::TransformPipeline::Transformations::AriaAttributes do
   it "adds a role attribute to the SVG document" do
-    document = Nokogiri::XML::Document.parse('<svg>Some document</svg>')
+    document = Nokogiri::XML::Document.parse("<svg>Some document</svg>")
     transformation = InlineSvg::TransformPipeline::Transformations::AriaAttributes.create_with_value({})
 
     expect(transformation.transform(document).to_html).to eq(
@@ -12,7 +12,7 @@
 
   context "aria-labelledby attribute" do
     it "adds 'title' when a title element is present" do
-      document = Nokogiri::XML::Document.parse('<svg><title>Some title</title>Some document</svg>')
+      document = Nokogiri::XML::Document.parse("<svg><title>Some title</title>Some document</svg>")
       transformation = InlineSvg::TransformPipeline::Transformations::AriaAttributes.create_with_value(true)
 
       expect(InlineSvg::IdGenerator).to receive(:generate).with("title", "Some title").
@@ -24,7 +24,7 @@
     end
 
     it "adds 'desc' when a description element is present" do
-      document = Nokogiri::XML::Document.parse('<svg><desc>Some description</desc>Some document</svg>')
+      document = Nokogiri::XML::Document.parse("<svg><desc>Some description</desc>Some document</svg>")
       transformation = InlineSvg::TransformPipeline::Transformations::AriaAttributes.create_with_value(true)
 
       expect(InlineSvg::IdGenerator).to receive(:generate).with("desc", "Some description").
@@ -36,7 +36,7 @@
     end
 
     it "adds both 'desc' and 'title' when title and description elements are present" do
-      document = Nokogiri::XML::Document.parse('<svg><title>Some title</title><desc>Some description</desc>Some document</svg>')
+      document = Nokogiri::XML::Document.parse("<svg><title>Some title</title><desc>Some description</desc>Some document</svg>")
       transformation = InlineSvg::TransformPipeline::Transformations::AriaAttributes.create_with_value(true)
 
       expect(InlineSvg::IdGenerator).to receive(:generate).with("title", "Some title").
@@ -50,7 +50,7 @@
     end
 
     it "uses existing IDs when they exist" do
-      document = Nokogiri::XML::Document.parse('<svg><title id="my-title">Some title</title><desc id="my-desc">Some description</desc>Some document</svg>')
+      document = Nokogiri::XML::Document.parse("<svg><title id='my-title'>Some title</title><desc id='my-desc'>Some description</desc>Some document</svg>")
       transformation = InlineSvg::TransformPipeline::Transformations::AriaAttributes.create_with_value(true)
 
       expect(InlineSvg::IdGenerator).to receive(:generate).with("my-title", "Some title").

spec/transformation_pipeline/transformations/height_spec.rb

@@ -9,4 +9,13 @@
       "<svg height=\"5%\">Some document</svg>\n"
     )
   end
+
+  it "handles documents without SVG root elements" do
+    document = Nokogiri::XML::Document.parse("<foo>bar</foo><svg>Some document</svg>")
+    transformation = InlineSvg::TransformPipeline::Transformations::Height.create_with_value("5%")
+
+    expect(transformation.transform(document).to_html).to eq(
+      "<foo>bar</foo>\n"
+    )
+  end
 end

spec/transformation_pipeline/transformations/transformation_spec.rb

@@ -0,0 +1,36 @@
+require 'inline_svg'
+require 'inline_svg/transform_pipeline'
+
+describe InlineSvg::TransformPipeline::Transformations::Transformation do
+  context "#with_svg" do
+    it "returns a Nokogiri::XML::Document representing the parsed document fragment" do
+      document = Nokogiri::XML::Document.parse("<svg>Some document</svg>")
+
+      transformation = InlineSvg::TransformPipeline::Transformations::Transformation.new(:irrelevant)
+      expect(transformation.with_svg(document).to_html).to eq(
+        "<svg>Some document</svg>\n"
+      )
+    end
+
+    it "yields to the block when the document contains an SVG element" do
+      document = Nokogiri::XML::Document.parse("<svg>Some document</svg>")
+      svg = document.at_css("svg")
+
+      transformation = InlineSvg::TransformPipeline::Transformations::Transformation.new(:irrelevant)
+
+      expect do |b|
+        transformation.with_svg(document, &b)
+      end.to yield_with_args(svg)
+    end
+
+    it "does not yield if the document does not contain an SVG element at the root" do
+      document = Nokogiri::XML::Document.parse("<foo>bar</foo><svg>Some document</svg>")
+
+      transformation = InlineSvg::TransformPipeline::Transformations::Transformation.new(:irrelevant)
+
+      expect do |b|
+        transformation.with_svg(document, &b)
+      end.not_to yield_control
+    end
+  end
+end