From 9c3217df50d462d6be4399e3e654d591bf704df7 Mon Sep 17 00:00:00 2001 From: erdgeist Date: Thu, 9 Jul 2026 15:29:40 +0200 Subject: Fix malformed-HTML fallout across RSS, link rewriting, and flash messaging Editors' TinyMCE output can contain unclosed void elements like
, 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 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. --- app/controllers/nodes_controller.rb | 18 ++++++++++++++--- app/models/page.rb | 39 ++++++++++++++----------------------- app/views/layouts/admin.html.erb | 7 +++++++ app/views/rss/updates.xml.builder | 4 +--- public/stylesheets/admin.css | 9 +++++++++ 5 files changed, 47 insertions(+), 30 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 = "
#{self.body}
" - 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} - - 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 - - tmp_body = xml_doc.to_s.gsub(/(\n\|\<\/div\>\n)/, "") - tmp_body.gsub!("", "") - - self.body = tmp_body + return unless self.body + + doc = Nokogiri::HTML::DocumentFragment.parse(self.body) + locales = I18n.available_locales.reject { |l| l == :root } + + doc.css('a').each do |link| + href = link['href'] + next unless href + next if href.start_with?('http://', 'https://') + next if href =~ /system\/uploads/ + + 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? %>
<%= 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] %> <%= flash[:error] %> <% 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; -- cgit v1.3