Consolidate html page title output logic into helper (#27563)

.haml-lint_todo.yml

@@ -31,4 +31,3 @@ linters:
       - 'app/views/admin/accounts/_buttons.html.haml'
       - 'app/views/admin/accounts/_local_account.html.haml'
       - 'app/views/admin/roles/_form.html.haml'
-      - 'app/views/layouts/application.html.haml'

app/helpers/application_helper.rb

@@ -91,6 +91,14 @@ module ApplicationHelper
     end
   end
 
+  def html_title
+    safe_join(
+      [content_for(:page_title).to_s.chomp, title]
+      .select(&:present?),
+      ' - '
+    )
+  end
+
   def title
     Rails.env.production? ? site_title : "#{site_title} (Dev)"
   end

app/views/layouts/application.html.haml

@@ -24,7 +24,7 @@
     %meta{ name: 'theme-color', content: '#191b22' }/
     %meta{ name: 'apple-mobile-web-app-capable', content: 'yes' }/
 
-    %title= content_for?(:page_title) ? safe_join([yield(:page_title).chomp.html_safe, title], ' - ') : title
+    %title= html_title
 
     = stylesheet_pack_tag 'common', media: 'all', crossorigin: 'anonymous'
     = stylesheet_pack_tag current_theme, media: 'all', crossorigin: 'anonymous'

spec/helpers/application_helper_spec.rb

@@ -296,5 +296,51 @@ describe ApplicationHelper do
       expect(helper.title).to eq 'site title'
       expect(Rails.env).to have_received(:production?)
     end
+
+    it 'returns site title with note on non-production environment' do
+      Setting.site_title = 'site title'
+      allow(Rails.env).to receive(:production?).and_return(false)
+      expect(helper.title).to eq 'site title (Dev)'
+      expect(Rails.env).to have_received(:production?)
+    end
+  end
+
+  describe 'html_title' do
+    before do
+      allow(Rails.env).to receive(:production?).and_return(true)
+    end
+
+    around do |example|
+      site_title = Setting.site_title
+      example.run
+      Setting.site_title = site_title
+    end
+
+    context 'with a page_title content_for value' do
+      it 'uses the value in the html title' do
+        Setting.site_title = 'Site Title'
+        helper.content_for(:page_title, 'Test Value')
+
+        expect(helper.html_title).to eq 'Test Value - Site Title'
+        expect(helper.html_title).to be_html_safe
+      end
+
+      it 'removes extra new lines' do
+        Setting.site_title = 'Site Title'
+        helper.content_for(:page_title, "Test Value\n")
+
+        expect(helper.html_title).to eq 'Test Value - Site Title'
+        expect(helper.html_title).to be_html_safe
+      end
+    end
+
+    context 'without any page_title content_for value' do
+      it 'returns the site title' do
+        Setting.site_title = 'Site Title'
+
+        expect(helper.html_title).to eq 'Site Title'
+        expect(helper.html_title).to be_html_safe
+      end
+    end
   end
 end