From 8e0b3513671aafaf29bfed6123e5a80c2ebdebdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 23 Feb 2026 07:13:50 +0100 Subject: [PATCH] Pass enabled value to external links controller so it can be properly disabled https://community.openproject.org/projects/openproject/work_packages/71946/activity --- app/helpers/application_helper.rb | 1 + .../controllers/external-links.controller.ts | 57 +++++++++++-------- spec/views/layouts/base.html.erb_spec.rb | 6 +- 3 files changed, 36 insertions(+), 28 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 10d6b126d3a..9a10f1ea181 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -305,6 +305,7 @@ module ApplicationHelper controller: "application auto-theme-switcher hover-card-trigger beforeunload external-links highlight-target-element", relative_url_root: root_path, overflowing_identifier: ".__overflowing_body", + external_links_enabled_value: Setting.capture_external_links?, rendered_at: Time.zone.now.iso8601, turbo: local_assigns[:turbo_opt_out] ? "false" : nil }.merge(user_theme_data_attributes) diff --git a/frontend/src/stimulus/controllers/external-links.controller.ts b/frontend/src/stimulus/controllers/external-links.controller.ts index e65f3737731..a9f3853934c 100644 --- a/frontend/src/stimulus/controllers/external-links.controller.ts +++ b/frontend/src/stimulus/controllers/external-links.controller.ts @@ -70,6 +70,12 @@ const shouldProcessLink = (link:HTMLAnchorElement) => { * dynamically loaded content. */ export default class ExternalLinksController extends ApplicationController { + static values = { + enabled: Boolean + }; + + declare readonly enabledValue:boolean; + connect() { useMutation(this, { attributes: true, childList: true, subtree: true, attributeFilter: ['target', 'href'] }); @@ -77,9 +83,9 @@ export default class ExternalLinksController extends ApplicationController { document.querySelectorAll(LINK_QUERY).forEach((link)=>{ if (!shouldProcessLink(link)) return; - if (isLinkBlank(link)) updateBlankLink(link); + if (isLinkBlank(link)) this.updateBlankLink(link); - if (isLinkExternal(link)) updateExternalLink(link); + if (isLinkExternal(link)) this.updateExternalLink(link); }); } @@ -89,16 +95,16 @@ export default class ExternalLinksController extends ApplicationController { if (isElement(node)) { // Added element itself is an external link if (isLink(node) && shouldProcessLink(node)) { - if (isLinkBlank(node)) updateBlankLink(node); - if (isLinkExternal(node)) updateExternalLink(node); + if (isLinkBlank(node)) this.updateBlankLink(node); + if (isLinkExternal(node)) this.updateExternalLink(node); } node.querySelectorAll(LINK_QUERY).forEach((link)=>{ if (!shouldProcessLink(link)) return; - if (isLinkBlank(link)) updateBlankLink(link); + if (isLinkBlank(link)) this.updateBlankLink(link); - if (isLinkExternal(link)) updateExternalLink(link); + if (isLinkExternal(link)) this.updateExternalLink(link); }); } }); @@ -110,28 +116,29 @@ export default class ExternalLinksController extends ApplicationController { isLink(mutation.target) && shouldProcessLink(mutation.target) ) { - if (mutation.attributeName === 'target' && isLinkBlank(mutation.target)) updateBlankLink(mutation.target); - if (mutation.attributeName === 'href' && isLinkExternal(mutation.target)) updateExternalLink(mutation.target); + if (mutation.attributeName === 'target' && isLinkBlank(mutation.target)) this.updateBlankLink(mutation.target); + if (mutation.attributeName === 'href' && isLinkExternal(mutation.target)) this.updateExternalLink(mutation.target); } }); } -} -function updateBlankLink(link:HTMLAnchorElement) { - // Ensure accessibility description - attributeTokenList(link, 'aria-describedby').add(BLANK_LINK_DESCRIPTION_ID); -} - -function updateExternalLink(link:HTMLAnchorElement) { - // Ensure external link behavior - link.target = '_blank'; - attributeTokenList(link, 'rel').add('noopener', 'noreferrer'); - - // Capture external links through redirect page - // The backend controller will redirect directly if the feature is disabled - if (!link.dataset.allowExternalLink) { - const originalHref = link.href; - const basePath = window.appBasePath ?? ''; - link.href = `${basePath}/external_redirect?url=${encodeURIComponent(originalHref)}`; + private updateBlankLink(link:HTMLAnchorElement) { + // Ensure accessibility description + attributeTokenList(link, 'aria-describedby').add(BLANK_LINK_DESCRIPTION_ID); } + + private updateExternalLink(link:HTMLAnchorElement) { + // Ensure external link behavior + link.target = '_blank'; + attributeTokenList(link, 'rel').add('noopener', 'noreferrer'); + + // Capture external links through redirect page + // The backend controller will redirect directly if the feature is disabled + if (this.enabledValue && !link.dataset.allowExternalLink) { + const originalHref = link.href; + const basePath = window.appBasePath ?? ''; + link.href = `${basePath}/external_redirect?url=${encodeURIComponent(originalHref)}`; + } + } + } diff --git a/spec/views/layouts/base.html.erb_spec.rb b/spec/views/layouts/base.html.erb_spec.rb index b3cd26a4760..3633476b36c 100644 --- a/spec/views/layouts/base.html.erb_spec.rb +++ b/spec/views/layouts/base.html.erb_spec.rb @@ -38,6 +38,7 @@ RSpec.describe "layouts/base" do include Capybara::RSpecMatchers include Redmine::MenuManager::MenuHelper + helper Redmine::MenuManager::MenuHelper let(:user) { build_stubbed(:user) } let(:anonymous) { build_stubbed(:anonymous) } @@ -104,7 +105,7 @@ RSpec.describe "layouts/base" do describe "icons" do let(:current_user) { anonymous } - context "not in development environment" do + context "when not in development environment" do before do render end @@ -189,12 +190,11 @@ RSpec.describe "layouts/base" do let(:a_token) { EnterpriseToken.new } let(:current_user) { anonymous } - context "EE is active and styles are present" do + context "when EE is active and styles are present", with_ee: %i[define_custom_style capture_external_links] do let(:custom_style) { create(:custom_style) } let(:primary_color) { create(:"design_color_primary-button-color") } before do - allow(EnterpriseToken).to receive(:allows_to?).with(:define_custom_style).and_return(true) allow(CustomStyle).to receive(:current).and_return(custom_style) end