Skip to content

Commit

Permalink
FIX: Don't diplay character reference in HTML diffs (discourse#4204)
Browse files Browse the repository at this point in the history
* 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 "&discourse#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
  • Loading branch information
KamilaBorowska authored and ZogStriP committed May 9, 2016
1 parent 1883fa2 commit 9d737d8
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
3 changes: 1 addition & 2 deletions lib/discourse_diff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,7 @@ def end_element(name)
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
Expand Down
12 changes: 12 additions & 0 deletions spec/components/discourse_diff_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@
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
Expand Down Expand Up @@ -86,6 +92,12 @@
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
Expand Down

0 comments on commit 9d737d8

Please sign in to comment.