From 0ebf15a2bf7d8f37399a3295f955c3464157b507 Mon Sep 17 00:00:00 2001
From: Bart Kamphorst <bart@kamphorst.com>
Date: Tue, 1 Aug 2023 16:27:00 +0200
Subject: [PATCH] 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                      | 10 ++++++++++
 Rakefile                        | 20 +-------------------
 gemspec.rb                      |  5 ++---
 lib/gollum-lib/filter/macro.rb  |  6 ++----
 lib/gollum-lib/macro.rb         |  2 +-
 lib/gollum-lib/macro/audio.rb   |  2 +-
 lib/gollum-lib/macro/flash.rb   | 11 +++++++++++
 lib/gollum-lib/macro/icon.rb    |  9 +++++++++
 lib/gollum-lib/macro/note.rb    | 14 ++------------
 lib/gollum-lib/macro/octicon.rb | 12 ------------
 lib/gollum-lib/macro/video.rb   |  2 +-
 lib/gollum-lib/macro/warn.rb    |  4 +---
 lib/gollum-lib/sanitization.rb  |  1 +
 lib/gollum-lib/version.rb       |  2 +-
 test/test_macros.rb             | 29 +++++++++++++++++++----------
 15 files changed, 62 insertions(+), 67 deletions(-)
 create mode 100644 lib/gollum-lib/macro/flash.rb
 create mode 100644 lib/gollum-lib/macro/icon.rb
 delete mode 100644 lib/gollum-lib/macro/octicon.rb

diff --git a/HISTORY.md b/HISTORY.md
index 68479dc01..04190a138 100644
--- a/HISTORY.md
+++ b/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)
diff --git a/Rakefile b/Rakefile
index f95bc2d3d..aa17492e0 100644
--- a/Rakefile
+++ b/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
diff --git a/gemspec.rb b/gemspec.rb
index 9eb487bb7..01d9b5e56 100644
--- a/gemspec.rb
+++ b/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
diff --git a/lib/gollum-lib/filter/macro.rb b/lib/gollum-lib/filter/macro.rb
index 5f79ac108..03e0ba88e 100644
--- a/lib/gollum-lib/filter/macro.rb
+++ b/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
diff --git a/lib/gollum-lib/macro.rb b/lib/gollum-lib/macro.rb
index 870bf4750..077c645d0 100644
--- a/lib/gollum-lib/macro.rb
+++ b/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
diff --git a/lib/gollum-lib/macro/audio.rb b/lib/gollum-lib/macro/audio.rb
index e222213ce..66826acb1 100755
--- a/lib/gollum-lib/macro/audio.rb
+++ b/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
diff --git a/lib/gollum-lib/macro/flash.rb b/lib/gollum-lib/macro/flash.rb
new file mode 100644
index 000000000..9c7a91890
--- /dev/null
+++ b/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
\ No newline at end of file
diff --git a/lib/gollum-lib/macro/icon.rb b/lib/gollum-lib/macro/icon.rb
new file mode 100644
index 000000000..7b9bdd8bf
--- /dev/null
+++ b/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
\ No newline at end of file
diff --git a/lib/gollum-lib/macro/note.rb b/lib/gollum-lib/macro/note.rb
index 4c0a7c7bf..67133b430 100644
--- a/lib/gollum-lib/macro/note.rb
+++ b/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
diff --git a/lib/gollum-lib/macro/octicon.rb b/lib/gollum-lib/macro/octicon.rb
deleted file mode 100644
index de8ab8ff0..000000000
--- a/lib/gollum-lib/macro/octicon.rb
+++ /dev/null
@@ -1,12 +0,0 @@
-module Gollum
-  class Macro
-    class Octicon < Gollum::Macro
-      def render(symbol, height = nil, width = nil)
-        parameters = {}
-        parameters[:height] = height if height
-        parameters[:width]  = width if width
-        "<div>#{Octicons::Octicon.new(symbol, parameters).to_svg}</div>"
-      end
-    end
-  end
-end
\ No newline at end of file
diff --git a/lib/gollum-lib/macro/video.rb b/lib/gollum-lib/macro/video.rb
index 9c64a0965..e2f05ebfa 100644
--- a/lib/gollum-lib/macro/video.rb
+++ b/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
diff --git a/lib/gollum-lib/macro/warn.rb b/lib/gollum-lib/macro/warn.rb
index 892b8a36d..1e7cc502b 100644
--- a/lib/gollum-lib/macro/warn.rb
+++ b/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
diff --git a/lib/gollum-lib/sanitization.rb b/lib/gollum-lib/sanitization.rb
index d5a79f02d..0f6ddf9f8 100644
--- a/lib/gollum-lib/sanitization.rb
+++ b/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
diff --git a/lib/gollum-lib/version.rb b/lib/gollum-lib/version.rb
index 3e9826030..14e9b9aeb 100644
--- a/lib/gollum-lib/version.rb
+++ b/lib/gollum-lib/version.rb
@@ -1,5 +1,5 @@
 module Gollum
   module Lib
-    VERSION = '5.2'
+    VERSION = '5.2.2'
   end
 end
diff --git a/test/test_macros.rb b/test/test_macros.rb
index ff535c022..ff633cea3 100644
--- a/test/test_macros.rb
+++ b/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