Skip to content

Commit

Permalink
Deprecate content for void elements in TagBuilder
Browse files Browse the repository at this point in the history
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
  • Loading branch information
skipkayhil committed Nov 24, 2023
1 parent 82e33e4 commit 3438979
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 9 deletions.
4 changes: 4 additions & 0 deletions 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*
Expand Down
31 changes: 24 additions & 7 deletions actionview/lib/action_view/helpers/tag_helper.rb
Expand Up @@ -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|
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions actionview/test/template/tag_helper_test.rb
Expand Up @@ -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
Expand Down

0 comments on commit 3438979

Please sign in to comment.