Fix malformed-HTML fallout across RSS, link rewriting, and flash messaging

Editors' TinyMCE output can contain unclosed void elements like <br>,
which is valid HTML5 but invalid XML -- three different places assumed
the stricter rule and broke or silently misbehaved on the looser one.
The Atom feed's <content type="xhtml"> block required real, well-formed
XML structure but was handed a raw, unescaped body string; switched to
type="html" with CGI.escapeHTML, matching how title/summary already
handle the same content. rewrite_links_in_body used libxml's strict XML
parser to rewrite internal links to be locale-prefixed, which raised on
exactly this class of malformed markup -- silently, since the whole
method was wrapped in rescue; nil, meaning the link rewrite (not the
save) quietly failed with no error anywhere. Replaced with Nokogiri's
lenient HTML parser, which repairs malformed void elements rather than
rejecting them; also drops the bare rescue now that the actual failure
mode it was guarding against shouldn't occur, and fixes two adjacent
bugs found while in this method: a typo'd /sytem/uploads/ regex that
could never match, and a missing https:// exclusion alongside the
existing http:// one.

Also addresses stale flash messaging surfaced while testing the above:
update's save confirmation was being clobbered by edit's own "locked
and ready" notice on the very next request, since nothing distinguished
a fresh lock acquisition from a redirect back after saving. The save
confirmation now names the next step (publish from Status) and flags a
stale translation if one exists, using Page#outdated_translations?,
already present but previously unused by any controller.

5 files changed, 44 insertions, 27 deletions

diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index 72d4a3e..38d42d9 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -65,15 +65,16 @@ class NodesController < ApplicationController
   end
 
   def edit
+    freshly_locked = @node.lock_owner != current_user
     @node.lock_for_editing!( current_user )
     @page = @node.autosave || @node.draft || @node.head
 
-    flash.now[:notice] = "Node locked and ready to edit" unless @node.autosave
-
     if @node.autosave
       flash.now[:notice] =
         "This page has unsaved changes from a previous session, shown below. " \
         "Save to keep them, or use \"Discard changes\" below to go back to the last saved version."
+    elsif freshly_locked
+      flash.now[:notice] = "Node locked and ready to edit"
     end
   rescue LockedByAnotherUser => e
     flash[:error] = e.message
@@ -85,7 +86,18 @@ class NodesController < ApplicationController
     @node.autosave!( page_params.merge(:tag_list => params[:tag_list]), current_user )
     @node.save_draft!(current_user)
 
-    flash[:notice] = "Draft has been saved: #{Time.now}"
+    flash[:notice] = "Draft saved. Publish your changes in the Status section once you're done."
+    flash[:status_path] = node_path(@node)
+
+    if @node.draft.translated_locales.size > 1
+      stale_locale = @node.draft.translated_locales.find do |locale|
+        @node.draft.outdated_translations?(locale: locale)
+      end
+      if stale_locale
+        flash[:stale_locale] = stale_locale
+        flash[:stale_locale_path] = edit_node_path(@node, locale: stale_locale)
+      end
+    end
 
     if params[:commit] == "Save + Unlock + Exit"
       @node.unlock!
diff --git a/app/models/page.rb b/app/models/page.rb
index fff044e..1a98e08 100644
--- a/app/models/page.rb
+++ b/app/models/page.rb
@@ -243,31 +243,22 @@ class Page < ApplicationRecord
     end
 
     def rewrite_links_in_body
-      begin
-        if self.body
-          tmp_body    = "<div>#{self.body}</div>"
-          xml_string  = XML::Parser.string( tmp_body )
-          xml_doc     = xml_string.parse
-          links       = xml_doc.find("//a[not(starts-with(@href, 'http://'))]")
-          links       = links.reject { |l| l[:href] =~ /system\/uploads/ }
-          locales     = I18n.available_locales.reject {|l| l == :root}
+      return unless self.body
 
-          links.each do |link|
-            unless locales.include? link[:href].slice(1,2).to_sym
-              unless link[:href] =~ /sytem\/uploads/
-                link[:href] = link[:href].sub(/^\//, "/#{I18n.locale}/")
-              end
-            end
-          end
+      doc = Nokogiri::HTML::DocumentFragment.parse(self.body)
+      locales = I18n.available_locales.reject { |l| l == :root }
 
-          tmp_body = xml_doc.to_s.gsub(/(\n\<div\>|\<\/div\>\n)/, "")
-          tmp_body.gsub!("<?xml version=\"1.0\" encoding=\"UTF-8\"?>", "")
+      doc.css('a').each do |link|
+        href = link['href']
+        next unless href
+        next if href.start_with?('http://', 'https://')
+        next if href =~ /system\/uploads/
 
-          self.body = tmp_body
+        unless locales.include?(href.slice(1, 2)&.to_sym)
+          link['href'] = href.sub(/^\//, "/#{I18n.locale}/")
         end
-      rescue
-        nil
       end
-    end
 
+      self.body = doc.to_html
+    end
 end
diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb
index 340eaf2..79b1380 100644
--- a/app/views/layouts/admin.html.erb
+++ b/app/views/layouts/admin.html.erb
@@ -33,6 +33,13 @@
       <% if flash[:notice].present? || flash[:error].present? %>
       <div id="flash">
         <%= flash[:notice] %>
+        <% if flash[:status_path] %>
+          <%= link_to 'Go to Status', flash[:status_path] %>
+        <% end %>
+        <% if flash[:stale_locale_path] %>
+          The <%= flash[:stale_locale] %> translation may be out of date —
+          <%= link_to 'review it', flash[:stale_locale_path] %> too.
+        <% end %>
         <% if flash[:error] %>
           <span id="flash_error"><%= flash[:error] %></span>
         <% end %>
diff --git a/app/views/rss/updates.xml.builder b/app/views/rss/updates.xml.builder
index 4b2e2f7..27845c4 100644
--- a/app/views/rss/updates.xml.builder
+++ b/app/views/rss/updates.xml.builder
@@ -22,9 +22,7 @@ xml.feed(:xmlns => "http://www.w3.org/2005/Atom", "xml:base" => @host) do
       xml.updated(item.updated_at.xmlschema)
       xml.published(item.published_at.xmlschema)
       xml.summary(CGI.escapeHTML(item.abstract.to_s))
-      xml.content(:type => "xhtml") do
-        xml.div(item.body, :xmlns => "http://www.w3.org/1999/xhtml")
-      end
+      xml.content(CGI.escapeHTML(item.body.to_s), :type => "html")
     end
     
   end
diff --git a/public/stylesheets/admin.css b/public/stylesheets/admin.css
index f00a658..3f95b0c 100644
--- a/public/stylesheets/admin.css
+++ b/public/stylesheets/admin.css
@@ -193,6 +193,15 @@ input[type=radio] {
   margin-left: 5px;
 }
 
+#flash a {
+  text-decoration: underline;
+  -webkit-text-decoration-style: wavy;
+  text-decoration-style: wavy;
+  text-decoration-color: #b0b0b0;
+  text-decoration-thickness: 1px;
+  text-underline-offset: 2px;
+}
+
 #flash span {
   letter-spacing: 1px;
   margin-right: 10px;