From cdf859621e96b1a7a5a8268d2fc4dee5828b436f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sun, 15 Oct 2023 16:04:02 +0200
Subject: [PATCH] Allow links in forms to open in new tabs

We used to open these links in new tabs, but accidentally stopped doing
so in commit 75a28fafc.

While, in general, automatically opening a link in a new tab/window is a
bad idea, the exception comes when people are filling in a form and
there are links to pages that contain information which will help them
fill in a form.

There are mainly two advantages of this approach. First, it makes less
likely for people to accidentally lose the information they were filling
in. And, second, having both the form and a help page open at the same
time can make it easier to fill in the form.

However, opening these links in new tabs also has disadvantages, like
taking control away from people or making it harder to navigate through
pages when using a mobile phone.

So this is a compromise solution.
---
 lib/consul_form_builder.rb                    | 12 +++++++++--
 ...h_terms_of_service_field_component_spec.rb | 21 +++++++++++++++++++
 spec/lib/consul_form_builder_spec.rb          | 11 ++++++++++
 spec/system/verification/residence_spec.rb    |  5 +++--
 4 files changed, 45 insertions(+), 4 deletions(-)
 create mode 100644 spec/components/shared/agree_with_terms_of_service_field_component_spec.rb

diff --git a/lib/consul_form_builder.rb b/lib/consul_form_builder.rb
index e9868185e..620a0886c 100644
--- a/lib/consul_form_builder.rb
+++ b/lib/consul_form_builder.rb
@@ -23,7 +23,7 @@ class ConsulFormBuilder < FoundationRailsHelper::FormBuilder
     if options[:label] == false
       super
     else
-      label = tag.span sanitize(label_text(attribute, options[:label])), class: "checkbox"
+      label = tag.span sanitized_label_text(attribute, options[:label]), class: "checkbox"
 
       super(attribute, options.merge(
         label: label,
@@ -51,7 +51,7 @@ class ConsulFormBuilder < FoundationRailsHelper::FormBuilder
       if text == false
         super
       else
-        super(attribute, sanitize(label_text(attribute, text)), options)
+        super(attribute, sanitized_label_text(attribute, text), options)
       end
     end
 
@@ -68,6 +68,14 @@ class ConsulFormBuilder < FoundationRailsHelper::FormBuilder
       end
     end
 
+    def sanitized_label_text(attribute, text)
+      sanitize(label_text(attribute, text), attributes: allowed_attributes)
+    end
+
+    def allowed_attributes
+      self.class.sanitized_allowed_attributes + ["target"]
+    end
+
     def label_options_for(options)
       label_options = options[:label_options] || {}
 
diff --git a/spec/components/shared/agree_with_terms_of_service_field_component_spec.rb b/spec/components/shared/agree_with_terms_of_service_field_component_spec.rb
new file mode 100644
index 000000000..6b7c8bc7a
--- /dev/null
+++ b/spec/components/shared/agree_with_terms_of_service_field_component_spec.rb
@@ -0,0 +1,21 @@
+require "rails_helper"
+
+describe Shared::AgreeWithTermsOfServiceFieldComponent do
+  before do
+    dummy_model = Class.new do
+      include ActiveModel::Model
+      attr_accessor :terms_of_service
+    end
+
+    stub_const("DummyModel", dummy_model)
+  end
+
+  let(:form) { ConsulFormBuilder.new(:dummy, DummyModel.new, ApplicationController.new.view_context, {}) }
+  let(:component) { Shared::AgreeWithTermsOfServiceFieldComponent.new(form) }
+
+  it "contains links that open in a new window" do
+    render_inline component
+
+    expect(page).to have_css "a[target=_blank]", count: 2
+  end
+end
diff --git a/spec/lib/consul_form_builder_spec.rb b/spec/lib/consul_form_builder_spec.rb
index d8d9adf5f..db57847d2 100644
--- a/spec/lib/consul_form_builder_spec.rb
+++ b/spec/lib/consul_form_builder_spec.rb
@@ -13,6 +13,17 @@ describe ConsulFormBuilder do
 
   let(:builder) { ConsulFormBuilder.new(:dummy, DummyModel.new, ApplicationController.new.view_context, {}) }
 
+  describe "label" do
+    it "accepts links that open in a new window in its content" do
+      render builder.text_field(:title, label: 'My <a href="/" target="_blank" title="New tab">title</a>')
+
+      expect(page).to have_link count: 1
+      expect(page).to have_link "title"
+      expect(page).to have_link "New tab"
+      expect(page).to have_css "a[target=_blank]"
+    end
+  end
+
   describe "hints" do
     it "does not generate hints by default" do
       render builder.text_field(:title)
diff --git a/spec/system/verification/residence_spec.rb b/spec/system/verification/residence_spec.rb
index c54e44fee..3603d4ba9 100644
--- a/spec/system/verification/residence_spec.rb
+++ b/spec/system/verification/residence_spec.rb
@@ -164,8 +164,9 @@ describe "Residence" do
     login_as(create(:user))
 
     visit new_residence_path
-    click_link "the terms and conditions of access"
 
-    expect(page).to have_content "Terms and conditions of access of the Census"
+    within_window(window_opened_by { click_link "the terms and conditions of access" }) do
+      expect(page).to have_content "Terms and conditions of access of the Census"
+    end
   end
 end