From 9d737d894dbefbcb9c7cda119c0965a2625c848c Mon Sep 17 00:00:00 2001
From: Konrad Borowski <x.fix@o2.pl>
Date: Mon, 9 May 2016 08:44:21 +0200
Subject: [PATCH] FIX: Don't diplay character reference in HTML diffs (#4204)

* FIX: Don't diplay character reference in HTML diffs

Before this change, HTML escaping was done before splitting text into
tokens, so token splitter saw literals like "&#39;", and split them as
it was normal text into parts into ["&", "#", "39", ";"]. This caused
diff to display character references, as those tokens used separate
HTML tags to display their insertion/deletion status.

* Avoid making one element arrays while generating diffs
---
 lib/discourse_diff.rb                  |  3 +--
 spec/components/discourse_diff_spec.rb | 12 ++++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/lib/discourse_diff.rb b/lib/discourse_diff.rb
index 60bdd3f064..91e4115fed 100644
--- a/lib/discourse_diff.rb
+++ b/lib/discourse_diff.rb
@@ -261,8 +261,7 @@ class DiscourseDiff
     end
 
     def characters(string)
-      string = CGI::escapeHTML(string)
-      @tokens.concat string.scan(/(\W|\w+[ \t]*)/).flatten
+      @tokens.concat string.scan(/\W|\w+[ \t]*/).map { |x| CGI::escapeHTML(x) }
     end
 
   end
diff --git a/spec/components/discourse_diff_spec.rb b/spec/components/discourse_diff_spec.rb
index c21737ad85..53150a5dfb 100644
--- a/spec/components/discourse_diff_spec.rb
+++ b/spec/components/discourse_diff_spec.rb
@@ -49,6 +49,12 @@ describe DiscourseDiff do
       expect(DiscourseDiff.new(before, after).inline_html).to eq("<div class=\"inline-diff\"><p class=\"diff-del\">this is the first paragraph</p><p>this is the second paragraph</p></div>")
     end
 
+    it "does not break diff on character references" do
+      before = "<p>'</p>"
+      after = "<p></p>"
+      expect(DiscourseDiff.new(before, after).inline_html).to eq("<div class=\"inline-diff\"><p><del>&#39;</del></p></div>")
+    end
+
   end
 
   describe "side_by_side_html" do
@@ -86,6 +92,12 @@ describe DiscourseDiff do
       expect(DiscourseDiff.new(before, after).side_by_side_html).to eq("<div class=\"span8\"><p class=\"diff-del\">this is the first paragraph</p><p>this is the second paragraph</p></div><div class=\"span8 offset1\"><p>this is the second paragraph</p></div>")
     end
 
+    it "does not break diff on character references" do
+      before = "<p>'</p>"
+      after = "<p></p>"
+      expect(DiscourseDiff.new(before, after).side_by_side_html).to eq("<div class=\"span8\"><p><del>&#39;</del></p></div><div class=\"span8 offset1\"><p></p></div>")
+    end
+
   end
 
   describe "side_by_side_markdown" do