Wrap javascript_tag contents in a CDATA section and add a cdata_section method to TagHelper. Closes #1691.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2543 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

7 files changed, 43 insertions, 24 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index b5fad73118..152c144a37 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Wrap javascript_tag contents in a CDATA section and add a cdata_section method to TagHelper #1691 [Michael Schuerig, Sam Stephenson]
+
 * Misc doc fixes (typos/grammar/etc). #2445. [coffee2code]
 
 * Speed improvement for session_options. #2287. [skaes@web.de]
diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb
index 72d9f6dcec..6fe90f970c 100644
--- a/actionpack/lib/action_view/helpers/javascript_helper.rb
+++ b/actionpack/lib/action_view/helpers/javascript_helper.rb
@@ -437,9 +437,13 @@ module ActionView
       # Returns a JavaScript tag with the +content+ inside. Example:
       #   javascript_tag "alert('All is good')" # => <script type="text/javascript">alert('All is good')</script>
       def javascript_tag(content)
-        content_tag("script", content, :type => "text/javascript")
+        content_tag("script", javascript_cdata_section(content), :type => "text/javascript")
       end
 
+      def javascript_cdata_section(content) #:nodoc:
+        "\n//#{cdata_section("\n#{content}\n//")}\n"
+      end
+      
     private
       def options_for_javascript(options)
         '{' + options.map {|k, v| "#{k}:#{v}"}.sort.join(', ') + '}'
@@ -480,11 +484,11 @@ module ActionView
       def build_observer(klass, name, options = {})
         options[:with] ||= 'value' if options[:update]
         callback = remote_function(options)
-        javascript = '<script type="text/javascript">'
-        javascript << "new #{klass}('#{name}', "
+        javascript  = "new #{klass}('#{name}', "
         javascript << "#{options[:frequency]}, " if options[:frequency]
         javascript << "function(element, value) {"
-        javascript << "#{callback}})</script>"
+        javascript << "#{callback}})"
+        javascript_tag(javascript)
       end
             
       def build_callbacks(options)
diff --git a/actionpack/lib/action_view/helpers/tag_helper.rb b/actionpack/lib/action_view/helpers/tag_helper.rb
index 2bd91ec547..6c71b8b767 100644
--- a/actionpack/lib/action_view/helpers/tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/tag_helper.rb
@@ -22,6 +22,15 @@ module ActionView
         "<#{name}#{tag_options(options.stringify_keys) if options}>#{content}</#{name}>"
       end
 
+      # Returns a CDATA section for the given +content+.  CDATA sections
+      # are used to escape blocks of text containing characters which would
+      # otherwise be recognized as markup. CDATA sections begin with the string
+      # <tt>&lt;![CDATA[</tt> and end with (and may not contain) the string 
+      # <tt>]]></tt>. 
+      def cdata_section(content)
+        "<![CDATA[#{content}]]>"
+      end
+
       private
         def tag_options(options)
           cleaned_options = convert_booleans(options.stringify_keys.reject {|key, value| value.nil?})
diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb
index eab7865937..461777e4e2 100644
--- a/actionpack/test/controller/capture_test.rb
+++ b/actionpack/test/controller/capture_test.rb
@@ -40,7 +40,7 @@ class CaptureTest < Test::Unit::TestCase
   def test_update_element_with_capture
     get :update_element_with_capture
     assert_equal(
-      "<script type=\"text/javascript\">$('products').innerHTML = '\\n  <p>Product 1</p>\\n  <p>Product 2</p>\\n';\n</script>" +
+      "<script type=\"text/javascript\">\n//<![CDATA[\n$('products').innerHTML = '\\n  <p>Product 1</p>\\n  <p>Product 2</p>\\n';\n\n//]]>\n</script>" +
         "\n\n$('status').innerHTML = '\\n  <b>You bought something!</b>\\n';", 
       @response.body.strip
     )
diff --git a/actionpack/test/template/java_script_macros_helper_test.rb b/actionpack/test/template/java_script_macros_helper_test.rb
index 5f96d06861..1e882dba04 100644
--- a/actionpack/test/template/java_script_macros_helper_test.rb
+++ b/actionpack/test/template/java_script_macros_helper_test.rb
@@ -23,15 +23,15 @@ class JavaScriptMacrosHelperTest < Test::Unit::TestCase
 
 
   def test_auto_complete_field
-    assert_dom_equal %(<script type=\"text/javascript\">new Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {})\n//]]>\n</script>),
       auto_complete_field("some_input", :url => { :action => "autocomplete" });
-    assert_dom_equal %(<script type=\"text/javascript\">new Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {tokens:','})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {tokens:','})\n//]]>\n</script>),
       auto_complete_field("some_input", :url => { :action => "autocomplete" }, :tokens => ',');
-    assert_dom_equal %(<script type=\"text/javascript\">new Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {tokens:[',']})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {tokens:[',']})\n//]]>\n</script>),
       auto_complete_field("some_input", :url => { :action => "autocomplete" }, :tokens => [',']);  
-    assert_dom_equal %(<script type=\"text/javascript\">new Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {min_chars:3})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {min_chars:3})\n//]]>\n</script>),
       auto_complete_field("some_input", :url => { :action => "autocomplete" }, :min_chars => 3);
-    assert_dom_equal %(<script type=\"text/javascript\">new Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {onHide:function(element, update){Alert('me');}})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Ajax.Autocompleter('some_input', 'some_input_auto_complete', 'http://www.example.com/autocomplete', {onHide:function(element, update){Alert('me');}})\n//]]>\n</script>),
       auto_complete_field("some_input", :url => { :action => "autocomplete" }, :on_hide => "function(element, update){Alert('me');}");
   end
   
@@ -50,7 +50,7 @@ class JavaScriptMacrosHelperTest < Test::Unit::TestCase
   def test_text_field_with_auto_complete
     assert_match "<style>",
       text_field_with_auto_complete(:message, :recipient)
-    assert_dom_equal %(<input id=\"message_recipient\" name=\"message[recipient]\" size=\"30\" type=\"text\" /><div class=\"auto_complete\" id=\"message_recipient_auto_complete\"></div><script type=\"text/javascript\">new Ajax.Autocompleter('message_recipient', 'message_recipient_auto_complete', 'http://www.example.com/auto_complete_for_message_recipient', {})</script>),
+    assert_dom_equal %(<input id=\"message_recipient\" name=\"message[recipient]\" size=\"30\" type=\"text\" /><div class=\"auto_complete\" id=\"message_recipient_auto_complete\"></div><script type=\"text/javascript\">\n//<![CDATA[\nnew Ajax.Autocompleter('message_recipient', 'message_recipient_auto_complete', 'http://www.example.com/auto_complete_for_message_recipient', {})\n//]]>\n</script>),
       text_field_with_auto_complete(:message, :recipient, {}, :skip_style => true)
   end
 end
diff --git a/actionpack/test/template/javascript_helper_test.rb b/actionpack/test/template/javascript_helper_test.rb
index caafe5387b..5967d29fc1 100644
--- a/actionpack/test/template/javascript_helper_test.rb
+++ b/actionpack/test/template/javascript_helper_test.rb
@@ -45,7 +45,7 @@ class JavaScriptHelperTest < Test::Unit::TestCase
   end
   
   def test_periodically_call_remote
-    assert_dom_equal %(<script type="text/javascript">new PeriodicalExecuter(function() {new Ajax.Updater('schremser_bier', 'http://www.example.com/mehr_bier', {asynchronous:true, evalScripts:true})}, 10)</script>),
+    assert_dom_equal %(<script type="text/javascript">\n//<![CDATA[\nnew PeriodicalExecuter(function() {new Ajax.Updater('schremser_bier', 'http://www.example.com/mehr_bier', {asynchronous:true, evalScripts:true})}, 10)\n//]]>\n</script>),
       periodically_call_remote(:update => "schremser_bier", :url => { :action => "mehr_bier" })
   end
   
@@ -106,12 +106,12 @@ class JavaScriptHelperTest < Test::Unit::TestCase
   end
   
   def test_observe_field
-    assert_dom_equal %(<script type=\"text/javascript\">new Form.Element.Observer('glass', 300, function(element, value) {new Ajax.Request('http://www.example.com/reorder_if_empty', {asynchronous:true, evalScripts:true})})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Form.Element.Observer('glass', 300, function(element, value) {new Ajax.Request('http://www.example.com/reorder_if_empty', {asynchronous:true, evalScripts:true})})\n//]]>\n</script>),
       observe_field("glass", :frequency => 5.minutes, :url => { :action => "reorder_if_empty" })
   end
   
   def test_observe_form
-    assert_dom_equal %(<script type=\"text/javascript\">new Form.Observer('cart', 2, function(element, value) {new Ajax.Request('http://www.example.com/cart_changed', {asynchronous:true, evalScripts:true})})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Form.Observer('cart', 2, function(element, value) {new Ajax.Request('http://www.example.com/cart_changed', {asynchronous:true, evalScripts:true})})\n//]]>\n</script>),
       observe_form("cart", :frequency => 2, :url => { :action => "cart_changed" })
   end
   
@@ -124,31 +124,31 @@ class JavaScriptHelperTest < Test::Unit::TestCase
   end
   
   def test_sortable_element
-    assert_dom_equal %(<script type=\"text/javascript\">Sortable.create('mylist', {onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}})</script>), 
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nSortable.create('mylist', {onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}})\n//]]>\n</script>), 
       sortable_element("mylist", :url => { :action => "order" })
-    assert_equal %(<script type=\"text/javascript\">Sortable.create('mylist', {constraint:'horizontal', onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}, tag:'div'})</script>), 
+    assert_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nSortable.create('mylist', {constraint:'horizontal', onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}, tag:'div'})\n//]]>\n</script>), 
       sortable_element("mylist", :tag => "div", :constraint => "horizontal", :url => { :action => "order" })
-    assert_dom_equal %|<script type=\"text/javascript\">Sortable.create('mylist', {constraint:'horizontal', containment:['list1','list2'], onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}})</script>|, 
+    assert_dom_equal %|<script type=\"text/javascript\">\n//<![CDATA[\nSortable.create('mylist', {constraint:'horizontal', containment:['list1','list2'], onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}})\n//]]>\n</script>|, 
       sortable_element("mylist", :containment => ['list1','list2'], :constraint => "horizontal", :url => { :action => "order" })
-    assert_dom_equal %(<script type=\"text/javascript\">Sortable.create('mylist', {constraint:'horizontal', containment:'list1', onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}})</script>), 
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nSortable.create('mylist', {constraint:'horizontal', containment:'list1', onUpdate:function(){new Ajax.Request('http://www.example.com/order', {asynchronous:true, evalScripts:true, parameters:Sortable.serialize('mylist')})}})\n//]]>\n</script>), 
       sortable_element("mylist", :containment => 'list1', :constraint => "horizontal", :url => { :action => "order" })
   end
   
   def test_draggable_element
-    assert_dom_equal %(<script type=\"text/javascript\">new Draggable('product_13', {})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Draggable('product_13', {})\n//]]>\n</script>),
       draggable_element('product_13')
-    assert_equal %(<script type=\"text/javascript\">new Draggable('product_13', {revert:true})</script>),
+    assert_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Draggable('product_13', {revert:true})\n//]]>\n</script>),
       draggable_element('product_13', :revert => true)
   end
   
   def test_drop_receiving_element
-    assert_dom_equal %(<script type=\"text/javascript\">Droppables.add('droptarget1', {onDrop:function(element){new Ajax.Request('http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nDroppables.add('droptarget1', {onDrop:function(element){new Ajax.Request('http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})\n//]]>\n</script>),
       drop_receiving_element('droptarget1')
-    assert_dom_equal %(<script type=\"text/javascript\">Droppables.add('droptarget1', {accept:'products', onDrop:function(element){new Ajax.Request('http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nDroppables.add('droptarget1', {accept:'products', onDrop:function(element){new Ajax.Request('http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})\n//]]>\n</script>),
       drop_receiving_element('droptarget1', :accept => 'products')
-    assert_dom_equal %(<script type=\"text/javascript\">Droppables.add('droptarget1', {accept:'products', onDrop:function(element){new Ajax.Updater('infobox', 'http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nDroppables.add('droptarget1', {accept:'products', onDrop:function(element){new Ajax.Updater('infobox', 'http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})\n//]]>\n</script>),
       drop_receiving_element('droptarget1', :accept => 'products', :update => 'infobox')
-    assert_dom_equal %(<script type=\"text/javascript\">Droppables.add('droptarget1', {accept:['tshirts','mugs'], onDrop:function(element){new Ajax.Updater('infobox', 'http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})</script>),
+    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nDroppables.add('droptarget1', {accept:['tshirts','mugs'], onDrop:function(element){new Ajax.Updater('infobox', 'http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})\n//]]>\n</script>),
       drop_receiving_element('droptarget1', :accept => ['tshirts','mugs'], :update => 'infobox')
   end
   
diff --git a/actionpack/test/template/tag_helper_test.rb b/actionpack/test/template/tag_helper_test.rb
index 5c413e8c89..7993afbf10 100644
--- a/actionpack/test/template/tag_helper_test.rb
+++ b/actionpack/test/template/tag_helper_test.rb
@@ -34,4 +34,8 @@ class TagHelperTest < Test::Unit::TestCase
     assert_equal content_tag("a", "Create", "href" => "create"),
                  content_tag("a", "Create", :href => "create")
   end
+  
+  def test_cdata_section
+    assert_equal "<![CDATA[<hello world>]]>", cdata_section("<hello world>")
+  end
 end