Prior to this commit, chaining more than one `token_list` calls with a
[data-action][] attribute value would result in one too many HTML
escapes. Additional subsequent calls would compound the problem.
For example, the following calls would result in an invalid descriptor
that's escaped too many times to be parsed.
```ruby
first = "click->controller#action1"
second = "click->controller#action2"
third = "click->controller#action3"
fourth = "click->controller#action4"
value = token_list(first, token_list(second, token_list(third)))
CGI.unescape_html value.to_s
# => "click->controller#action1 click->controller#action2 click->controller#action3 click->controller#action4"
```
By [CGI.unescape_html][] each `String` value before passing it to
[token_list][] (which re-escapes the value), we can preserve a lossless
concatenation process while also preserving the HTML safety.
After this commit, the previous example works as expected:
```ruby
first = "click->controller#action1"
second = "click->controller#action2"
third = "click->controller#action3"
fourth = "click->controller#action4"
value = token_list(first, token_list(second, token_list(third)))
CGI.unescape_html value.to_s
# => "click->controller#action1 click->controller#action2 click->controller#action3 click->controller#action4"
```
[unescaping]: https://ruby-doc.org/stdlib-2.5.3/libdoc/cgi/rdoc/CGI/Util.html#method-i-unescape_html
[token_list]:
https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-token_list
[data-action]: https://stimulus.hotwired.dev/reference/actions
I do this a lot:
```erb
<%= select :post, :author, authors, required: true %>
```
It doesn't work; the `required` attribute is ignored! Instead, you need to do this:
```erb
<%= select :post, :author, authors, {}, required: true %>
```
It's hard to remember the right API, and it looks to me like a code smell. It looks even smellier when you end up with this:
```erb
<%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
```
Where this would be nicer, but again, the `required` attribute is ignored:
```erb
<%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
```
This PR implements a special handling for `required`, `multiple`, and `size` HTML attributes so that these now do the same thing:
```erb
<%= select :post, :author, authors, include_blank: "Choose an option", required: true %>
<%= select :post, :author, authors, { include_blank: "Choose an option" }, { required: true } %>
```
ps. as proof I'm not the only person who makes this mistake, one of the tests in the Rails test suite was wrong! The test added in https://github.com/rails/rails/pull/40522 puts the `multiple` attribute in the wrong place and has the wrong assertion as as result. This PR includes a fix for the test.
This change makes date/time options (value, min, max) in `time_field`, `date_field`, `datetime_field`, `week_field`, `month_field` form helpers behave in a unified way.
Currently if you do this:
```ruby
check_box_tag "admin", "1", checked: false
```
It is treated [as truthy](19f9922523/actionview/lib/action_view/helpers/form_tag_helper.rb (L444)), and your checkbox is checked. This can be a bit surprising, particularly because the `FormHelper` version [does support](19f9922523/actionview/lib/action_view/helpers/form_helper.rb (L1285)) a keyword argument.
```ruby
f.check_box "admin", checked: false
```
So this PR updates `check_box_tag` and `radio_button_tag` to support `checked` as a positional or keyword argument, this way you can use the same API in both cases.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
You no longer need to call `new` when passing a class to `dom_id`.
This makes `dom_id` behave like `dom_class` in this regard.
Apart from saving a few keystrokes, it prevents Ruby from needing to
instantiate a whole new object just to generate a string.
Before:
```ruby
dom_id(Post) # NoMethodError: undefined method `to_key' for Post:Class
```
After:
```ruby
dom_id(Post) # "new_post"
```
You can still call `dom_id(Post.new)`.
When word_wrap's break_sequence contains non-rstrippable
characters, word_wrap fails to strip the extra break_sequence at the
end properly. Using chomp! helps cut exactly what was specified in the
argument.
It's possible for `ActionView::Helpers::FormTagHelper#field_name` calls
made by instances constructed through `fields` and `fields_for` helpers
to have an `object_name` argument that's `nil`. For example, the
following will raise an `undefined method `empty?' for nil:NilClass`
exception:
```erb
<%= fields do |f| %>
<%= f.field_name :body %>
<% end %>
```
To guard against those calls, replace the method's call to
`String#empty?` with `Object#blank?`, since `NilClass#empty?` is not
defined.
Because these strings contain no HTML elements and the basic entities
are escaped, they are safe to be included as-is as PCDATA in HTML
content. Tagging them as html-safe avoids double-escaping entities
when being concatenated to a SafeBuffer during rendering.
Fixes https://github.com/rails/rails-html-sanitizer/issues/124
Ensure models passed to `form_with` attempt to call `to_model`.
Now that `form_for` is implemented in terms of `form_with`, this commit
also removes the `convert_to_model` call from the `form_for` implementation.
To exercise this behavior, change existing `form_with` test coverage.
Prior to this change, a call to `form_with` made with a `model:` argument
that responds to `to_model` would not incorporate the instance's persistence
state into the form's HTTP verb. After this change, the persistence state
inferred from the `model:` argument's `to_model` call is incorporated into
the `<form>` element's `[method]` attribute.
This is a separate follow-up change proposed in [rails/rails#44328][].
The original change to restore old behavior _deliberately_ excluded
applying the same logic to `form_with`, since it would be a breaking
change from how `form_with` behaved previously.
This commit proposed making that breaking change.
[rails/rails#44328]: https://github.com/rails/rails/pull/44328#discussion_r808475585
Add the method ERB::Util.xml_name_escape to escape dangerous characters
in names of tags and names of attributes, following the specification of
XML.
Use that method in the tag helpers of ActionView::Helpers. Rename the option
:escape_attributes to :escape, to simplify by applying the option to the whole
tag.
Now it's possible to write
audio_tag(user.audio_file)
video_tag(user.video_file)
Instead of
audio_tag(polymorphic_path(user.audio_file))
video_tag(polymorphic_path(user.video_file))
image_tag already supported that, so this follows the same pattern.
PR #11517 updated `grouped_options_for_select` to allow passing HTML
attributes of the optgroup as the last element of each array, which is
called internally by `select` when it detects grouped choices.
However, the `select` helper detected grouped choices by seeing if the
[last element is an Array](6ec669b65d/actionview/lib/action_view/helpers/tags/select.rb (L37)),
meaning if you passed a hash of HTML attributes, it would no longer
treat the choices as grouped. This conflicted with
`grouped_options_for_select`, which assumes the individual options are
the [second element](6ec669b65d/actionview/lib/action_view/helpers/form_options_helper.rb (L546)),
not the last element.
Now there's agreement between `select` and `grouped_options_for_select`
in expecting the individual option choices to be the second element,
allowing the hash of HTML attributes to exist as the last element and
properly trigger grouped options.
The background
---
Configuration for replacing a collection was introduced in
[rails/rails#36716][].
However, since [rails/rails#42596][] has been merged, Rails 7.1 and
beyond will default to _replacing_ an Active Storage `has_many_attached`
relationship, as opposed to _appending to it_.
The problem
---
With replacement as the established precedent, it's currently a
challenge to replace an existing collection with an empty one.
The solution
---
This commit makes two changes.
The first is to Action View and its form building helpers. The change
draws inspiration from how an `<input type="checkbox">` field (or
collection of fields) is paired with an `<input type="hidden">` field to
represent the unchecked value. The change pairs any `<input type="file"
multiple="multiple">` elements with an `<input type="hidden">` element
to represent an empty collection. Like the [check_box][] form builder
method, the `file_field` method accepts an `include_hidden:` option to
skip the creation of the hidden element.
The second is to how Active Storage generates attribute assignment
methods through `has_many_attached`. With the possibility of an `<input
type="file">` field being paired with an `<input type="hidden"
value="">` field, the backing models need to be able to coerce an
"empty-ish" value into an empty list. For example:
```ruby
@user.highlights = [""]
@user.highlights # => []
```
When combined, these changes enable consumer applications to submit
"empty" collections to blank out existing attachments.
Support is configured through the
`config.active_storage.multiple_file_field_include_hidden` configuration
value, which defaults to `false`.
[check_box]: https://edgeapi.rubyonrails.org/classes/ActionView/Helpers/FormBuilder.html#method-i-check_box
[rails/rails#36716]: https://github.com/rails/rails/pull/36716
[rails/rails#42596]: https://github.com/rails/rails/pull/42596
Since `<button>` elements translate their `[name]` and `[value]`
attributes to the resulting `<form>` element submission, and are encoded
into the resulting `URLSearchParams` or `FormData` instance, Action View
`FormBuilder` instances should support encoding a method name the same
way it does for other fields.
For instance, consider this HTML:
```html
<button>Publish</button>
<button name="post[draft]" value="true">Save as draft</button>
```
Clicking the "Publish" button would submit the form without encoding any
additional `[name]` and `[value]` pairs.
Clicking the "Save as draft" button would submit the form and encode
`post[draft]=true` into the submission.
This commit changes the `FormBuilder#button` method to interpret a
`Symbol` as the first argument as a method name argument, and encodes
its value based on the form's `model:` or `scope:` value:
```erb
<%= form.button :draft, value: true do %>
Save as draft
<% end %>
end
<%# => <button name="post[draft]" value="true" type="submit"> %>
<%# Save as draft %>
<%# </button> %>
```
Co-authored-by: Rafael Mendonça França <rafael@rubyonrails.org>
Some background
---
By default, when a `<form>` is declared without an `[action]` attribute,
browsers will encode a `<form>`'s fields into the _current_ URL.
This can be useful for a `<form method="get">` that operates on the
current page. For example, when filtering search results, a form that
sorts:
```html
<form method="get">
<button name="sort" value="desc">Most to least</button>
<button name="sort" value="asc">Least to most</button>
</form>
```
can operate on a page that is filtered in another way by merging the
`?sort=asc` or `?sort=desc` values _into_ the existing page, which might
have the `?q=...` string set elsewhere.
The problem
---
Prior to this commit, none of the `<form>` construction variations
supported declaring a `<form>` without an `[action]` attribute.
`form_with`, `form_for`, and `form_tag` all default to `url_for({})`
when a `url:` or `action:` option is omitted.
The solution
---
Treat `url: false`, or `action: false` as an escape hatch to signal to
Action View that we don't need to transform the `model:` option or
argument into a Rails route.
Similarly, when calling `button_to` with `false` as the URL options
arguments will construct a `<form>` element without an `[action]`
attribute.
Instead of treating it as an anonymous block, execute the
`ActionView::Base.field_error_proc` within the context of the
`ActionView::Base` instance.
This enables consumer applications to continue to override the proc as
they see fit, but frees them from declaring templating logic within a
`config/initializers/*.rb`, `config/environments/*.rb` or
`config/application.rb` file.
This makes it possible to replace something like:
```ruby
config.action_view.field_error_proc = proc do |html_tag, instance|
<<~HTML.html_safe
#{html_tag}
<span class="errors">#{instance.error_message.to_sentence}</span>
HTML
end
```
With inline calls to Action View helpers like:
```ruby
config.action_view.field_error_proc = proc do |html_tag, instance|
safe_join [ html_tag, tag.span(instance.error_message.to_sentence, class: "errors") ]
end
```
Or with a view partial rendering, like:
```ruby
config.action_view.field_error_proc = proc do |html_tag, instance|
render partial: "application/field_with_errors", locals: { html_tag: html_tag, instance: instance }
end
```
Then, elsewhere in `app/views/application/field_with_errors.html.erb`:
```erb
<%= html_tag %>
<span class="errors"><%= instance.error_message.to_sentence %></span>
```
Infer HTTP verb `[method]` from a model or Array with model as the first
argument to `button_to` when combined with a block:
```ruby
button_to(Workshop.find(1)){ "Update" }
#=> <form method="post" action="/workshops/1" class="button_to">
#=> <input type="hidden" name="_method" value="patch" autocomplete="off" />
#=> <button type="submit">Update</button>
#=> </form>
button_to([ Workshop.find(1), Session.find(1) ]) { "Update" }
#=> <form method="post" action="/workshops/1/sessions/1" class="button_to">
#=> <input type="hidden" name="_method" value="patch" autocomplete="off" />
#=> <button type="submit">Update</button>
#=> </form>
```
Prior to this change, the constructed `<form>` was always submitted with
a `[method="post"]` and _always_ omitted the `<input type="hidden"
name="_method" value="...">` field, regardless of the return value of
the "model" argument's `#persisted?` predicate.
Use Kernel::Float(..., exceptions:false) instead of a rescue block in
ActionView::Helpers::NumberHelper and
ActiveSupport::NumberHelper::NumberConverter to slightly improve
performance.
Also remove documentation that incorrectly states
ActiveSupport::NumberHelper supports the `raise:` option.
Add `weekday_select` method
Create `Tags::WeekdaySelect` class
Add `weekday_select` to `FromBuilder`
Add Documentation
Allow `WeekdaySelect` to use selected option if value is nil
Doc fix
Add tests
Use kwrd args
Update CHANGELOG
Fix `Tags::WeekdaySelect` for updated kwrd args
Update CHANGELOG format
Condense `weekday_options_for_select` method
Update tests for kwargs
Caching something that shouldn't be cached is a potential source of
bugs and security vulnerabilities. For example, one could write a
form helper that outputs a request-specific auth token, only for
the helper to be used inside of a `cache` block.
In the GitHub application, we implemented a caching? method and used
it to raise an error if a specific code path is being cached that
we don't want to be cached.
I've credited its original author, @btoews.
Co-authored-by: Ben Toews <mastahyeti@gmail.com>
Co-authored-by: John Hawthorn <jhawthorn@github.com>
Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
Rafael Mendonça França