Merge pull request #50159 from skipkayhil/hm-deprecate-void-content

Deprecate content for void elements in TagBuilder

actionview/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Deprecate passing content to void elements when using `tag.br` type tag builders.
+
+    *Hartley McGuire*
+
 *   Fix the `number_to_human_size` view helper to correctly work with negative numbers.
 
     *Earlopain*

actionview/lib/action_view/helpers/tag_helper.rb

@@ -58,22 +58,39 @@ def #{method_name}(content = nil, escape: true, **options, &block)
           end
         end
 
-        def self.define_void_element(name, code_generator:, method_name: name.to_s.underscore, self_closing: false)
+        def self.define_void_element(name, code_generator:, method_name: name.to_s.underscore)
           code_generator.define_cached_method(method_name, namespace: :tag_builder) do |batch|
             batch.push(<<~RUBY)
               def #{method_name}(content = nil, escape: true, **options, &block)
                 if content || block
-                  tag_string(#{name.inspect}, content, escape: escape, **options, &block)
+                  ActionView.deprecator.warn <<~TEXT
+                    Putting content inside a void element (#{name}) is invalid
+                    according to the HTML5 spec, and so it is being deprecated
+                    without replacement. In Rails 7.3, passing content as a
+                    positional argument will raise, and using a block will have
+                    no effect.
+                  TEXT
+                  tag_string("#{name}", content, escape: escape, **options, &block)
                 else
-                  void_tag_string(#{name.inspect}, options, escape, #{self_closing})
+                  self_closing_tag_string("#{name}", options, escape, ">")
                 end
               end
             RUBY
           end
         end
 
-        def self.define_self_closing_element(name, **options)
-          define_void_element(name, self_closing: true, **options)
+        def self.define_self_closing_element(name, code_generator:, method_name: name.to_s.underscore)
+          code_generator.define_cached_method(method_name, namespace: :tag_builder) do |batch|
+            batch.push(<<~RUBY)
+              def #{method_name}(content = nil, escape: true, **options, &block)
+                if content || block
+                  tag_string("#{name}", content, escape: escape, **options, &block)
+                else
+                  self_closing_tag_string("#{name}", options, escape)
+                end
+              end
+            RUBY
+          end
         end
 
         ActiveSupport::CodeGenerator.batch(self, __FILE__, __LINE__) do |code_generator|
@@ -228,8 +245,8 @@ def tag_string(name, content = nil, escape: true, **options, &block)
           content_tag_string(name, content, options, escape)
         end
 
-        def void_tag_string(name, options, escape = true, self_closing = false)
-          "<#{name}#{tag_options(options, escape)}#{self_closing ? " />" : ">"}".html_safe
+        def self_closing_tag_string(name, options, escape = true, tag_suffix = " />")
+          "<#{name}#{tag_options(options, escape)}#{tag_suffix}".html_safe
         end
 
         def content_tag_string(name, content, options, escape = true)

actionview/test/template/tag_helper_test.rb

@@ -27,11 +27,15 @@ def test_tag_builder_void_tag
   end
 
   def test_tag_builder_void_tag_with_forced_content
-    assert_equal "<br>some content</br>", tag.br("some content")
+    assert_deprecated(ActionView.deprecator) do
+      assert_equal "<br>some content</br>", tag.br("some content")
+    end
   end
 
   def test_tag_builder_void_tag_with_empty_content
-    assert_equal "<br></br>", tag.br("")
+    assert_deprecated(ActionView.deprecator) do
+      assert_equal "<br></br>", tag.br("")
+    end
   end
 
   def test_tag_builder_self_closing_tag