Remove octicons altogether from gollum-lib. (#441)

* Remove octicons altogether from gollum-lib.
* Rename Octicon macro to Icon. Leave finding and rendering the actual icon to frontend (gollum).
* Remove hardcoded octicon names from gollum-lib.
* Add Flash macro that comes without default icon.
* Upgrade minitest-reporters.

HISTORY.md

@@ -1,3 +1,13 @@
+# 5.2.2 / 2023-01-18
+
+* Bugfix release: set Nokogiri default XHTML conversion options more relaibly. See https://github.com/sparklemotion/nokogiri/issues/2761
+
+
+# 5.2.1 / 2022-09-13
+
+* Fixed: 'controls' attribute in audio and video tags should not be sanitized. #430 (@dometto)
+
+
 # 5.2 / 2022-05-28
 
 * Conditionally render "editable" heading classes. Resolves https://github.com/gollum/gollum/issues/1785 (@benjaminwil)

Rakefile

@@ -126,7 +126,7 @@ task :release => :build do
   Rake::Task[:changelog].execute
   sh "git commit --allow-empty -a -m 'Release #{version}'"
   sh "git pull --rebase origin master"
-  sh "git tag v#{version}"
+  sh "git tag -n v#{version}"
   sh "git push origin master"
   sh "git push origin v#{version}"
   sh "gem push pkg/#{name}-#{version}.gem"
@@ -225,21 +225,3 @@ task :changelog do
   `cat #{history_file} >> #{temp.path}`
   `cat #{temp.path} > #{history_file}`
 end
-
-desc 'Precompile assets'
-task :precompile do
-  require './lib/gollum/app.rb'
-  Precious::App.set(:environment, :production)
-  env = Precious::Assets.sprockets
-  path = ENV.fetch('GOLLUM_ASSETS_PATH', ::File.join(File.dirname(__FILE__), 'lib/gollum/public/assets'))
-  manifest = Sprockets::Manifest.new(env, path)
-  Sprockets::Helpers.configure do |config|
-    config.environment = env
-    config.prefix      = Precious::Assets::ASSET_URL
-    config.digest      = true
-    config.public_path = path
-    config.manifest    = manifest
-  end
-  puts "Precompiling assets to #{path}..."
-  manifest.compile(Precious::Assets::MANIFEST)
-end

gemspec.rb

@@ -26,7 +26,6 @@ def specification(version, default_adapter, platform = nil)
     s.add_dependency 'loofah', '~> 2.3'
     s.add_dependency 'github-markup', '~> 4.0'
     s.add_dependency 'gemojione', '~> 4.1'
-    s.add_dependency 'octicons', '~> 17.0'
     s.add_dependency 'twitter-text', '1.14.7'
 
     s.add_development_dependency 'org-ruby', '~> 0.9.9'
@@ -45,7 +44,7 @@ def specification(version, default_adapter, platform = nil)
     s.add_development_dependency 'rb-readline', '~> 0.5.1'
     # updating minitest-reporters requires a new minitest which fails with gollum's tests.
     s.add_development_dependency 'test-unit', '~> 3.3'
-    s.add_development_dependency 'minitest-reporters', '~> 1.4'
+    s.add_development_dependency 'minitest-reporters', '~> 1.5'
     s.add_development_dependency 'nokogiri-diff', '~> 0.2.0'
     s.add_development_dependency 'guard', '~> 2.16'
     s.add_development_dependency 'guard-minitest', '~> 2.4'
@@ -92,7 +91,7 @@ def specification(version, default_adapter, platform = nil)
     lib/gollum-lib/macro/global_toc.rb
     lib/gollum-lib/macro/navigation.rb
     lib/gollum-lib/macro/note.rb
-    lib/gollum-lib/macro/octicon.rb
+    lib/gollum-lib/macro/icon.rb
     lib/gollum-lib/macro/series.rb
     lib/gollum-lib/macro/video.rb
     lib/gollum-lib/macro/warn.rb

lib/gollum-lib/filter/macro.rb

@@ -1,5 +1,5 @@
 # ~*~ encoding: utf-8 ~*~
-require 'octicons'
+
 
 # Replace specified tokens with dynamically generated content.
 class Gollum::Filter::Macro < Gollum::Filter
@@ -53,9 +53,7 @@ def process(data)
         begin
           Gollum::Macro.instance(macro, @markup.wiki, @markup.page).render(*args)
         rescue StandardError => e
-          icon = Octicons::Octicon.new('zap', {width: 24, height: 24})
-          icon.options[:class] << ' mr-2'
-          "<div class='flash flash-error'>#{icon.to_svg}Macro Error for #{macro}: #{e.message}</div>"
+          %Q(<div class="flash flash-error gollum-macro-error my-2">Macro Error for #{macro}: #{e.message}</div>)
         end
       end
     end

lib/gollum-lib/macro.rb

@@ -38,7 +38,7 @@ def initialize(macro_name)
       end
 
       def render(*_args)
-        "!!!Unknown macro: #{@macro_name}!!!"
+        html_error("Unknown macro: #{@macro_name}")
       end
     end
   end

lib/gollum-lib/macro/audio.rb

@@ -2,7 +2,7 @@ module Gollum
   class Macro
     class Audio < Gollum::Macro
       def render (fname)
-        "<audio width=\"100%\" height=\"100%\" src=\"#{CGI::escapeHTML(fname)}\" controls=\"\"> HTML5 audio is not supported on this Browser.</audio>"
+        "<audio width=\"100%\" height=\"100%\" src=\"#{CGI::escapeHTML(fname)}\" controls=\"true\"> HTML5 audio is not supported on this Browser.</audio>"
       end
     end
   end

lib/gollum-lib/macro/flash.rb

@@ -0,0 +1,11 @@
+module Gollum
+  class Macro
+    class Flash < Gollum::Macro
+      def render(message, icon='', type='')
+        flash_type = ['warn', 'error', 'success'].include?(type) ? "flash-#{type}" : '' 
+        flash_icon = icon.empty? ? '' : %Q(data-gollum-icon="#{icon}")
+        %Q(<div class="flash #{flash_type} my-2" #{flash_icon}>#{message}</div>)
+      end
+    end
+  end
+end

lib/gollum-lib/macro/icon.rb

@@ -0,0 +1,9 @@
+module Gollum
+  class Macro
+    class Icon < Gollum::Macro
+      def render(icon)
+        %Q(<div class="gollum-icon" data-gollum-icon="#{icon}"></div>)
+      end
+    end
+  end
+end

lib/gollum-lib/macro/note.rb

@@ -1,18 +1,8 @@
 module Gollum
   class Macro
     class Note < Gollum::Macro
-      def render(notice, octicon = 'info')
-        icon = ""
-        unless octicon.empty?
-          begin
-            icon = Octicons::Octicon.new(octicon, {width: 24, height: 24})
-          rescue RuntimeError
-            icon = Octicons::Octicon.new('info', {width: 24, height: 24})
-          end
-          icon.options[:class] << ' mr-2'
-          icon = icon.to_svg
-        end
-        "<div class='flash'>#{icon}#{notice}</div>"
+      def render(notice)
+        %Q(<div class="flash gollum-note my-2">#{notice}</div>)
       end
     end
   end

lib/gollum-lib/macro/video.rb

@@ -2,7 +2,7 @@ module Gollum
   class Macro
     class Video < Gollum::Macro
       def render (fname)
-        "<video width=\"100%\" height=\"100%\" src=\"#{CGI::escapeHTML(fname)}\" controls=\"\"> HTML5 video is not supported on this Browser.</video>"
+        "<video width=\"100%\" height=\"100%\" src=\"#{CGI::escapeHTML(fname)}\" controls=\"true\"> HTML5 video is not supported on this Browser.</video>"
       end
     end
   end

lib/gollum-lib/macro/warn.rb

@@ -2,9 +2,7 @@ module Gollum
   class Macro
     class Warn < Gollum::Macro
       def render(warning)
-        icon = Octicons::Octicon.new('alert', {width: 24, height: 24})
-        icon.options[:class] << ' mr-2'
-        "<div class='flash flash-warn'>#{icon.to_svg}#{warning}</div>"
+        %Q(<div class="flash flash-warn gollum-warning my-2">#{warning}</div>)
       end
     end
   end

lib/gollum-lib/sanitization.rb

@@ -1,4 +1,5 @@
 ::Loofah::HTML5::SafeList::ACCEPTABLE_PROTOCOLS.add('apt')
+::Loofah::HTML5::SafeList::ALLOWED_ATTRIBUTES.add('controls')
 
 module Gollum
   class Sanitization

lib/gollum-lib/version.rb

@@ -1,5 +1,5 @@
 module Gollum
   module Lib
-    VERSION = '5.2'
+    VERSION = '5.2.2'
   end
 end

test/test_macros.rb

@@ -169,26 +169,29 @@ def render(opts)
     assert_match /<audio (.*) src="#{file}"(.*)> (.*)<\/audio>/, @wiki.pages[0].formatted_data
   end
 
-  test "Octicon macro given a symbol and dimensions displays octicon" do
-    @wiki.write_page("OcticonMacroPage", :markdown, '<<Octicon("globe", 64, 64)>>', commit_details)
-    assert_match /<div><svg.*class=\"octicon octicon-globe\".*/, @wiki.pages[0].formatted_data
-    assert_match /<div><svg.*height=\"64\"/, @wiki.pages[0].formatted_data
-    assert_match /<div><svg.*width=\"64\"/, @wiki.pages[0].formatted_data
+  test "Icon macro given a symbol renders gollum-icon div with data-gollum-icon attribute" do
+    @wiki.write_page("IconMacroPage", :markdown, '<<Icon("globe")>>', commit_details)
+    assert_match /<div class=\"gollum-icon\".*data-gollum-icon=\"globe\".*/, @wiki.pages[0].formatted_data
+  end
+
+  test "Flash macro given a type and icon name renders gollum-flash with data-* attributes" do
+    @wiki.write_page("FlashMacroPage", :markdown, '<<Flash("Not all those who wander are lost", "bell", "success")>>', commit_details)
+    assert_match /<div class=\"flash flash-success my-2\".*data-gollum-icon=\"bell\".*/, @wiki.pages[0].formatted_data
   end
 
   test "Note macro given a string displays a regular flash message box" do
     @wiki.write_page("NoteMacroPage", :markdown, '<<Note("Did you know Bilbo is a Hobbit?")>>', commit_details)
-    assert_match /<div class=\"flash\"><svg.*class=\"octicon octicon-info mr-2\".*Did you know Bilbo.*/, @wiki.pages[0].formatted_data
+    assert_match /<div class=\"flash gollum-note my-2\">.*Did you know Bilbo.*/, @wiki.pages[0].formatted_data
   end
 
   test "Warn macro given a string displays a flash-warning message box" do
     @wiki.write_page("WarnMacroPage", :markdown, '<<Warn("Be careful not to mention hobbits in conversation too much.")>>', commit_details)
-    assert_match /<div class=\"flash flash-warn\"><svg.*class=\"octicon octicon-alert mr-2\".*Be careful.*/, @wiki.pages[0].formatted_data
+    assert_match /<div class=\"flash flash-warn gollum-warning my-2\">.*Be careful.*/, @wiki.pages[0].formatted_data
   end
 
   test "Macro errors are reported in place in a flash-error message box" do
-    @wiki.write_page("OcticonMacroPage", :markdown, '<<Octicon("foobar", 64, 64)>>', commit_details)
-    assert_match /<div class=\"flash flash-error\"><svg.*class=\"octicon octicon-zap mr-2\".*Macro Error for Octicon: Couldn't find octicon symbol for "foobar".*/, @wiki.pages[0].formatted_data
+    @wiki.write_page("IconMacroPage", :markdown, '<<Note("foobar", 64, 64)>>', commit_details)
+    assert_match /<div class=\"flash flash-error gollum-macro-error my-2\">Macro Error for Note: wrong number of arguments.*/, @wiki.pages[0].formatted_data
   end
 
   test "Macros escape HTML" do
@@ -198,7 +201,7 @@ def render(opts)
 
   test "Note macro renders HTML code" do
     @wiki.write_page("HTMLNoteMacroPage", :markdown, '<<Note("<span>test</span>")>>', commit_details)
-    assert_match /<div class=\"flash\"><svg.*class=\"octicon octicon-info mr-2\".*<span>test<\/span>.*/, @wiki.pages[0].formatted_data
+    assert_match /<div class=\"flash gollum-note my-2\".*<span>test<\/span>.*/, @wiki.pages[0].formatted_data
   end
 
   test "Series macro escapes page names" do
@@ -207,4 +210,10 @@ def render(opts)
     @wiki.write_page("_Footer", :markdown, "<<Series(test)>>", commit_details)
     assert_match /Next(.*)test-2&lt;span&gt;/, @wiki.page("test-1").footer.formatted_data
   end
+
+  test "Control attributes for Audio and Video are not sanitized" do
+    @wiki.write_page("AudioTagTest", :markdown, "<<Audio(foo)>>\n<<Video(bar)>>", commit_details)
+    # The Macros must return controls=true until https://github.com/flavorjones/loofah/issues/242 is resolved
+    assert_match /<audio (.*)controls(.*)>(.*)<video (.*)controls(.*)>/m, @wiki.pages[0].formatted_data
+  end
 end