Strip raw HTML from article titles (#1337)

* Add remove_wrapper_paragraph_tag arg to MarkdownHelper#render_markdown

* Blank space

* Move some page title related helpers to MetaHelper from ApplicationController

* Start PageTitle PORO

* Add test case: PageTitle.with text:

* Add test case: PageTitle.with path: arg

* Un-implement #page_title helper, just passes along @title

* Strip HTML from PageTitle#content

* Require rails/html/sanitizer in PageTitle

* Use PageTitle#content in #page_title

* Add spec for passing an Array to PageTitle.new

* Add fallback rubric to #page_title

* Make text a positional arg for easier normal case calling

* Update categories titles

* Update ArticleArchive titles

* Update Books titles

* Podcast / Episode titles

* Logos titles

* Pages titles

* Search titles

* Support titles

* Tag titles

* Tool titles

* Video titles

* Sign in title

* Add issue number to Issue#show

* Add PageTitle to #admin_title

* Reset render_markdown

* Delete #title_prefix

* Reset app/views/articles/_related.html.erb

* Add /contact and /about titles

.rubocop.yml

@@ -15,7 +15,7 @@ Layout/AlignHash:
   EnforcedColonStyle: table
 
 Metrics/AbcSize:
-  Max: 63
+  Max: 64
 
 Metrics/BlockLength:
   Exclude:

app/controllers/admin/admin_controller.rb

@@ -5,15 +5,15 @@ class AdminController < ApplicationController
     layout 'admin'
 
     def admin_title model = nil, keys = []
-      return t(".#{action_name}_title", default: '') if model.blank?
+      return PageTitle.new(['Admin', t(".#{action_name}_title", default: '')]).content if model.blank?
 
       translation_vars = {}
 
       keys.each_with_object(translation_vars) do |key, hash|
         hash[key] = model.send(key)
       end
 
-      t(".#{action_name}_title", translation_vars)
+      PageTitle.new(['Admin', t(".#{action_name}_title", translation_vars)]).content
     rescue NoMethodError
       logger.error "#{controller_path}:#{action_name} has an issue with the page title"
       ''

app/controllers/admin/locales_controller.rb

@@ -17,7 +17,7 @@ def new
     end
 
     def edit
-      @title = admin_title(@locale, %i[id source_path])
+      @title = admin_title(@locale, %i[id name abbreviation name_in_english])
     end
 
     def create

app/controllers/application_controller.rb

@@ -80,16 +80,6 @@ def current_resource_name
   end
   helper_method :current_resource_name
 
-  # TODO: move to meta helper
-  def page_title
-    if @title.present?
-      t(:site_name) + prepend_admin_if_needed + @title
-    else
-      t(:site_name)
-    end
-  end
-  helper_method :page_title
-
   def title_for *page_keys
     pieces = []
 
@@ -106,16 +96,6 @@ def title_for *page_keys
     pieces.flatten.join ' : '
   end
 
-  # TODO: move to meta helper
-  def prepend_admin_if_needed
-    if controller_path.match(%r{\Aadmin\/.*\z}).present?
-      " #{t('admin.title_prepend')} : "
-    else
-      ' : '
-    end
-  end
-  helper_method :prepend_admin_if_needed
-
   # TODO: move to a helper
   def author
     t(:site_author)

app/controllers/article_archives_controller.rb

@@ -1,8 +1,8 @@
 class ArticleArchivesController < ApplicationController
   def index
-    @html_id    = 'page'
-    @body_id    = 'article-archives'
-    @page_title = 'Articles'
+    @html_id = 'page'
+    @body_id = 'article-archives'
+    @title   = PageTitle.new 'Articles'
 
     @article_archive = ArticleArchive.new(year:  params[:year],
                                           month: params[:month],

app/controllers/articles_controller.rb

@@ -36,7 +36,9 @@ def show
     # redirect to proper URL, chomping /feed off of the end
     return redirect_to @article.path if request.path.ends_with? '/feed'
 
-    @title = @article.name
+    # Page title
+    @title = PageTitle.new @article.name
+
     @live_blog = @article.collection_root?
 
     @previous_article = Article.previous(@article).first

app/controllers/auth/sessions_controller.rb

@@ -6,7 +6,7 @@ class SessionsController < Admin::AdminController
 
     # /signin
     def new
-      @page_title = 'Sign In'
+      @title   = PageTitle.new 'Sign In'
       @body_id = 'signin'
       # TODO: why doesn't this prevent a signed in user going to /signin
       return redirect_to(root_url) if signed_in?

app/controllers/books_controller.rb

@@ -22,7 +22,7 @@ def index
     @html_id = 'page'
     @body_id = 'tools'
     @type    = 'books'
-    @title   = title_for :books
+    @title   = PageTitle.new title_for(:books)
 
     @bullet_books = BOOK_SLUGS.map { |slug| Book.find_by(slug: slug) }
   end
@@ -32,28 +32,28 @@ def show
     @body_id  = 'tools'
     @type     = 'books'
     @editable = @book
-    @title    = title_for :books, @book.slug.underscore
+    @title    = PageTitle.new title_for(:books, @book.slug.underscore)
     @tool     = @book
   end
 
   def extras
     @html_id = 'page'
     @body_id = 'tools'
-    @title   = title_for :books, @book.slug.underscore, :extras
+    @title   = PageTitle.new title_for(:books, @book.slug.underscore, :extras)
 
     render "#{Theme.name}/books/extras"
   end
 
   def lit_kit
     @html_id = 'page'
     @body_id = 'tools'
-    @title   = title_for :books, :lit_kit
+    @title   = PageTitle.new title_for(:books, :lit_kit)
   end
 
   def into_libraries
     @html_id = 'page'
     @body_id = 'tools'
-    @title   = title_for :books, :into_libraries
+    @title   = PageTitle.new title_for(:books, :into_libraries)
   end
 
   private

app/controllers/categories_controller.rb

@@ -7,14 +7,13 @@ class CategoriesController < ApplicationController
   def show
     @html_id = 'page'
     @body_id = 'category'
-    @title   = title_for :categories, @category.name
   end
 
   def index
     @html_id    = 'page'
     @body_id    = 'categories'
     @categories = Category.all
-    @title      = title_for :categories
+    @title      = PageTitle.new title_for(:categories)
   end
 
   def feed
@@ -37,7 +36,7 @@ def set_category
   end
 
   def set_title
-    @title = @category.name
+    @title = PageTitle.new title_for(:categories, @category.name)
   end
 
   def set_articles

app/controllers/episodes_controller.rb

@@ -6,14 +6,14 @@ def show
     @html_id  = 'page'
     @body_id  = 'podcast'
     @editable = @episode
-    @title    = title_for :podcasts, @episode.name, :transcript
+    @title    = PageTitle.new title_for :podcasts, @episode.name, :transcript
   end
 
   def transcript
     @html_id  = 'page'
     @body_id  = 'podcast'
     @editable = @episode
-    @title    = title_for :podcasts, @episode.name, :transcript
+    @title    = PageTitle.new title_for :podcasts, @episode.name, :transcript
 
     render 'episodes/show'
   end

app/controllers/issues_controller.rb

@@ -5,6 +5,8 @@ def show
     @issue    = Issue.find_by(issue: params[:issue_number], journal_id: journal.id)
     @tool     = @issue
     @editable = @issue
+
+    @title = PageTitle.new title_for :journals, journal.name, @issue.issue
     render 'books/show'
   end
 end

app/controllers/logos_controller.rb

@@ -3,7 +3,7 @@ def index
     @html_id = 'page'
     @body_id = 'tools'
     @tools   = Logo.live.published.page(params[:page]).per(100)
-    @title   = title_for :logos
+    @title   = PageTitle.new title_for :logos
   end
 
   def show
@@ -14,6 +14,6 @@ def show
     @body_id  = 'tools'
     @type     = 'logos'
     @editable = @tool
-    @title    = title_for :zines, @tool.name
+    @title    = PageTitle.new title_for :logos, @tool.name
   end
 end

app/controllers/pages_controller.rb

@@ -5,23 +5,24 @@ class PagesController < ApplicationController
 
   def show
     @editable = @page
+    @title    = PageTitle.new I18n.t("page_titles.about.#{@page.slug}")
 
     # no layout
     render html: @page.content.html_safe, layout: false if @page.content_in_html?
   end
 
   def library
-    @title = I18n.t('page_titles.about.library')
+    @title = PageTitle.new I18n.t('page_titles.about.library')
   end
 
   def post_order_success
-    @title    = title_for I18n.t('page_titles.about.store'), I18n.t('page_titles.about.post_order_success')
+    @title    = PageTitle.new title_for I18n.t('page_titles.about.store'), I18n.t('page_titles.about.post_order_success')
     @order_id = params[:ordernum]
   end
 
   # TODO: make this view localizable
   def submission_guidelines
-    @title = I18n.t('page_titles.about.submission_guidelines')
+    @title = PageTitle.new I18n.t('page_titles.about.submission_guidelines')
   end
 
   private

app/controllers/podcast_controller.rb

@@ -4,14 +4,14 @@ def index
     @body_id  = 'podcast'
     @podcasts = Podcast.all.sort_by { |podcast| podcast.latest_episode.published_at }.reverse
     @editable = @podcasts.first
-    @title    = title_for :podcasts
+    @title    = PageTitle.new title_for :podcasts
   end
 
   def show
     @html_id = 'page'
     @body_id = 'podcast'
     @podcast = Podcast.find_by(slug: params[:slug])
-    @title   = title_for @podcast.name
+    @title   = PageTitle.new title_for @podcast.name
   end
 
   def feed; end

app/controllers/search_controller.rb

@@ -12,13 +12,13 @@ def index
       @results = @search.perform.page(params[:page]).per(15)
     end
 
-    @title = title_for :search, :results, "“#{@query}”"
+    @title = PageTitle.new title_for :search, :results, "“#{@query}”"
   end
 
   def advanced
     @html_id = 'page'
     @body_id = 'search'
-    @title   = title_for :search, :advanced
+    @title   = PageTitle.new title_for :search, :advanced
 
     @advanced_search = AdvancedSearch.new(params[:q])
   end

app/controllers/support_controller.rb

@@ -4,7 +4,7 @@ class SupportController < ApplicationController
   def new
     @html_id = 'page'
     @body_id = 'support'
-    @title   = t('views.support.new.heading')
+    @title   = PageTitle.new t('views.support.new.heading')
   end
 
   def create
@@ -24,7 +24,7 @@ def create
   def thanks
     @html_id = 'page'
     @body_id = 'support'
-    @title   = t('views.support.thanks.heading')
+    @title   = PageTitle.new t('views.support.thanks.heading')
   end
 
   def create_session
@@ -55,7 +55,7 @@ def create_session
   def edit
     @html_id = 'page'
     @body_id = 'support-edit'
-    @title   = t('views.support.edit.heading')
+    @title   = PageTitle.new t('views.support.edit.heading')
 
     @support_session = SupportSession.find_by token: params[:token]
 

app/controllers/tags_controller.rb

@@ -22,7 +22,7 @@ def set_tag
   end
 
   def set_title
-    @title = @tag.name
+    @title = PageTitle.new @tag.name
   end
 
   def set_articles

app/controllers/tools_controller.rb

@@ -66,7 +66,7 @@ def set_tool
   end
 
   def set_title
-    @title = title_for [@type.capitalize.to_sym, @tool&.name].compact
+    @title = PageTitle.new title_for [@type.capitalize.to_sym, @tool&.name].compact
   end
 
   def set_editable

app/controllers/videos_controller.rb

@@ -2,7 +2,7 @@ class VideosController < ApplicationController
   def index
     @html_id = 'page'
     @body_id = 'watch'
-    @title   = title_for :videos
+    @title   = PageTitle.new title_for :videos
     @videos  = Video.live.published.page(params[:page]).per(20)
   end
 
@@ -13,6 +13,6 @@ def show
     @editable = @video
     @html_id  = 'page'
     @body_id  = 'video'
-    @title    = title_for :videos, @video.title
+    @title    = PageTitle.new title_for :videos, @video.title
   end
 end

app/helpers/meta_helper.rb

@@ -23,4 +23,14 @@ def meta_image thing
       t('head.meta_image_url')
     end
   end
+
+  def page_title
+    if @title.nil?
+      PageTitle.new.content
+    elsif @title.is_a? PageTitle
+      @title.content
+    else
+      @title
+    end
+  end
 end

app/lib/page_title.rb

@@ -0,0 +1,25 @@
+require 'rails/html/sanitizer'
+
+class PageTitle
+  attr_reader :text, :path
+
+  def initialize text = nil, path: nil
+    @text = text
+    @path = path
+  end
+
+  def content
+    prefix = I18n.t :site_name
+    suffix = text || path_pieces(path)
+
+    title = [prefix, suffix].flatten.compact.join ' : '
+
+    Rails::Html::FullSanitizer.new.sanitize title
+  end
+
+  private
+
+  def path_pieces path
+    String(path).split('/').map(&:capitalize)
+  end
+end

app/views/article_archives/index.html.erb

@@ -1,5 +1,5 @@
 <% content_for(:head) do %>
-  <%= rel_next_prev_link_tags(@article_archive.articles) %>
+  <%= rel_next_prev_link_tags @article_archive.articles %>
 <% end %>
 
 <header>

app/views/articles/_related.html.erb

@@ -20,6 +20,7 @@
 
             <header class="article-titles">
               <h2 class="p-x-title"><%= link_to article.title, article.path %></h2>
+
               <% if article.subtitle.present? %>
                 <h3 class="p-x-subtitle"><%= link_to article.subtitle, article.path %></h3>
               <% end %>

app/views/articles/index.atom.erb

@@ -32,7 +32,7 @@
 
         <link rel="alternate" type="text/html" href="<%= strip_subdomain(request.base_url) + article.path %>" />
 
-        <title><%= article.name %></title>
+        <title><%= strip_tags article.name %></title>
         <summary><%= article.summary %></summary>
 
         <% article.categories.each do |category| %>