Skip to content

Commit 3438979

Browse files
committed
Deprecate content for void elements in TagBuilder
According to the [HTML5 Spec][1] > Void elements can't have any contents (since there's no end tag, no > content can be put between the start tag and the end tag). Up to this point, the only special handling of void elements has been to use ">" to close them instead of "/>" (which is optional but valid according to the spec) > Then, if the element is one of the void elements, ... , then there may > be a single U+002F SOLIDUS character (/), ... . On void elements, it > does not mark the start tag as self-closing but instead is unnecessary > and has no effect of any kind. This commit deprecates the ability to pass content (either through the positional "content" parameter or a block) to a void element since it is not valid according to the Spec. This has the benefit of both encouraging more correct HTML generation, and also simplifying the method definition of void elements once the deprecation has been removed. This commit additionally tweaks the signature of "void_tag_string" slightly with two changes. The first change is renaming it to be "self_closing_tag_string". This is more accurate than "void_tag_string" because the definition of "void element" is more specific and has more requirements than "self closing element". For example, tags in the SVG namespace _can_ be self closing but the "/" at the end of the start tag is _not_ optional because they are not void elements. The second change to this method is swapping from a boolean "self_closing" parameter to a string "tag_suffix" parameter. This enables the void element method definition to specialize the tag_suffix (to just ">") without either void elements or self closing elements having to pay the runtime cost of the self_closing conditional since we know at method definition time which suffix each type of tag should use. [1]: https://html.spec.whatwg.org/multipage/syntax.html#void-elements
1 parent 82e33e4 commit 3438979

File tree

3 files changed

+34
-9
lines changed

3 files changed

+34
-9
lines changed

actionview/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
* Deprecate passing content to void elements when using `tag.br` type tag builders.
2+
3+
*Hartley McGuire*
4+
15
* Fix the `number_to_human_size` view helper to correctly work with negative numbers.
26

37
*Earlopain*

actionview/lib/action_view/helpers/tag_helper.rb

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,22 +58,39 @@ def #{method_name}(content = nil, escape: true, **options, &block)
5858
end
5959
end
6060

61-
def self.define_void_element(name, code_generator:, method_name: name.to_s.underscore, self_closing: false)
61+
def self.define_void_element(name, code_generator:, method_name: name.to_s.underscore)
6262
code_generator.define_cached_method(method_name, namespace: :tag_builder) do |batch|
6363
batch.push(<<~RUBY)
6464
def #{method_name}(content = nil, escape: true, **options, &block)
6565
if content || block
66-
tag_string(#{name.inspect}, content, escape: escape, **options, &block)
66+
ActionView.deprecator.warn <<~TEXT
67+
Putting content inside a void element (#{name}) is invalid
68+
according to the HTML5 spec, and so it is being deprecated
69+
without replacement. In Rails 7.3, passing content as a
70+
positional argument will raise, and using a block will have
71+
no effect.
72+
TEXT
73+
tag_string("#{name}", content, escape: escape, **options, &block)
6774
else
68-
void_tag_string(#{name.inspect}, options, escape, #{self_closing})
75+
self_closing_tag_string("#{name}", options, escape, ">")
6976
end
7077
end
7178
RUBY
7279
end
7380
end
7481

75-
def self.define_self_closing_element(name, **options)
76-
define_void_element(name, self_closing: true, **options)
82+
def self.define_self_closing_element(name, code_generator:, method_name: name.to_s.underscore)
83+
code_generator.define_cached_method(method_name, namespace: :tag_builder) do |batch|
84+
batch.push(<<~RUBY)
85+
def #{method_name}(content = nil, escape: true, **options, &block)
86+
if content || block
87+
tag_string("#{name}", content, escape: escape, **options, &block)
88+
else
89+
self_closing_tag_string("#{name}", options, escape)
90+
end
91+
end
92+
RUBY
93+
end
7794
end
7895

7996
ActiveSupport::CodeGenerator.batch(self, __FILE__, __LINE__) do |code_generator|
@@ -228,8 +245,8 @@ def tag_string(name, content = nil, escape: true, **options, &block)
228245
content_tag_string(name, content, options, escape)
229246
end
230247

231-
def void_tag_string(name, options, escape = true, self_closing = false)
232-
"<#{name}#{tag_options(options, escape)}#{self_closing ? " />" : ">"}".html_safe
248+
def self_closing_tag_string(name, options, escape = true, tag_suffix = " />")
249+
"<#{name}#{tag_options(options, escape)}#{tag_suffix}".html_safe
233250
end
234251

235252
def content_tag_string(name, content, options, escape = true)

actionview/test/template/tag_helper_test.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,15 @@ def test_tag_builder_void_tag
2727
end
2828

2929
def test_tag_builder_void_tag_with_forced_content
30-
assert_equal "<br>some content</br>", tag.br("some content")
30+
assert_deprecated(ActionView.deprecator) do
31+
assert_equal "<br>some content</br>", tag.br("some content")
32+
end
3133
end
3234

3335
def test_tag_builder_void_tag_with_empty_content
34-
assert_equal "<br></br>", tag.br("")
36+
assert_deprecated(ActionView.deprecator) do
37+
assert_equal "<br></br>", tag.br("")
38+
end
3539
end
3640

3741
def test_tag_builder_self_closing_tag

0 commit comments

Comments
 (0)