Don't escape html entities when rich == raw line

changelogs/unreleased/36041-notification-title.yml

+---
+title: Don't escape html entities in InlineDiffMarkdownMarker
+merge_request:
+author:

lib/gitlab/string_range_marker.rb

 module Gitlab
   class StringRangeMarker
-    attr_accessor :raw_line, :rich_line
-
-    def initialize(raw_line, rich_line = raw_line)
-      @raw_line = raw_line
-      @rich_line = ERB::Util.html_escape(rich_line)
+    attr_accessor :raw_line, :rich_line, :html_escaped
+
+    def initialize(raw_line, rich_line = nil)
+      @raw_line = raw_line.dup
+      if rich_line.nil?
+        @rich_line = raw_line.dup
+        @html_escaped = false
+      else
+        @rich_line = ERB::Util.html_escape(rich_line)
+        @html_escaped = true
+      end
     end
 
     def mark(marker_ranges)
       return rich_line unless marker_ranges
 
-      rich_marker_ranges = []
-      marker_ranges.each do |range|
-        # Map the inline-diff range based on the raw line to character positions in the rich line
-        rich_positions = position_mapping[range].flatten
-        # Turn the array of character positions into ranges
-        rich_marker_ranges.concat(collapse_ranges(rich_positions))
+      if html_escaped
+        rich_marker_ranges = []
+        marker_ranges.each do |range|
+          # Map the inline-diff range based on the raw line to character positions in the rich line
+          rich_positions = position_mapping[range].flatten
+          # Turn the array of character positions into ranges
+          rich_marker_ranges.concat(collapse_ranges(rich_positions))
+        end
+      else
+        rich_marker_ranges = marker_ranges
       end
 
       offset = 0
@@ -31,7 +41,7 @@ module Gitlab
         offset += text.length - original_text.length
       end
 
-      rich_line.html_safe
+      @html_escaped ? rich_line.html_safe : rich_line
     end
 
     private

spec/helpers/diff_helper_spec.rb

@@ -135,10 +135,10 @@ describe DiffHelper do
     it "returns strings with marked inline diffs" do
       marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line)
 
-      expect(marked_old_line).to eq(%q{abc <span class="idiff left right deletion">&#39;def&#39;</span>})
-      expect(marked_old_line).to be_html_safe
-      expect(marked_new_line).to eq(%q{abc <span class="idiff left right addition">&quot;def&quot;</span>})
-      expect(marked_new_line).to be_html_safe
+      expect(marked_old_line).to eq(%q{abc <span class="idiff left right deletion">'def'</span>})
+      expect(marked_old_line).not_to be_html_safe
+      expect(marked_new_line).to eq(%q{abc <span class="idiff left right addition">"def"</span>})
+      expect(marked_new_line).not_to be_html_safe
     end
   end
 

spec/lib/gitlab/diff/inline_diff_markdown_marker_spec.rb

@@ -6,9 +6,9 @@ describe Gitlab::Diff::InlineDiffMarkdownMarker do
     let(:inline_diffs) { [2..5] }
     let(:subject) { described_class.new(raw).mark(inline_diffs, mode: :deletion) }
 
-    it 'marks the range' do
-      expect(subject).to eq("ab{-c 'd-}ef'")
-      expect(subject).to be_html_safe
+    it 'does not escape html etities and marks the range' do
+      expect(subject).to eq("ab{-c 'd-}ef'")
+      expect(subject).not_to be_html_safe
     end
   end
 end

spec/lib/gitlab/diff/inline_diff_marker_spec.rb

@@ -2,11 +2,13 @@ require 'spec_helper'
 
 describe Gitlab::Diff::InlineDiffMarker do
   describe '#mark' do
+    let(:inline_diffs) { [2..5] }
+    let(:raw) { "abc 'def'" }
+
+    subject { described_class.new(raw, rich).mark(inline_diffs) }
+
     context "when the rich text is html safe" do
-      let(:raw) { "abc 'def'" }
       let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">&#39;def&#39;</span>}.html_safe }
-      let(:inline_diffs) { [2..5] }
-      let(:subject) { described_class.new(raw, rich).mark(inline_diffs) }
 
       it 'marks the range' do
         expect(subject).to eq(%{<span class="abc">ab<span class="idiff left">c</span></span><span class="space"><span class="idiff"> </span></span><span class="def"><span class="idiff right">&#39;d</span>ef&#39;</span>})
@@ -15,12 +17,10 @@ describe Gitlab::Diff::InlineDiffMarker do
     end
 
     context "when the text text is not html safe" do
-      let(:raw) { "abc 'def'" }
-      let(:inline_diffs) { [2..5] }
-      let(:subject) { described_class.new(raw).mark(inline_diffs) }
+      let(:rich) { "abc 'def' differs" }
 
       it 'marks the range' do
-        expect(subject).to eq(%{ab<span class="idiff left right">c &#39;d</span>ef&#39;})
+        expect(subject).to eq(%{ab<span class="idiff left right">c &#39;d</span>ef&#39; differs})
         expect(subject).to be_html_safe
       end
     end

spec/lib/gitlab/string_range_marker_spec.rb

@@ -2,34 +2,39 @@ require 'spec_helper'
 
 describe Gitlab::StringRangeMarker do
   describe '#mark' do
+    def mark_diff(rich = nil)
+      raw = 'abc <def>'
+      inline_diffs = [2..5]
+
+      described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:|
+        "LEFT#{text}RIGHT"
+      end
+    end
+
     context "when the rich text is html safe" do
-      let(:raw)  { "abc <def>" }
       let(:rich) { %{<span class="abc">abc</span><span class="space"> </span><span class="def">&lt;def&gt;</span>}.html_safe }
-      let(:inline_diffs) { [2..5] }
-      subject do
-        described_class.new(raw, rich).mark(inline_diffs) do |text, left:, right:|
-          "LEFT#{text}RIGHT"
-        end
-      end
 
       it 'marks the inline diffs' do
-        expect(subject).to eq(%{<span class="abc">abLEFTcRIGHT</span><span class="space">LEFT RIGHT</span><span class="def">LEFT&lt;dRIGHTef&gt;</span>})
-        expect(subject).to be_html_safe
+        expect(mark_diff(rich)).to eq(%{<span class="abc">abLEFTcRIGHT</span><span class="space">LEFT RIGHT</span><span class="def">LEFT&lt;dRIGHTef&gt;</span>})
+        expect(mark_diff(rich)).to be_html_safe
       end
     end
 
     context "when the rich text is not html safe" do
-      let(:raw)  { "abc <def>" }
-      let(:inline_diffs) { [2..5] }
-      subject do
-        described_class.new(raw).mark(inline_diffs) do |text, left:, right:|
-          "LEFT#{text}RIGHT"
+      context 'when rich text equals raw text' do
+        it 'marks the inline diffs' do
+          expect(mark_diff).to eq(%{abLEFTc <dRIGHTef>})
+          expect(mark_diff).not_to be_html_safe
         end
       end
 
-      it 'marks the inline diffs' do
-        expect(subject).to eq(%{abLEFTc &lt;dRIGHTef&gt;})
-        expect(subject).to be_html_safe
+      context 'when rich text doeas not equal raw text' do
+        let(:rich)  { "abc <def> differs" }
+
+        it 'marks the inline diffs' do
+          expect(mark_diff(rich)).to eq(%{abLEFTc &lt;dRIGHTef&gt; differs})
+          expect(mark_diff(rich)).to be_html_safe
+        end
       end
     end
   end