Merge branch 'anakashima/gitlab-ce-fix_wiki_toc_indent' into 'master'

Wiki table of contents are now properly nested to reflect header level

See merge request !13909

3 files changed, 113 insertions, 18 deletions

diff --git a/changelogs/unreleased/fix_wiki_toc_indent.yml b/changelogs/unreleased/fix_wiki_toc_indent.yml
new file mode 100644
index 00000000000..60da2e455f2
--- /dev/null
+++ b/changelogs/unreleased/fix_wiki_toc_indent.yml
@@ -0,0 +1,5 @@
+---
+title: Wiki table of contents are now properly nested to reflect header level
+merge_request: 13650
+author: Akihiro Nakashima
+type: fixed
diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb
index 8e7084f2543..47151626208 100644
--- a/lib/banzai/filter/table_of_contents_filter.rb
+++ b/lib/banzai/filter/table_of_contents_filter.rb
@@ -22,40 +22,94 @@ module Banzai
         result[:toc] = ""
 
         headers = Hash.new(0)
+        header_root = current_header = HeaderNode.new
 
         doc.css('h1, h2, h3, h4, h5, h6').each do |node|
-          text = node.text
+          if header_content = node.children.first
+            id = node
+              .text
+              .downcase
+              .gsub(PUNCTUATION_REGEXP, '') # remove punctuation
+              .tr(' ', '-') # replace spaces with dash
+              .squeeze('-') # replace multiple dashes with one
 
-          id = text.downcase
-          id.gsub!(PUNCTUATION_REGEXP, '') # remove punctuation
-          id.tr!(' ', '-') # replace spaces with dash
-          id.squeeze!('-') # replace multiple dashes with one
+            uniq = headers[id] > 0 ? "-#{headers[id]}" : ''
+            headers[id] += 1
+            href = "#{id}#{uniq}"
 
-          uniq = (headers[id] > 0) ? "-#{headers[id]}" : ''
-          headers[id] += 1
+            current_header = HeaderNode.new(node: node, href: href, previous_header: current_header)
 
-          if header_content = node.children.first
-            # namespace detection will be automatically handled via javascript (see issue #22781)
-            namespace = "user-content-"
-            href = "#{id}#{uniq}"
-            push_toc(href, text)
-            header_content.add_previous_sibling(anchor_tag("#{namespace}#{href}", href))
+            header_content.add_previous_sibling(anchor_tag(href))
           end
         end
 
-        result[:toc] = %Q{<ul class="section-nav">\n#{result[:toc]}</ul>} unless result[:toc].empty?
+        push_toc(header_root.children, root: true)
 
         doc
       end
 
       private
 
-      def anchor_tag(id, href)
-        %Q{<a id="#{id}" class="anchor" href="##{href}" aria-hidden="true"></a>}
+      def anchor_tag(href)
+        %Q{<a id="user-content-#{href}" class="anchor" href="##{href}" aria-hidden="true"></a>}
       end
 
-      def push_toc(href, text)
-        result[:toc] << %Q{<li><a href="##{href}">#{text}</a></li>\n}
+      def push_toc(children, root: false)
+        return if children.empty?
+
+        klass = ' class="section-nav"' if root
+
+        result[:toc] << "<ul#{klass}>"
+        children.each { |child| push_anchor(child) }
+        result[:toc] << '</ul>'
+      end
+
+      def push_anchor(header_node)
+        result[:toc] << %Q{<li><a href="##{header_node.href}">#{header_node.text}</a>}
+        push_toc(header_node.children)
+        result[:toc] << '</li>'
+      end
+
+      class HeaderNode
+        attr_reader :node, :href, :parent, :children
+
+        def initialize(node: nil, href: nil, previous_header: nil)
+          @node = node
+          @href = href
+          @children = []
+
+          @parent = find_parent(previous_header)
+          @parent.children.push(self) if @parent
+        end
+
+        def level
+          return 0 unless node
+
+          @level ||= node.name[1].to_i
+        end
+
+        def text
+          return '' unless node
+
+          @text ||= node.text
+        end
+
+        private
+
+        def find_parent(previous_header)
+          return unless previous_header
+
+          if level == previous_header.level
+            parent = previous_header.parent
+          elsif level > previous_header.level
+            parent = previous_header
+          else
+            parent = previous_header
+            parent = parent.parent while parent.level >= level
+          end
+
+          parent
+        end
       end
     end
   end
diff --git a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb
index ff6b19459bb..85eddde732e 100644
--- a/spec/lib/banzai/filter/table_of_contents_filter_spec.rb
+++ b/spec/lib/banzai/filter/table_of_contents_filter_spec.rb
@@ -96,5 +96,41 @@ describe Banzai::Filter::TableOfContentsFilter do
       expect(links.last.attr('href')).to eq '#header-2'
       expect(links.last.text).to eq 'Header 2'
     end
+
+    context 'table of contents nesting' do
+      let(:results) do
+        result(
+          header(1, 'Header 1') <<
+          header(2, 'Header 1-1') <<
+          header(3, 'Header 1-1-1') <<
+          header(2, 'Header 1-2') <<
+          header(1, 'Header 2') <<
+          header(2, 'Header 2-1')
+        )
+      end
+
+      it 'keeps list levels regarding header levels' do
+        items = doc.css('li')
+
+        # Header 1
+        expect(items[0].ancestors).to satisfy_none { |node| node.name == 'li' }
+
+        # Header 1-1
+        expect(items[1].ancestors).to include(items[0])
+
+        # Header 1-1-1
+        expect(items[2].ancestors).to include(items[0], items[1])
+
+        # Header 1-2
+        expect(items[3].ancestors).to include(items[0])
+        expect(items[3].ancestors).not_to include(items[1])
+
+        # Header 2
+        expect(items[4].ancestors).to satisfy_none { |node| node.name == 'li' }
+
+        # Header 2-1
+        expect(items[5].ancestors).to include(items[4])
+      end
+    end
   end
 end