Merge pull request #6398 from opf/feature/toc_wysiwyg

Bump ckeditor to 10.1.0 and enables toc macro

[ci skip]

app/assets/stylesheets/content/editor/_ckeditor.sass

@@ -17,3 +17,13 @@
   // Min height for the editable section
   .ck-editor__editable
     min-height: 20vh
+
+.macro
+  display: flex
+  align-items: center
+  flex-direction: column
+  background: #EEEEEE
+
+  svg
+    height: 10%
+    width: 10%

config/locales/en.yml

@@ -1471,7 +1471,7 @@ en:
   label_summary: "Summary"
   label_system: "System"
   label_system_storage: "Storage information"
-  label_table_of_contents: "Table of Contents"
+  label_table_of_contents: "Table of contents"
   label_tag: "Tag"
   label_text: "Long text"
   label_this_month: "this month"

config/locales/js-en.yml

@@ -81,6 +81,9 @@ en:
     description_select_work_package: "Select work package #%{id}"
     description_selected_columns: "Selected Columns"
     description_subwork_package: "Child of work package #%{id}"
+    editor:
+      macro:
+        toc: 'Table of contents'
     error:
       internal: "An internal error has occurred."
       cannot_save_changes_with_message: "Cannot save your changes due to the following error: %{error}"

lib/open_project/a_r_object_cache.rb

@@ -1,14 +0,0 @@
-module OpenProject
-  class ARObjectCache
-    def self.within(&block)
-      ActiveRecord::Base.extend(::OpenProject::ARObjectCache::ObjectCache) unless ActiveRecord::Base.ancestors.include?(::OpenProject::ARObjectCache::ObjectCache)
-      block.call
-    end
-
-    module ObjectCache
-      def find(*args)
-        super
-      end
-    end
-  end
-end

lib/open_project/text_formatting/filters/macro_filter.rb

@@ -30,15 +30,26 @@
 
 module OpenProject::TextFormatting
   module Filters
-      def anchor_icon
-        %Q(<span aria-hidden="true" class="wiki-anchor"></span>)
+    class MacroFilter < HTML::Pipeline::Filter
+      cattr_accessor :registered
+
+      def self.register(macro)
+        self.registered ||= []
+
+        registered << macro
       end
 
       def call
-        return doc if context[:headings] == false
+        doc.search('macro').each do |macro|
+          registered.each do |macro_class|
+            macro_class.apply_conditionally(macro, result)
+          end
+        end
 
-        super
+        doc
       end
     end
   end
 end
+
+OpenProject::TextFormatting::Filters::MacroFilter.register(OpenProject::TextFormatting::Filters::Macros::Toc)

lib/open_project/text_formatting/filters/macros/toc.rb

@@ -0,0 +1,55 @@
+#-- encoding: UTF-8
+
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2017 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2017 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+module OpenProject::TextFormatting::Filters::Macros
+  module Toc
+    HTML_CLASS = 'toc'.freeze
+
+    module_function
+
+    def apply_conditionally(macro, result)
+      insert_toc(macro, result) if is?(macro)
+    end
+
+    def insert_toc(macro, result)
+      raise 'The HTML::Pipeline::TableOfContentsFilters needs to run before.' unless result[:toc]
+
+      macro.replace(heading + result[:toc])
+    end
+
+    def is?(macro)
+      macro['class'].include?(HTML_CLASS)
+    end
+
+    def heading
+      "<h1>#{I18n.t(:label_table_of_contents)}</h1>"
+    end
+  end
+end

lib/open_project/text_formatting/filters/markdown_filter.rb

@@ -31,7 +31,6 @@
 module OpenProject::TextFormatting
   module Filters
     class MarkdownFilter < HTML::Pipeline::MarkdownFilter
-
       # Convert Markdown to HTML using CommonMarker
       def call
         options = [:GITHUB_PRE_LANG]

lib/open_project/text_formatting/filters/pattern_matcher_filter.rb

@@ -31,7 +31,6 @@
 module OpenProject::TextFormatting
   module Filters
     class PatternMatcherFilter < HTML::Pipeline::Filter
-
       # Skip text nodes that are within preformatted blocks
       PREFORMATTED_BLOCKS = %w(pre code).to_set
 
@@ -42,7 +41,6 @@ module OpenProject::TextFormatting
         ]
       end
 
-
       def call
         doc.search('.//text()').each do |node|
           next if has_ancestor?(node, PREFORMATTED_BLOCKS)

lib/open_project/text_formatting/filters/sanitization_filter.rb

@@ -31,6 +31,8 @@
 module OpenProject::TextFormatting
   module Filters
     class SanitizationFilter < HTML::Pipeline::SanitizationFilter
+      WHITELIST[:elements] << 'macro'
+      WHITELIST[:attributes].merge!('macro' => ['class'])
     end
   end
 end

lib/open_project/text_formatting/formatters/markdown/formatter.rb

@@ -53,22 +53,12 @@ module OpenProject::TextFormatting::Formatters
         [
           :markdown,
           :sanitization,
+          HTML::Pipeline::TableOfContentsFilter,
+          :macro,
           :pattern_matcher,
           :autolink
         ]
       end
-
-      protected
-
-      def located_filters
-        filters.map do |f|
-          if [Symbol, String].include? f.class
-            OpenProject::TextFormatting::Filters.const_get("#{f}_filter".classify)
-          else
-            f
-          end
-        end
-      end
     end
   end
 end

spec/features/work_packages/attachments/attachment_upload_spec.rb

@@ -18,7 +18,7 @@ describe 'Upload attachment to work package', js: true do
   let(:wp_page) { ::Pages::FullWorkPackage.new(work_package, project) }
   let(:attachments) { ::Components::Attachments.new }
   let(:field) { WorkPackageEditorField.new wp_page, 'description' }
-  let(:image_fixture) {  Rails.root.join('spec/fixtures/files/image.png') }
+  let(:image_fixture) { Rails.root.join('spec/fixtures/files/image.png') }
 
   before do
     login_as(dev)
@@ -32,8 +32,13 @@ describe 'Upload attachment to work package', js: true do
       target = find('.op-ckeditor-element')
       attachments.drag_and_drop_file(target, image_fixture)
 
+      # Besides testing caption functionality this also slows down clicking on the submit button
+      # so that the image is properly embedded
+      page.find('figure.image figcaption').base.send_keys('Some image caption')
+
       field.submit_by_click
       expect(field.display_element).to have_selector('img')
+      expect(field.display_element).to have_content('Some image caption')
     end
   end
 

spec/features/wysiwyg/tables_spec.rb

@@ -49,9 +49,11 @@ describe 'Wysiwyg tables',
         editor.in_editor do |container, editable|
           # strangely, we need visible: :all here
           container.find('.ck-button', visible: :all, text: 'Insert table').click
+          # 2x2
+          container.find('.ck-insert-table-dropdown-grid-box:nth-of-type(12)').click
 
           # Edit table
-          tds = editable.all('table.ck-widget td')
+          tds = editable.all('.table.ck-widget td')
           values = %w(h1 h2 c1 c2)
           expect(tds.length).to eq(4)
 

spec/lib/open_project/text_formatting/markdown/markdown_formatting_spec.rb

@@ -52,20 +52,28 @@ describe OpenProject::TextFormatting::Formatters::Markdown::Formatter do
 
   it 'should escaping' do
     assert_html_output(
-      'this is a <script>'      => 'this is a &lt;script&gt;'
+      'this is a <script>' => 'this is a &lt;script&gt;'
     )
   end
 
   it 'should use of backslashes followed by numbers in headers' do
+    html = <<-HTML.strip_heredoc
+      <h1>
+        <a id="20090209" class="anchor" href="#20090209" aria-hidden="true">
+          <span aria-hidden="true" class="octicon octicon-link"></span>
+        </a>
+        2009\\02\\09
+      </h1>
+    HTML
     assert_html_output({
-                         '# 2009\02\09'      => '<h1>2009\02\09</h1>'
+                         '# 2009\02\09' => html
                        }, false)
   end
 
   it 'should double dashes should not strikethrough' do
     assert_html_output(
-      'double -- dashes -- test'  => 'double -- dashes -- test',
-      'double -- **dashes** -- test'  => 'double -- <strong>dashes</strong> -- test'
+      'double -- dashes -- test' => 'double -- dashes -- test',
+      'double -- **dashes** -- test' => 'double -- <strong>dashes</strong> -- test'
     )
   end
 
@@ -81,7 +89,6 @@ describe OpenProject::TextFormatting::Formatters::Markdown::Formatter do
     )
   end
 
-
   describe 'mail address autolink' do
     it 'prints autolinks for user references not existing' do
       assert_html_output(
@@ -94,16 +101,16 @@ describe OpenProject::TextFormatting::Formatters::Markdown::Formatter do
       let(:role) { FactoryBot.create(:role, permissions: %i(view_work_packages)) }
       let(:current_user) do
         FactoryBot.create(:user,
-                           member_in_project: project,
-                           member_through_role: role)
+                          member_in_project: project,
+                          member_through_role: role)
       end
       let(:user) do
         FactoryBot.create(:user,
-                           login: 'foo@bar.com',
-                           firstname: 'Foo',
-                           lastname: 'Barrit',
-                           member_in_project: project,
-                           member_through_role: role)
+                          login: 'foo@bar.com',
+                          firstname: 'Foo',
+                          lastname: 'Barrit',
+                          member_in_project: project,
+                          member_through_role: role)
       end
 
       before do
@@ -121,71 +128,71 @@ describe OpenProject::TextFormatting::Formatters::Markdown::Formatter do
 
   it 'should blockquote' do
     # orig raw text
-    raw = <<-RAW
-John said:
-> Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Maecenas sed libero.
-> Nullam commodo metus accumsan nulla. Curabitur lobortis dui id dolor.
->
-> * Donec odio lorem,
-> * sagittis ac,
-> * malesuada in,
-> * adipiscing eu, dolor.
->
-> >Nulla varius pulvinar diam. Proin id arcu id lorem scelerisque condimentum. Proin vehicula turpis vitae lacus.
->
-> Proin a tellus. Nam vel neque.
-
-He's right.
-RAW
+    raw = <<-RAW.strip_heredoc
+      John said:
+      > Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Maecenas sed libero.
+      > Nullam commodo metus accumsan nulla. Curabitur lobortis dui id dolor.
+      >
+      > * Donec odio lorem,
+      > * sagittis ac,
+      > * malesuada in,
+      > * adipiscing eu, dolor.
+      >
+      > >Nulla varius pulvinar diam. Proin id arcu id lorem scelerisque condimentum. Proin vehicula turpis vitae lacus.
+      >
+      > Proin a tellus. Nam vel neque.
+
+      He's right.
+    RAW
 
     # expected html
-    expected = <<-EXPECTED
-<p>John said:</p>
-<blockquote>
-<p>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Maecenas sed libero.<br>
-Nullam commodo metus accumsan nulla. Curabitur lobortis dui id dolor.</p>
-<ul>
-  <li>Donec odio lorem,</li>
-  <li>sagittis ac,</li>
-  <li>malesuada in,</li>
-  <li>adipiscing eu, dolor.</li>
-</ul>
-<blockquote>
-<p>Nulla varius pulvinar diam. Proin id arcu id lorem scelerisque condimentum. Proin vehicula turpis vitae lacus.</p>
-</blockquote>
-<p>Proin a tellus. Nam vel neque.</p>
-</blockquote>
-<p>He's right.</p>
-EXPECTED
+    expected = <<-EXPECTED.strip_heredoc
+      <p>John said:</p>
+      <blockquote>
+      <p>Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Maecenas sed libero.<br>
+      Nullam commodo metus accumsan nulla. Curabitur lobortis dui id dolor.</p>
+      <ul>
+        <li>Donec odio lorem,</li>
+        <li>sagittis ac,</li>
+        <li>malesuada in,</li>
+        <li>adipiscing eu, dolor.</li>
+      </ul>
+      <blockquote>
+      <p>Nulla varius pulvinar diam. Proin id arcu id lorem scelerisque condimentum. Proin vehicula turpis vitae lacus.</p>
+      </blockquote>
+      <p>Proin a tellus. Nam vel neque.</p>
+      </blockquote>
+      <p>He's right.</p>
+    EXPECTED
 
     expect(to_html(raw).gsub(%r{\s+}, '')).to eq(expected.gsub(%r{\s+}, ''))
   end
 
   it 'should table' do
-    raw = <<-RAW
-This is a table with header cells:
-
-|header|header|
-|------|------|
-|cell11|cell12|
-|cell21|cell23|
-|cell31|cell32|
-RAW
-
-    expected = <<-EXPECTED
-<p>This is a table with header cells:</p>
-
-<table>
-  <thead>
-    <tr><th>header</th><th>header</th></tr>
-  </thead>
-  <tbody>
-  <tr><td>cell11</td><td>cell12</td></tr>
-  <tr><td>cell21</td><td>cell23</td></tr>
-  <tr><td>cell31</td><td>cell32</td></tr>
-  </tbody>
-</table>
-EXPECTED
+    raw = <<-RAW.strip_heredoc
+      This is a table with header cells:
+
+      |header|header|
+      |------|------|
+      |cell11|cell12|
+      |cell21|cell23|
+      |cell31|cell32|
+    RAW
+
+    expected = <<-EXPECTED.strip_heredoc
+      <p>This is a table with header cells:</p>
+
+      <table>
+        <thead>
+          <tr><th>header</th><th>header</th></tr>
+        </thead>
+        <tbody>
+        <tr><td>cell11</td><td>cell12</td></tr>
+        <tr><td>cell21</td><td>cell23</td></tr>
+        <tr><td>cell31</td><td>cell32</td></tr>
+        </tbody>
+      </table>
+    EXPECTED
 
     expect(to_html(raw).gsub(%r{\s+}, '')).to eq(expected.gsub(%r{\s+}, ''))
   end
@@ -202,6 +209,88 @@ EXPECTED
     expect(expected.gsub(%r{\s+}, '')).to eq(to_html(raw).gsub(%r{\s+}, ''))
   end
 
+  it 'inserts table of contents triggered by a macro' do
+    markdown = <<-MARKDOWN.strip_heredoc
+      <macro class="toc"></macro>
+
+      # The first h1 heading
+
+      Some text after the first h1 heading
+
+      ## The first h2 heading
+
+      Some text after the first h2 heading
+
+      ### The first h3 heading
+
+      Some text after the first h3 heading
+
+      # The second h1 heading
+
+      Some text after the second h1 heading
+
+      ## The second h2 heading
+
+      Some text after the second h2 heading
+
+      ### The second h3 heading
+
+      Some text after the second h3 heading
+    MARKDOWN
+
+    html = <<-HTML.strip_heredoc
+      <p><h1>Table of contents</h1><ul class="section-nav">
+      <li><a href="#the-first-h1-heading">The first h1 heading</a></li>
+      <li><a href="#the-first-h2-heading">The first h2 heading</a></li>
+      <li><a href="#the-first-h3-heading">The first h3 heading</a></li>
+      <li><a href="#the-second-h1-heading">The second h1 heading</a></li>
+      <li><a href="#the-second-h2-heading">The second h2 heading</a></li>
+      <li><a href="#the-second-h3-heading">The second h3 heading</a></li>
+      </ul></p>
+      <h1>
+        <a id="the-first-h1-heading" class="anchor" href="#the-first-h1-heading" aria-hidden="true">
+          <span aria-hidden="true" class="octicon octicon-link"></span>
+        </a>
+        The first h1 heading
+      </h1>
+      <p>Some text after the first h1 heading</p>
+      <h2>
+        <a id="the-first-h2-heading" class="anchor" href="#the-first-h2-heading" aria-hidden="true">
+          <span aria-hidden="true" class="octicon octicon-link"></span>
+        </a>
+        The first h2 heading
+      </h2>
+      <p>Some text after the first h2 heading</p>
+      <h3>
+        <a id="the-first-h3-heading" class="anchor" href="#the-first-h3-heading" aria-hidden="true">
+          <span aria-hidden="true" class="octicon octicon-link"></span>
+        </a>
+        The first h3 heading
+      </h3>
+      <p>Some text after the first h3 heading</p>
+      <h1>
+        <a id="the-second-h1-heading" class="anchor" href="#the-second-h1-heading" aria-hidden="true">
+          <span aria-hidden="true" class="octicon octicon-link"></span>
+        </a>The second h1 heading
+      </h1>
+      <p>Some text after the second h1 heading</p>
+      <h2>
+        <a id="the-second-h2-heading" class="anchor" href="#the-second-h2-heading" aria-hidden="true">
+          <span aria-hidden="true" class="octicon octicon-link"></span>
+        </a>The second h2 heading
+      </h2>
+      <p>Some text after the second h2 heading</p>
+      <h3>
+        <a id="the-second-h3-heading" class="anchor" href="#the-second-h3-heading" aria-hidden="true">
+          <span aria-hidden="true" class="octicon octicon-link"></span>
+        </a>The second h3 heading
+      </h3>
+      <p>Some text after the second h3 heading</p>
+    HTML
+
+    assert_html_output({ markdown => html }, false)
+  end
+
   private
 
   def assert_html_output(to_test, expect_paragraph = true)

spec/lib/open_project/text_formatting/textile/textile_spec.rb

@@ -720,7 +720,7 @@ h2. Attributes
         expect(html).to be_html_eql(%{
           <fieldset class='form--fieldset -collapsible'>
             <legend class='form--fieldset-legend' title='Show/Hide table of contents'>
-              <a href='#'>Table of Contents</a>
+              <a href='#'>Table of contents</a>
             </legend>
             <div>
               <ul class="toc">

spec/requests/api/v3/work_package_resource_spec.rb

@@ -183,7 +183,7 @@ describe 'API v3 Work package resource', type: :request, content_type: :json do
         end
 
         it 'should resolve simple macros' do
-          expect(parsed_response['description']).to have_text('Table of Contents')
+          expect(parsed_response['description']).to have_text('Table of contents')
         end
       end