Merge branch 'rs-fix-highlighting' into 'master'

Syntax highlighting improvements

On the server side:

During development I would occasionally see SanitizationFilter sanitizing
the result of SyntaxHighlightFilter, even though its attributes were
whitelisted. This updates the `clean_spans` transformer to return the
whitelisted node as [suggested by the Sanitize docs](http://git.io/vZR8i).

On the client side:

- Makes the syntax_highlight JS more flexible
- Adds JS specs
- Simplifies highlighting of new notes
- Adds highlighting to Markdown preview

See merge request !1278

5 files changed, 63 insertions, 7 deletions

diff --git a/app/assets/javascripts/dropzone_input.js.coffee b/app/assets/javascripts/dropzone_input.js.coffee
index a0dcaa8c27a..6f789e668af 100644
--- a/app/assets/javascripts/dropzone_input.js.coffee
+++ b/app/assets/javascripts/dropzone_input.js.coffee
@@ -167,6 +167,7 @@ class @DropzoneInput
           dataType: "json"
         ).success (data) ->
           preview.html data.body
+          preview.syntaxHighlight()
 
           renderReferencedUsers data.references.users
 
diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee
index b7f2c63c5a7..ce638c2641b 100644
--- a/app/assets/javascripts/notes.js.coffee
+++ b/app/assets/javascripts/notes.js.coffee
@@ -122,8 +122,9 @@ class @Notes
     # or skip if rendered
     if @isNewNote(note)
       @note_ids.push(note.id)
-      $('ul.main-notes-list').append(note.html)
-      $('.js-syntax-highlight').syntaxHighlight()
+      $('ul.main-notes-list').
+        append(note.html).
+        syntaxHighlight()
       @initTaskList()
 
   ###
diff --git a/app/assets/javascripts/syntax_highlight.coffee b/app/assets/javascripts/syntax_highlight.coffee
index 71295cd4b08..980f0232d10 100644
--- a/app/assets/javascripts/syntax_highlight.coffee
+++ b/app/assets/javascripts/syntax_highlight.coffee
@@ -1,3 +1,5 @@
+# Syntax Highlighter
+#
 # Applies a syntax highlighting color scheme CSS class to any element with the
 # `js-syntax-highlight` class
 #
@@ -6,7 +8,13 @@
 #   <div class="js-syntax-highlight"></div>
 #
 $.fn.syntaxHighlight = ->
-  $(this).addClass(gon.user_color_scheme)
+  if $(this).hasClass('js-syntax-highlight')
+    # Given the element itself, apply highlighting
+    $(this).addClass(gon.user_color_scheme)
+  else
+    # Given a parent element, recurse to any of its applicable children
+    $children = $(this).find('.js-syntax-highlight')
+    $children.syntaxHighlight() if $children.length
 
 $(document).on 'ready page:load', ->
   $('.js-syntax-highlight').syntaxHighlight()
diff --git a/lib/gitlab/markdown/sanitization_filter.rb b/lib/gitlab/markdown/sanitization_filter.rb
index 68ed57f6257..e368de7d848 100644
--- a/lib/gitlab/markdown/sanitization_filter.rb
+++ b/lib/gitlab/markdown/sanitization_filter.rb
@@ -67,12 +67,16 @@ module Gitlab
 
       def clean_spans
         lambda do |env|
-          return unless env[:node_name] == 'span'
-          return unless env[:node].has_attribute?('class')
+          node = env[:node]
 
-          unless has_ancestor?(env[:node], 'pre')
-            env[:node].remove_attribute('class')
+          return unless node.name == 'span'
+          return unless node.has_attribute?('class')
+
+          unless has_ancestor?(node, 'pre')
+            node.remove_attribute('class')
           end
+
+          { node_whitelist: [node] }
         end
       end
     end
diff --git a/spec/javascripts/syntax_highlight_spec.js.coffee b/spec/javascripts/syntax_highlight_spec.js.coffee
new file mode 100644
index 00000000000..6a73b6bf32c
--- /dev/null
+++ b/spec/javascripts/syntax_highlight_spec.js.coffee
@@ -0,0 +1,42 @@
+#= require syntax_highlight
+
+describe 'Syntax Highlighter', ->
+  stubUserColorScheme = (value) ->
+    window.gon ?= {}
+    window.gon.user_color_scheme = value
+
+  describe 'on a js-syntax-highlight element', ->
+    beforeEach ->
+      fixture.set('<div class="js-syntax-highlight"></div>')
+
+    it 'applies syntax highlighting', ->
+      stubUserColorScheme('monokai')
+
+      $('.js-syntax-highlight').syntaxHighlight()
+
+      expect($('.js-syntax-highlight')).toHaveClass('monokai')
+
+  describe 'on a parent element', ->
+    beforeEach ->
+      fixture.set """
+        <div class="parent">
+          <div class="js-syntax-highlight"></div>
+          <div class="foo"></div>
+          <div class="js-syntax-highlight"></div>
+        </div>
+      """
+
+    it 'applies highlighting to all applicable children', ->
+      stubUserColorScheme('monokai')
+
+      $('.parent').syntaxHighlight()
+
+      expect($('.parent, .foo')).not.toHaveClass('monokai')
+      expect($('.monokai').length).toBe(2)
+
+    it 'prevents an infinite loop when no matches exist', ->
+      fixture.set('<div></div>')
+
+      highlight = -> $('div').syntaxHighlight()
+
+      expect(highlight).not.toThrow()