Merge pull request #13703 from opf/fix/target_top_renderer

Default to target=_top for all links rendered in CKEditor
opf · Sep 21, 2023 · 2fda271 · 2fda271
2 parents 59f72c1 + fd7b78c
commit 2fda271
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 76 deletions.
diff --git a/lib/open_project/text_formatting/filters/link_attribute_filter.rb b/lib/open_project/text_formatting/filters/link_attribute_filter.rb
@@ -31,7 +31,7 @@ module Filters
     class LinkAttributeFilter < HTML::Pipeline::Filter
       def call
         links.each do |node|
-          node['target'] = context[:target] if context[:target].present?
+          node['target'] = context.fetch(:target, '_top')
         end
 
         doc

diff --git a/modules/documents/spec/application_helper_spec.rb b/modules/documents/spec/application_helper_spec.rb
@@ -65,31 +65,32 @@
       let(:document_link) do
         link_to('Test document',
                 { controller: 'documents', action: 'show', id: document.id },
-                class: 'document op-uc-link')
+                class: 'document op-uc-link',
+                target: '_top')
       end
 
       context "Plain link" do
         subject { format_text("document##{document.id}") }
 
-        it { is_expected.to eq("<p class=\"op-uc-p\">#{document_link}</p>") }
+        it { is_expected.to be_html_eql("<p class=\"op-uc-p\">#{document_link}</p>") }
       end
 
       context "Link with document name" do
         subject { format_text("document##{document.id}") }
 
-        it { is_expected.to eq("<p class=\"op-uc-p\">#{document_link}</p>") }
+        it { is_expected.to be_html_eql("<p class=\"op-uc-p\">#{document_link}</p>") }
       end
 
       context "Escaping plain link" do
         subject { format_text("!document##{document.id}") }
 
-        it { is_expected.to eq("<p class=\"op-uc-p\">document##{document.id}</p>") }
+        it { is_expected.to be_html_eql("<p class=\"op-uc-p\">document##{document.id}</p>") }
       end
 
       context "Escaping link with document name" do
         subject { format_text('!document:"Test document"') }
 
-        it { is_expected.to eq('<p class="op-uc-p">document:"Test document"</p>') }
+        it { is_expected.to be_html_eql('<p class="op-uc-p">document:"Test document"</p>') }
       end
     end
 
@@ -99,29 +100,29 @@
       context "By name without project" do
         subject { format_text("document:\"#{document.title}\"", project: the_other_project) }
 
-        it { is_expected.to eq('<p class="op-uc-p">document:"Test document"</p>') }
+        it { is_expected.to be_html_eql('<p class="op-uc-p">document:"Test document"</p>') }
       end
 
       context "By id and given project" do
         subject { format_text("#{identifier}:document##{document.id}", project: the_other_project) }
 
         it {
-          expect(subject).to eq("<p class=\"op-uc-p\"><a class=\"document op-uc-link\" href=\"/documents/#{document.id}\">Test document</a></p>")
+          expect(subject).to be_html_eql("<p class=\"op-uc-p\"><a class=\"document op-uc-link\" href=\"/documents/#{document.id}\" target=\"_top\">Test document</a></p>")
         }
       end
 
       context "By name and given project" do
         subject { format_text("#{identifier}:document:\"#{document.title}\"", project: the_other_project) }
 
         it {
-          expect(subject).to eq("<p class=\"op-uc-p\"><a class=\"document op-uc-link\" href=\"/documents/#{document.id}\">Test document</a></p>")
+          expect(subject).to be_html_eql("<p class=\"op-uc-p\"><a class=\"document op-uc-link\" href=\"/documents/#{document.id}\" target=\"_top\">Test document</a></p>")
         }
       end
 
       context "Invalid link" do
         subject { format_text("invalid:document:\"Test document\"", project: the_other_project) }
 
-        it { is_expected.to eq('<p class="op-uc-p">invalid:document:"Test document"</p>') }
+        it { is_expected.to be_html_eql('<p class="op-uc-p">invalid:document:"Test document"</p>') }
       end
     end
   end

diff --git a/modules/documents/spec/lib/open_project/markdown_formatting_spec.rb b/modules/documents/spec/lib/open_project/markdown_formatting_spec.rb
@@ -78,7 +78,8 @@ def controller
       link_to(
         'My document',
         { controller: '/documents', action: 'show', id: document.id, only_path: true },
-        class: 'document op-uc-link'
+        class: 'document op-uc-link',
+        target: '_top'
       )
     end
 

diff --git a/modules/meeting/spec/lib/open_project/markdown_formatting_spec.rb b/modules/meeting/spec/lib/open_project/markdown_formatting_spec.rb
@@ -82,7 +82,8 @@ def controller
       link_to(
         'Monthly coordination',
         { controller: '/meetings', action: 'show', id: meeting.id, only_path: true },
-        class: 'meeting op-uc-link'
+        class: 'meeting op-uc-link',
+        target: '_top'
       )
     end
 

diff --git a/spec/features/forums/attachment_upload_spec.rb b/spec/features/forums/attachment_upload_spec.rb
@@ -111,7 +111,6 @@
     click_button 'Create'
 
     attachments_list.expect_attached('image.png')
-
     within '.toolbar-items' do
       click_on "Edit"
     end

diff --git a/spec/lib/open_project/text_formatting/markdown/in_tool_links_spec.rb b/spec/lib/open_project/text_formatting/markdown/in_tool_links_spec.rb
diff --git a/spec/lib/open_project/text_formatting/markdown/lists_spec.rb b/spec/lib/open_project/text_formatting/markdown/lists_spec.rb
@@ -135,7 +135,7 @@
                   <td>
                     <ul class="op-uc-list_task-list op-uc-list">
                       <li>
-                        <a class="op-uc-link" href="https://example.com/">
+                        <a class="op-uc-link" target="_top" href="https://example.com/">
                           <label class="op-uc-list__label">
                             <input type="checkbox" disabled="disabled">
                             <span class="op-uc-list__label__description">asdfasd</span>
@@ -217,7 +217,7 @@
                         <ul class="op-uc-list_task-list op-uc-list">
                           <li class="op-uc-list--item">
                             <input type="checkbox" class="op-uc-list--task-checkbox" disabled>
-                            <a class="op-uc-link" href="https://example.com/" rel="noopener noreferrer">
+                            <a class="op-uc-link" target="_top" href="https://example.com/" rel="noopener noreferrer">
                               <span>asdfasd</span>
                               <span> asdf</span>
                             </a>
@@ -316,7 +316,7 @@
                           <li class="op-uc-list--item">
                             <input type="checkbox" class="op-uc-list--task-checkbox" disabled>
                             <span>asdfasdfasdf </span>
-                            <a class="op-uc-link" href="https://example.com/" rel="noopener noreferrer">
+                            <a class="op-uc-link" target="_top" href="https://example.com/" rel="noopener noreferrer">
                               <span>foobar</span>
                             </a>
                           </li>

diff --git a/spec/lib/open_project/text_formatting/markdown/mentions_spec.rb b/spec/lib/open_project/text_formatting/markdown/mentions_spec.rb
@@ -87,7 +87,8 @@
                   #{link_to(linked_project_member.name,
                             { controller: :users, action: :show, id: linked_project_member.id },
                             title: "User #{linked_project_member.name}",
-                            class: 'user-mention op-uc-link')}
+                            class: 'user-mention op-uc-link',
+                            target: '_top')}
                 </p>
               EXPECTED
             end
@@ -133,7 +134,8 @@
                   #{link_to(linked_project_member.name,
                             { controller: :users, action: :show, id: linked_project_member.id },
                             title: "User #{linked_project_member.name}",
-                            class: 'user-mention op-uc-link')}
+                            class: 'user-mention op-uc-link',
+                            target: '_top')}
                 </p>
               EXPECTED
             end
@@ -156,7 +158,8 @@
                   #{link_to(linked_project_member.name,
                             { controller: :users, action: :show, id: linked_project_member.id },
                             title: "User #{linked_project_member.name}",
-                            class: 'user-mention op-uc-link')}
+                            class: 'user-mention op-uc-link',
+                            target: '_top')}
                 </p>
               EXPECTED
             end
@@ -180,7 +183,8 @@
                     #{link_to(linked_project_member.name,
                               { controller: :users, action: :show, id: linked_project_member.id },
                               title: "User #{linked_project_member.name}",
-                              class: 'user-mention op-uc-link')}
+                              class: 'user-mention op-uc-link',
+                              target: '_top')}
                   </p>
                 EXPECTED
               end
@@ -208,7 +212,8 @@
                     #{link_to(linked_project_member.name,
                               { controller: :users, action: :show, id: linked_project_member.id },
                               title: "User #{linked_project_member.name}",
-                              class: 'user-mention op-uc-link')}
+                              class: 'user-mention op-uc-link',
+                              target: '_top')}
                   </p>
                 EXPECTED
               end
@@ -232,7 +237,8 @@
                   #{link_to(linked_project_member.name,
                             { controller: :users, action: :show, id: linked_project_member.id },
                             title: "User #{linked_project_member.name}",
-                            class: 'user-mention op-uc-link')}
+                            class: 'user-mention op-uc-link',
+                            target: '_top')}
                 </p>
               EXPECTED
             end
@@ -292,7 +298,7 @@
               let(:expected) do
                 <<~EXPECTED
                   <p class="op-uc-p">
-                    Link to <a class="user-mention op-uc-link" href="/users/#{user.id}" title="User Foo Barrit">Foo Barrit</a>
+                    Link to <a class="user-mention op-uc-link" target="_top" href="/users/#{user.id}" title="User Foo Barrit">Foo Barrit</a>
                   </p>
                 EXPECTED
               end
@@ -312,7 +318,7 @@
               let(:expected) do
                 <<~EXPECTED
                   <p class="op-uc-p">
-                    Link to <a class="user-mention op-uc-link" href="http://openproject.org/users/#{user.id}" title="User Foo Barrit">Foo Barrit</a>
+                    Link to <a class="user-mention op-uc-link" target="_top" href="http://openproject.org/users/#{user.id}" title="User Foo Barrit">Foo Barrit</a>
                   </p>
                 EXPECTED
               end
@@ -351,6 +357,7 @@
                 <p class="op-uc-p">
                   Link to
                   <a class="user-mention op-uc-link"
+                     target="_top"
                      href="/groups/#{linked_project_member_group.id}"
                      title="Group #{linked_project_member_group.name}">
                     #{linked_project_member_group.name}
@@ -396,6 +403,7 @@
               <<~EXPECTED
                 <p class="op-uc-p">
                   <a class="user-mention op-uc-link"
+                     target="_top"
                      href="/groups/#{linked_project_member_group.id}"
                      title="Group #{linked_project_member_group.name}">
                     #{linked_project_member_group.name}

diff --git a/spec/lib/open_project/text_formatting/markdown/setting_variable_spec.rb b/spec/lib/open_project/text_formatting/markdown/setting_variable_spec.rb
@@ -61,10 +61,12 @@
           </p>
           <p class="op-uc-p">
             <a href="#{OpenProject::Application.root_url}/foo/bar" rel="noopener noreferrer"
+               target="_top"
                class="op-uc-link">Link with setting</a>
           </p>
           <p class="op-uc-p">
             <a href="#{OpenProject::Application.root_url}/foo/bar" rel="noopener noreferrer"
+               target="_top"
                class="op-uc-link">Saved and transformed link with setting</a>
           </p>
           <p class="op-uc-p">

diff --git a/spec/lib/open_project/text_formatting/markdown/user_provided_links_spec.rb b/spec/lib/open_project/text_formatting/markdown/user_provided_links_spec.rb
@@ -37,14 +37,14 @@
     it_behaves_like 'format_text produces' do
       let(:raw) do
         <<~RAW
-          this is a <a style="display:none;" href="http://malicious">
+          this is a <a style="display:none;" target="_top" href="http://malicious">
         RAW
       end
 
       let(:expected) do
         <<~EXPECTED
           <p class="op-uc-p">
-            this is a <a href="http://malicious" rel="noopener noreferrer" class="op-uc-link">
+            this is a <a href="http://malicious" target="_top" rel="noopener noreferrer" class="op-uc-link">
           </p>
         EXPECTED
       end
@@ -101,7 +101,7 @@
         let(:expected) do
           <<~EXPECTED
             <p class="op-uc-p">
-              Link to <a href="/foo/bar" class="op-uc-link" rel="noopener noreferrer">relative path</a>
+              Link to <a href="/foo/bar" target="_top" class="op-uc-link" rel="noopener noreferrer">relative path</a>
             </p>
           EXPECTED
         end
@@ -121,7 +121,7 @@
         let(:expected) do
           <<~EXPECTED
             <p class="op-uc-p">
-              Link to <a href="http://openproject.org/foo/bar" class="op-uc-link" rel="noopener noreferrer">relative path</a>
+              Link to <a href="http://openproject.org/foo/bar" target="_top" class="op-uc-link" rel="noopener noreferrer">relative path</a>
             </p>
           EXPECTED
         end

diff --git a/spec/lib/open_project/text_formatting/markdown/work_package_buttons_macro_spec.rb b/spec/lib/open_project/text_formatting/markdown/work_package_buttons_macro_spec.rb
@@ -74,7 +74,7 @@ def error_html(exception_msg)
       let(:expected) do
         <<~EXPECTED
           <p class="op-uc-p">
-            <a class="op-uc-link" href="/projects/my-project/work_packages/new">New work package</a>
+            <a class="op-uc-link" target="_top" href="/projects/my-project/work_packages/new">New work package</a>
           </p>
         EXPECTED
       end
@@ -106,7 +106,7 @@ def error_html(exception_msg)
       let(:expected) do
         <<~EXPECTED
           <p class="op-uc-p">
-            <a class="op-uc-link" href="/projects/my-project/work_packages/new?type=#{type.id}">New MyTaskName</a>
+            <a class="op-uc-link" target="_top" href="/projects/my-project/work_packages/new?type=#{type.id}">New MyTaskName</a>
           </p>
         EXPECTED
       end
@@ -123,7 +123,7 @@ def error_html(exception_msg)
         let(:expected) do
           <<~EXPECTED
             <p class="op-uc-p">
-              <a class="button op-uc-link" href="/projects/my-project/work_packages/new?type=#{type.id}">New MyTaskName</a>
+              <a class="button op-uc-link" target="_top" href="/projects/my-project/work_packages/new?type=#{type.id}">New MyTaskName</a>
             </p>
           EXPECTED
         end

diff --git a/spec/requests/api/v3/render_resource_spec.rb b/spec/requests/api/v3/render_resource_spec.rb
@@ -69,9 +69,15 @@
 
             it_behaves_like 'valid response' do
               let(:text) do
-                '<p class="op-uc-p">Hello World! This <em>is</em> markdown with a ' +
-                  '<a href="http://community.openproject.org" rel="noopener noreferrer" class="op-uc-link">link</a> ' +
-                  'and ümläutß.</p>'
+                <<~HTML
+                  <p class="op-uc-p">
+                    Hello World! This <em>is</em> markdown with a
+                    <a target="_top"
+                       href="http://community.openproject.org"
+                       rel="noopener noreferrer"
+                       class="op-uc-link">link</a>
+                    and ümläutß.</p>
+                HTML
               end
             end
           end
@@ -81,9 +87,14 @@
             let(:id) { work_package.id }
             let(:href) { "/work_packages/#{id}" }
             let(:text) do
-              '<p class="op-uc-p">Hello World! Have a look at <a ' \
-                "class=\"issue work_package preview-trigger op-uc-link\" " \
-                "href=\"#{href}\">##{id}</a></p>"
+              <<~HTML
+                <p class="op-uc-p">
+                  Hello World! Have a look at
+                  <a class="issue work_package preview-trigger op-uc-link"
+                     target="_top"
+                     href="#{href}">##{id}</a>
+                </p>
+              HTML
             end
 
             context 'with work package context' do