From 8ee382087d06c50d2f8c7f60ce79294af0b89201 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 18 May 2015 15:44:45 -0400
Subject: [PATCH] Subclass TaskList::Filter to fix a bug

Instead of using a fork, we subclass the filter and only apply the
`task-list` class to list items that actually are task lists.

Closes #1645

See https://github.com/github/task_list/pull/60
---
 Gemfile                                       |  2 +-
 Gemfile.lock                                  |  6 ++---
 app/assets/stylesheets/pages/notes.scss       |  4 ++--
 lib/gitlab/markdown.rb                        |  4 ++--
 lib/gitlab/markdown/task_list_filter.rb       | 23 +++++++++++++++++++
 .../gitlab/markdown/task_list_filter_spec.rb  | 14 +++++++++++
 6 files changed, 45 insertions(+), 8 deletions(-)
 create mode 100644 lib/gitlab/markdown/task_list_filter.rb
 create mode 100644 spec/lib/gitlab/markdown/task_list_filter_spec.rb

diff --git a/Gemfile b/Gemfile
index f7da36be948..59adf6d2d65 100644
--- a/Gemfile
+++ b/Gemfile
@@ -94,7 +94,7 @@ gem "seed-fu"
 
 # Markdown and HTML processing
 gem 'html-pipeline', '~> 1.11.0'
-gem 'task_list',     '~> 1.0.0', require: 'task_list/railtie'
+gem 'task_list',     '1.0.2', require: 'task_list/railtie'
 gem 'github-markup'
 gem 'redcarpet',     '~> 3.2.3'
 gem 'RedCloth'
diff --git a/Gemfile.lock b/Gemfile.lock
index b6cf03b0fd4..9ea77151c74 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -338,7 +338,7 @@ GEM
     method_source (0.8.2)
     mime-types (1.25.1)
     mimemagic (0.3.0)
-    mini_portile (0.6.1)
+    mini_portile (0.6.2)
     minitest (5.3.5)
     mousetrap-rails (1.4.6)
     multi_json (1.10.1)
@@ -350,7 +350,7 @@ GEM
       net-ssh (>= 2.6.5)
     net-ssh (2.8.0)
     newrelic_rpm (3.9.4.245)
-    nokogiri (1.6.5)
+    nokogiri (1.6.6.2)
       mini_portile (~> 0.6.0)
     nprogress-rails (0.1.2.3)
     oauth (0.4.7)
@@ -802,7 +802,7 @@ DEPENDENCIES
   spring-commands-spinach (= 1.0.0)
   stamp
   state_machine
-  task_list (~> 1.0.0)
+  task_list (= 1.0.2)
   test_after_commit
   thin
   tinder (~> 1.9.2)
diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss
index e943be67dbf..42b8ecabb38 100644
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@@ -79,11 +79,11 @@ ul.notes {
         word-wrap: break-word;
         @include md-typography;
 
-        // Reduce left padding of first ul element
+        // Reduce left padding of first task list ul element
         ul.task-list:first-child {
           padding-left: 10px;
 
-          // sub-lists should be padded normally
+          // sub-tasks should be padded normally
           ul {
             padding-left: 20px;
           }
diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb
index 133010adcaf..c0fb22e7f36 100644
--- a/lib/gitlab/markdown.rb
+++ b/lib/gitlab/markdown.rb
@@ -1,5 +1,4 @@
 require 'html/pipeline'
-require 'task_list/filter'
 
 module Gitlab
   # Custom parser for GitLab-flavored Markdown
@@ -19,6 +18,7 @@ module Gitlab
     autoload :SanitizationFilter,           'gitlab/markdown/sanitization_filter'
     autoload :SnippetReferenceFilter,       'gitlab/markdown/snippet_reference_filter'
     autoload :TableOfContentsFilter,        'gitlab/markdown/table_of_contents_filter'
+    autoload :TaskListFilter,               'gitlab/markdown/task_list_filter'
     autoload :UserReferenceFilter,          'gitlab/markdown/user_reference_filter'
 
     # Public: Parse the provided text with GitLab-Flavored Markdown
@@ -113,7 +113,7 @@ module Gitlab
         Gitlab::Markdown::CommitReferenceFilter,
         Gitlab::Markdown::LabelReferenceFilter,
 
-        TaskList::Filter
+        Gitlab::Markdown::TaskListFilter
       ]
     end
   end
diff --git a/lib/gitlab/markdown/task_list_filter.rb b/lib/gitlab/markdown/task_list_filter.rb
new file mode 100644
index 00000000000..c6eb2e2bf6d
--- /dev/null
+++ b/lib/gitlab/markdown/task_list_filter.rb
@@ -0,0 +1,23 @@
+require 'task_list/filter'
+
+module Gitlab
+  module Markdown
+    # Work around a bug in the default TaskList::Filter that adds a `task-list`
+    # class to every list element, regardless of whether or not it contains a
+    # task list.
+    #
+    # This is a (hopefully) temporary fix, pending a new release of the
+    # task_list gem.
+    #
+    # See https://github.com/github/task_list/pull/60
+    class TaskListFilter < TaskList::Filter
+      def add_css_class(node, *new_class_names)
+        if new_class_names.include?('task-list')
+          super if node.children.any? { |c| c['class'] == 'task-list-item' }
+        else
+          super
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/markdown/task_list_filter_spec.rb b/spec/lib/gitlab/markdown/task_list_filter_spec.rb
new file mode 100644
index 00000000000..2a1e1cc5127
--- /dev/null
+++ b/spec/lib/gitlab/markdown/task_list_filter_spec.rb
@@ -0,0 +1,14 @@
+require 'spec_helper'
+
+module Gitlab::Markdown
+  describe TaskListFilter do
+    def filter(html, options = {})
+      described_class.call(html, options)
+    end
+
+    it 'does not apply `task-list` class to non-task lists' do
+      exp = act = %(<ul><li>Item</li></ul>)
+      expect(filter(act).to_html).to eq exp
+    end
+  end
+end