Merge pull request #15243 from opf/fix/52160/relative-links

Set target=_top for all links, also external
opf · Apr 16, 2024 · 3292f2f · 3292f2f
2 parents 6bb7703 + 36d7649
commit 3292f2f
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 7 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,14 +31,16 @@ module Filters
     class LinkAttributeFilter < HTML::Pipeline::Filter
       def call
         links.each do |node|
+          next if node["target"] || node["href"]&.start_with?("#")
+
           node["target"] = context.fetch(:target, "_top")
         end
 
         doc
       end
 
       def links
-        doc.css('a[href^="/"]')
+        doc.css('a')
       end
     end
   end

diff --git a/modules/meeting/spec/features/structured_meetings/turbo_links_spec.rb b/modules/meeting/spec/features/structured_meetings/turbo_links_spec.rb
@@ -0,0 +1,71 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 the OpenProject GmbH
+#
+# 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-2013 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 COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+
+require_relative "../../support/pages/meetings/new"
+require_relative "../../support/pages/structured_meeting/show"
+
+RSpec.describe "Structured meetings links caught by turbo",
+               :js,
+               :with_cuprite do
+  include Rails.application.routes.url_helpers
+
+  shared_let(:project) { create(:project, enabled_module_names: %w[meetings]) }
+  shared_let(:user) do
+    create(:user,
+           lastname: "First",
+           member_with_permissions: { project => %i[view_meetings create_meetings edit_meetings delete_meetings manage_agendas
+                                                    view_work_packages] }).tap do |u|
+      u.pref[:time_zone] = "utc"
+
+      u.save!
+    end
+  end
+  shared_let(:meeting1) { create(:structured_meeting, title: "First meeting", project:) }
+  shared_let(:meeting2) { create(:structured_meeting, title: "Other meeting", project:) }
+
+  let(:notes) do
+    <<~NOTES
+      [Meeting link](#{meeting_url(meeting2)})
+    NOTES
+  end
+  let!(:agenda_item) { create(:meeting_agenda_item, meeting: meeting1, notes:) }
+  let(:show_page) { Pages::StructuredMeeting::Show.new(meeting1) }
+
+  before do
+    login_as user
+    show_page.visit!
+  end
+
+  it "can link to the other meeting" do
+    click_link_or_button "Meeting link"
+    expect(page).to have_current_path meeting_path(meeting2)
+    expect(page).to have_css("#content", text: "Other meeting", visible: :visible)
+  end
+end
diff --git a/spec/lib/open_project/text_formatting/markdown/lists_spec.rb b/spec/lib/open_project/text_formatting/markdown/lists_spec.rb
@@ -285,7 +285,7 @@
                           <input type="checkbox" disabled="disabled">
                           <span class="op-uc-list__label__description">asdfasdfasdf </span>
                         </label>
-                        <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">
                             <span class="op-uc-list__label__description">foobar</span>
                           </label>
@@ -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" href="https://example.com/" target="_top" 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
@@ -313,7 +313,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

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
@@ -60,11 +60,11 @@
             Inline reference to base_url variable: #{Rails.application.root_url}
           </p>
           <p class="op-uc-p">
-            <a href="#{Rails.application.root_url}/foo/bar" rel="noopener noreferrer"
+            <a href="#{Rails.application.root_url}/foo/bar" target="_top" rel="noopener noreferrer"
                class="op-uc-link">Link with setting</a>
           </p>
           <p class="op-uc-p">
-            <a href="#{Rails.application.root_url}/foo/bar" rel="noopener noreferrer"
+            <a href="#{Rails.application.root_url}/foo/bar" target="_top" rel="noopener noreferrer"
                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
@@ -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/requests/api/v3/render_resource_spec.rb b/spec/requests/api/v3/render_resource_spec.rb
@@ -73,6 +73,7 @@
                   <p class="op-uc-p">
                     Hello World! This <em>is</em> markdown with a
                     <a href="http://community.openproject.org"
+                       target="_top"
                        rel="noopener noreferrer"
                        class="op-uc-link">link</a>
                     and ümläutß.</p>