Prior to this commit, the
[ActionView::Helpers::UrlHelper#button_to][button_to] helper rendered
`<input type="submit">` elements when passed its contents as a String
argument, and rendered `<button type="submit">` elements when passed its
contents as a block.
This difference is subtle, and might lead to surprises.
Additionally, a `<form>` element's submitter can encode a `name`/`value`
pairing, which will be submitted as part of the request. When
`button_to` renders an `<input type="submit">` element, the "button"
content is rendered as a `[value]` attribute, which prevents any
meaningful data from being encoded.
Since it's a single `<button>` or `<input type="submit">` within a
`<form>`, missing out on that opportunity to encode information might
not be a show stopper, but ensuring that a `<button>` element is
rendered _without_ a default `[value]` attribute enables applications to
encode additional information that can be accessed JavaScript as
`element.value`, instead of a workaround like
`element.getAttribute("data-value")`.
Support rendering `input` elements with button_to
---
To support the original behavior of `button_to` rendering `<input
type="submit">` elements when invoked _without_ a block, expose the
`app.config.button_to_generates_button_tag` configuration flag.
By default, it's set to `true` and ensures that all `button_to` calls
render `<button>` elements. To revert to the original behavior, set it
to `false`.
[button_to]: https://api.rubyonrails.org/v6.0/classes/ActionView/Helpers/UrlHelper.html#method-i-button_to
Co-authored-by: Dusan Orlovic <duleorlovic@gmail.com>
PR #39939 added support for the `Link` header being generated
automatically when using `stylesheet_link_tag` and
`javascript_include_tag`. However not everything should be
preloaded, e.g. a link to a legacy IE stylesheet has no need to be
preloaded because IE doesn't support the header and in some browsers it
will trigger the preload even though it's not used since it's inside an
IE conditional comment. This leads to increased bandwith costs and
slower application performance.
To allow more flexibility for sites that may have complex needs for the
`Link` header this commit adds a configuration option that disables it
completely and leaves it up to the application to decide how to handle
generating a `Link` header.
* Fix `SELECT COUNT` queries when rendering ActiveRecord collections
Fixes#40837
When rendering collections, calling `size` when the collection is an
ActiveRecord relation causes unwanted `SELECT COUNT(*)` queries. This
change ensures the collection is an array before getting the size, and
also loads the relation for any further array inspections.
* Test queries when rendering relation collections
* Add `length` support to partial collection iterator
Allows getting the size of a relation without duplicating records, but
still loads the relation. The length method existence needs to be
checked because you can pass in an `Enumerator`, which does not respond
to `length`.
* Ensure unsubscribed from notifications after tests
[Rafael Mendonça França + aar0nr]
When a stylesheet or javascript link tag (or preload link tag) is output
in the view, it also gets sent as a Link header for preloading. This
Link header is missing the integrity attribute even when one is set on
the tag, which prevents the browser from using the preloaded resource.
Co-authored-by: Adrianna Chang <adrianna.chang@shopify.com>
`I18n.translate` returns `nil` when given a `nil` key, *unless* a
`default` is also specified. If a `default` is specified, the `nil` key
is treated as a missing key.
In Rails 6.0, the `translate` helper always returned `nil` when given a
`nil` key. After #40773, the `translate` helper always raised an
`I18n::ArgumentError` when given a `nil` key. This commit fixes the
`translate` helper to mirror the `I18n.translate` behavior when given a
`nil` key, with and without a `default`.
Follow-up to #39989.
`I18n.translate` converts the initial key (but not `default` keys) to a
string before performing a lookup. For parity, we should do the same.
Browser native support for lazy loading images is now a part of the
official HTML standard. To indicate to the browser that an image should
be lazily loaded, add the `loading="lazy"` attribute to the `img` tag.
Or, in Rails parlance, add the `loading: "lazy"` option to the
`image_tag` call.
This commit adds `Rails.application.config.action_view.image_loading` to
configure the default value of the `image_tag` `:loading` option. Thus
by setting `config.action_view.image_loading = "lazy"`, an application
can opt in to lazy loading images sitewide, without changing view code.
`ActionView::Helpers::FormBuilder#id`
---
Generate an HTML `id` attribute value.
Return the [`<form>` element's][mdn-form] `id` attribute.
```html+erb
<%= form_for @post do |f| %>
<%# ... %>
<% content_for :sticky_footer do %>
<%= form.button(form: f.id) %>
<% end %>
<% end %>
```
In the example above, the `:sticky_footer` content area will exist
outside of the `<form>` element. [By declaring the `form` HTML
attribute][mdn-button-attr-form], we hint to the browser that the
generated `<button>` element should be treated as the `<form>` element's
submit button, regardless of where it exists in the DOM.
[A similar pattern could be used for `<input>`
elements][mdn-input-attr-form] (or other form controls) that do not
descend from the `<form>` element.
`ActionView::Helpers::FormBuilder#field_id`
---
Generate an HTML <tt>id</tt> attribute value for the given field
Return the value generated by the <tt>FormBuilder</tt> for the given
attribute name.
```html+erb
<%= form_for @post do |f| %>
<%= f.label :title %>
<%= f.text_field :title, aria: { describedby: form.field_id(:title, :error) } %>
<span id="<%= f.field_id(:title, :error) %>">is blank</span>
<% end %>
```
In the example above, the <tt><input type="text"></tt> element built by
the call to <tt>FormBuilder#text_field</tt> declares an
<tt>aria-describedby</tt> attribute referencing the <tt><span></tt>
element, sharing a common <tt>id</tt> root (<tt>post_title</tt>, in this
case).
This method is powered by the `field_id` helper declared in
`action_view/helpers/form_tag_helper`, which is made available for
general template calls, separate from a `FormBuilder` instance.
[mdn-form]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/form
[mdn-button-attr-form]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#attr-form
[mdn-input-attr-form]: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attr-form
[mdn-aria-describedby]: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-describedby_attribute
[w3c-wai]: https://www.w3.org/WAI/tutorials/forms/notifications/#listing-errors
Transforms a Hash into HTML Attributes, ready to be interpolated into
ERB.
```html+erb
<input <%= tag.attributes(type: :text, aria: { label: "Search" }) %> >
<%# => <input type="text" aria-label="Search"> %>
```
Utilizes the `ActionView::Helpers::TagHelper#tag_options` implementation
to enable combining ERB with attribute transformation, without requiring
templates to replace HTML strings with `tag` or `content_tag`
invocations.
Co-authored-by: David Heinemeier Hansson <david@loudthinking.com>
When serializing a Regexp instance, encode the [Regexp#source][]. When
encoding a value into a [pattern][] attribute from ERB/Ruby, declaring
the String can be tedious. For example, one might attempt to encode
`\w+` as `"\\\w+"`, but once serialized to the browser, that is not
equivalent to the `"\w+"` HTML attribute.
Instead, enable declaring Regexp and Regexp literals as attributes, and
encoding them as their source String.
[Regexp#source]: https://ruby-doc.org/core-2.7.2/Regexp.html#method-i-source
[pattern]: https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/pattern
* Fix regression for select tag helper with array
v6.1.0.rc1 does not generate DOM with the selected attribute
when the object's method returns an array.
This is because it has been changed by #34809 to always convert
to a string.
This commit fixes the issue.
## Steps to reproduce
```ruby
# frozen_string_literal: true
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails", ENV["RAILS_VERSION"]
end
require "action_view"
require "minitest/autorun"
class BugTest < ActionView::TestCase
Post = Struct.new("Post", :tags)
def test_stuff
@post = Post.new
@post.tags = ["foo", "bar"]
expected = <<~DOM.strip
<select name="post[tags]" id="post_tags"><option selected="selected" value="foo">foo</option>
<option selected="selected" value="bar">bar</option>
<option value="buz">buz</option></select>
DOM
assert_dom_equal(expected, select("post", "tags", %W(foo bar buz), { multiple: true }))
end
end
```
The test succeeds on v6.0.3.4, but the test fails on v6.1.0.rc1.
* Update actionview/lib/action_view/helpers/tags/select.rb
[Takumi Shotoku + Rafael Mendonça França]
As a follow-up to [rails/rails#40479][], ensure that empty Hash and
Array arguments are treated as `nil`.
For example, when conditionally rendering an [aria-describedby][]
attribute to associate an input with a related validation error, treat
an empty Array as `nil`, an omit the attribute entirely:
```html+erb
<% post = Post.new %>
<%= form_with model: post do |form| %>
<%= form.text_field :title, aria: { describedby: { post_title_error: post.errors[:title].any? } } %>
<%= tag.span(post.errors[:title].to_sentence, id: :post_title_error) if post.errors[:title].any? %>
<% end %>
```
In this example, when there are no errors, the desired outcome is for
the `<input type="text" name="post[title]">` element to _omit_ the
`[aria-describedby="post_title_error"]` attribute, and to only include
it when there are errors on the `title` attribute.
Without this change, the desired outcome can be achieved with a
combination of a `#token_list` and `#presence` call:
```diff
<% post = Post.new %>
<%= form_with model: post do |form| %>
- <%= form.text_field :title, aria: { describedby: {post_title_error: post.errors[:title].any?} }
+ <%= form.text_field :title, aria: { describedby: token_list(post_title_error: post.errors[:title].any?).presence } %>
<%= tag.span(post.errors[:title].to_sentence, id: :post_title_error) if post.errors[:title].any? %>
<% end %>
```
[rails/rails#40479]: https://github.com/rails/rails/pull/40479
[aria-describedby]: https://www.w3.org/WAI/WCAG21/Techniques/aria/ARIA21#example-2-identifying-errors-in-data-format
When translating a `<button>` element's contents, it is tedious to make
the translation text available to a block scope.
For instance, when rendering a `<button type="submit">` with an SVG
element as its child, passing translated label text to that SVG
element's [`<title>`][svg-title] element requires an extra call to
`I18n.translate`.
Prior to this commit, doing so would require a double lookup of the
translation key:
```erb
<%# one time here, implicitly %>
<%= form.button do %>
<svg>
<title>
<!-- one time here, explicitly -->
<%= translate("helpers.submit.post.create") %>
</title>
<!-- ... -->
</svg>
<% end %>
```
This commit modifies the `ActionView::Helpers::FormBuilder#button` to
check for invocations that are passed a block, and conditionally yield
the contents of `submit_default_value` as the argument.
The new view code might look something like this:
```erb
<%= form.button do |text| %>
<svg>
<title><%= text %></title>
<!-- ... -->
</svg>
<% end %>
```
Callers of the helper are still free to omit the block parameter.
[svg-title]: https://developer.mozilla.org/en-US/docs/Web/SVG/Element/title
When translating a `<label>` element's contents, it is difficult (or
"possible", yet undocumented) to make the translation text available to
a block scope.
For instance, when rendering a `rich_text_area`, passing the
`aria-label` attribute might be important.
Prior to this commit, doing so would require a double lookup of the
translation key:
```erb
<%# one time here, implicitly %>
<%= form.label(:content) do %>
<%= form.rich_text_area(
:content,
# one time here, explicitly
"aria-label" => translate("helpers.label.post.content"),
) %>
<% end %>
```
The current implementation of the `#label` helper method already yields
an instance of `ActionView::Helpers::Tags::Label::LabelBuilder`, but
that class is undocumented. Instance of that class respond to
`#translation` calls, which will return the translated text content.
By aliasing `#translation` to `#to_s`, we're able to expose that value
without the burden of exposing an additional class to the public API.
Instead, view-level interpolation (either `<%= %>`, `#{ }`, or direct
calls to [`capture`][capture] will coerce the value to a String, and
implicitly invoke `#translation`.
The new view code might look something like this:
```erb
<%= form.label(:content) do |label| %>
<%= form.rich_text_area(:content, "aria-label" => label) %>
<% end %>
```
Callers of the helper are still free to omit the block parameter.
[capture]: https://api.rubyonrails.org/classes/ActionView/Helpers/CaptureHelper.html#method-i-capture
In https://github.com/rails/rails/pull/37919, support
for rendering objects that respond_to render_in in
controllers was added. However, the implementation
did not support layouts.
This change updates the implementation from #37919
to more closely match the rest of the
ActionView::Template classes, enabling the use of layouts.
Co-authored-by: Felipe Sateler <fsateler@gmail.com>
When rendering a view with annotate_rendered_view_with_filenames
enabled, we were inserting newlines after the BEGIN and END
comments. These newlines are not ideal in some use cases, such
as rendering templates to strings that are then passed around
our application.
As the newlines were added for aesthetic purposes, we think it
makes sense to remove them so that rendered template strings do
not contain extraneous newlines.
As a result of removing these newlines, the annotations
no longer affect the reporting of template error on the correct
line, as addressed in #38950. As such, we've reverted those
changes as well.
Co-authored-by: Chris Gavin <chrisgavin@github.com>
Prior to this change, the following call raises:
```ruby
with_options(id: "with-options") { |t| t.p "content" }
```
The `ActionView::Helpers::TagHelper::TagBuilder` implementation relies
on `method_missing` to dispatch calls to `tag_string` where the missing
method name is the resulting element's tagName. Unfortunately,
[`Kernel#p` already exists][Kernel#p] and is invoked before
`method_missing` can intervene.
This commit rectifies this by declaring `TagBuilder#p` and overriding
the existent `#p` instance method.
[Kernel#p]: https://ruby-doc.org/core-2.7.2/Kernel.html#method-i-p
This refactor incidentally fixes a corner case when `translate` is
called with a block, the translation is missing, and
`debug_missing_translation` is false.
This commit adds a test for the above corner case, and additional tests
for existing behavior.
Prior to this commit, when a translation key indicated that the
translation text was HTML, the value returned by `I18n.translate` would
always be marked as `html_safe`. However, the value returned by
`I18n.translate` could be an untrusted value directly from
`options[:default]`.
This commit ensures values directly from `options[:default]` are not
marked as `html_safe`.
If automatically_disable_submit_tag is set to false then disable_with
is ignored, as result in all cases where disable_with is explicitly set
to false will produce unexpected result
Previously the same class, ActionView::TestCase::TestController, was
used to build a controller for every ActionView::TestCase class.
This caused issues when helpers/helper methods were set directly on the
controller (which from one test we seem to want to support).
This commit solves this by creating a new controller class for every
test case, which gives the controller a unique set of helpers to work
with.
Co-authored-by: John Crepezzi <seejohnrun@github.com>
This commit extends the `ActionView::Helpers#translate` (and by way of
alias, `#t`) helper methods to accept blocks.
When invoked with a block, the `translate` call will yield the
translated text as its first block argument, along with the resolved
translation key as its second:
```erb
<%= translate(".key") do |translation, resolved_key| %>
<span data-i18n-key="<%= resolved_key %>"><%= translation %></span>
<% end %>
```
In cases where relative translation keys are foregone in lieu of fully
qualified keys, or if the caller is not interested in the resolved key,
the second block argument can be omitted:
```erb
<%= translate("action.template.key") do |translation| %>
<p><%= translation %></p>
<p><%= translation %>, but a second time</p>
<% end %>
```
A benefit of yielding the translation is that it enabled template-local
variable re-use. Alternatively, [`Object#tap`][tap] could be used.
Prior to this commit, however, the resolution of the translation key was
internal to `ActionView`, and unavailable to the caller (unless they
were willing to explicitly determine the resolved key themselves). By
making it available as a block parameter, it could be used to annotate
the translated value in the resulting elements.
[tap]: https://ruby-doc.org/core-2.7.0/Object.html#method-i-tap
When rendering views an anonymous subclass is created by calling
ActionView::Base.with_empty_template_cache.
This causes inspect to return an unhelpful description when calling
inspect:
`#<#<Class:0x012345012345>:<0x012345012345>`.
This can be confusing when exceptions are raised because it's hard to
figure out where to look. For example calling an undefined method in a
template would raise the following exception:
undefined method `undefined' for #<#<Class:0x012345012345>:<0x012345012345>
Instead we can return the non-anonymous superclass name.
undefined method `undefined' for #<ActionView::Base:0x01234502345>
The anonymous class is created in ActionView::Base.with_empty_template_cache.
See f9bea6304d
This seems to be done for performance reasons only, without expecting a
change to calling `inspect`.
This pull request fixes#38697
It is caused by `@controller.singleton_class.include @routes.url_helpers` when `@controller` is nil in `ActionController::TestCase`.
* Without this commit
```ruby
% cd actionview
% PARALLEL_WORKERS=1 bin/test test/actionpack/controller/layout_test.rb test/template/url_helper_test.rb --seed 16702 -n "/^(?:LayoutSetInResponseTest#(?:test_layout_symbol_set_in_controller_returning_nil_falls_back_to_default)|UrlHelperTest#(?:test_url_for_with_array_and_only_path_set_to_false))$/"
Run options: --seed 16702 -n "/^(?:LayoutSetInResponseTest#(?:test_layout_symbol_set_in_controller_returning_nil_falls_back_to_default)|UrlHelperTest#(?:test_url_for_with_array_and_only_path_set_to_false))$/"
.E
Error:
UrlHelperTest#test_url_for_with_array_and_only_path_set_to_false:
ArgumentError: Missing host to link to! Please provide the :host parameter, set default_url_options[:host], or set :only_path to true
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/http/url.rb:64:in `full_url_for'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/http/url.rb:54:in `url_for'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:333:in `block in <class:RouteSet>'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:838:in `url_for'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:270:in `call'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:213:in `call'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/route_set.rb:326:in `block in define_url_helper'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb:233:in `polymorphic_method'
/Users/yahonda/src/github.com/rails/rails/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb:116:in `polymorphic_url'
/Users/yahonda/src/github.com/rails/rails/actionview/lib/action_view/routing_url_for.rb:104:in `url_for'
/Users/yahonda/src/github.com/rails/rails/actionview/test/template/url_helper_test.rb:102:in `test_url_for_with_array_and_only_path_set_to_false'
bin/test test/template/url_helper_test.rb:100
Finished in 0.042275s, 47.3093 runs/s, 47.3093 assertions/s.
2 runs, 2 assertions, 0 failures, 1 errors, 0 skips
%
```
The `:include_blank` option of various `<select>`-related helpers causes
an `<option>` element with no content to be rendered. However, the
[HTML spec] says that unless an `<option>` element has a `label`
attribute (which must be non-empty), its content must be "Text that is
not inter-element whitespace."
In #24923, this issue was addressed for `select_tag` by adding a `label`
attribute to the `<option>`. This commit addresses the issue in the
same manner for `FormBuilder#select` and various date / time select
helpers.
[HTML spec]: https://html.spec.whatwg.org/multipage/form-elements.html#the-option-element
Follow up to c07dff7227.
Actually it is not the cop's fault, but we mistakenly use `^`, `$`, and
`\Z` in much places, the cop doesn't correct those conservatively.
I've checked all those usage and replaced all safe ones.
When rendering a template with an explicit JS format, typically via
`respond_to :js`, we want to be able to render HTML partials without
having to specify their format, in order to make SJR more ergonomic.
Following the introduction of the @current_template variable in 1581cab,
the @virtual_path variable is now redundant, as the value of the virtual
path may be accessed via @current_template.virtual_path. This commit
removes @virtual_path and replaces any references to @virtual_path with
@current_template.virtual_path.
A Rails view may rely on several templates (e.g. layouts and partials)
in addition to the template for the action being rendered (e.g.
"show.html.erb"). To track which view file is currently being rendered
for the purpose of generating template tree digests used in cache
fragment keys, Action View uses a stack, the top item of which is
accessed via the @current_template variable (introduced in 1581cab).
Consider the following template:
<!-- home.html.erb -->
<%= render layout: "wrapper" do %>
<%= cache "foo" %>
HOME
<%= end %>
<%= end %>
Inside the block passed to the render helper, @current_template
corresponds to the wrapper.html.erb template instead of home.html.erb.
As wrapper.html.erb is then used as the root node for generating the
template tree digest used in the cache fragment key, the cache fragment
fails to expire upon changes to home.html.erb. Additionally, should a
second template use the wrapper.html.erb layout and contain a cache
fragment with the same key, the cache fragment keys for both templates
will be identical - causing cached content to "leak" from one view to
another (as described in #38984).
This commit skips adding templates to the stack when rendered as a
layout with a block via the render helper, ensuring correct and unique
cache fragment digests. Additionally, the virtual_path keyword arguments
found in CacheHelper and all references to the are removed as they no
longer possess any function. (Following the introduction of
@current_template, virtual_path is accessed via
@current_template.virtual_path rather than as a standalone variable.)
This allows `render_in` components that compile their own templates to
use their view context's current output buffer while a
`with_output_buffer` block is being evaluated.
Partially addresses #39377.
The template resolver is supposed to determine the handler, format, and
variant from the path in extract_handler_and_format_and_variant.
Previously this behaviour was close but didn't exactly match the
behaviour of finding templates, and in some cases (particularly with
handlers or formats missing) would return incorrect results.
This commit introduces Resolver::PathParser, a class which should be
able to accurately from any path inside a view directory be able to tell
us exactly the prefix, partial, variant, locale, format, variant, and
handler of that template.
This works by building a building a regexp from the known handlers and
file types. This requires that any resolvers have their cache cleared
when new handlers or types are registered (this was already somewhat the
requirement, since resolver lookups are cached, but this makes it
necessary in more situations).
The template resolver is supposed to determine the handler, format, and
variant from the path in extract_handler_and_format_and_variant.
This commit adds tests for how variants are parsed from the filenames
found in the resolver. It includes two skipped tests which currently fail.
- Add the configuration option for annotating templates with file names to the generated app.
- Add `annotate_rendered_view_with_filenames` option to configuring guide.
Allowing templates with "." introduces some ambiguity. Is index.html.erb
a template named "index" with format "html", or is it a template named
"index.html" without a format? We know it's probably the former, but if
we asked ActionView to render "index.html" we would currently get some
combination of the two: a Template with index.html as the name and
virtual path, but with html as the format.
This deprecates having "." anywhere in the template's name, we should
reserve this character for specifying formats. I think in 99% of cases
this will be people specifying `index.html` instead of simply `index`.
This was actually once deprecated in the 3.x series (removed in
6c57177f2c) but I don't think we can rely
on nobody having introduced this in the past 8 years.
ActionView::DependencyTracker looks through ERB templates using a regex
to find render calls. Previously this would incorrectly pick up
interpolated strings, like `render "foo/#{bar}"`.
This does not attempt to completely correct DependencyTracker, we can't
parse Ruby accurately with a regex, but should avoid a relatively common
case that previously was generating warnings.
I wrote this shell script to find words from the Rails repo,
so I can paste them into https://www.horsepaste.com/ for
the [codenames game](https://en.m.wikipedia.org/wiki/Codenames_(board_game)).
```bash
git grep -Il '' | \
grep -v -E "CHANGELOG|Gemfile|gemspec|package\.json|yarn\.lock" | \
xargs cat | \
sed '/[^ ]\{10,\}/d' | \
sed 's/\([A-Z]\)/ \1/g' | \
tr 'A-Z' 'a-z' | \
tr -c -s 'a-z' '\n' | \
sed '/^.\{0,3\}$/d' | \
sort | \
uniq | \
tr '\n' ',' | \
pbcopy
```
You can see the result in https://www.horsepaste.com/rails-fixed.
Click "Next game" to cycle the words.
Found some typos in the codebase from this 😂
This is how I generated the list of possible typos:
```bash
git grep -Il '' | \
grep -v -E "CHANGELOG|Gemfile|gemspec|package\.json|yarn\.lock" | \
xargs cat | \
sed '/[^ ]\{10,\}/d' | \
sed 's/\([A-Z]\)/ \1/g' | \
tr 'A-Z' 'a-z' | \
tr -c -s 'a-z' '\n' | \
sed '/^.\{0,3\}$/d' | \
sort | \
uniq | \
aspell --ignore-case list
```
I manually reviewed the list and made the corrections
in this commit. The rest on the list are either:
* Bugs in my script: it split things like "doesn't" into
"doesn" and "t", if find things like `#ffffff` and
extracts "ffffff" as a word, etc
* British spelling: honour, optimised
* Foreign words: bonjour, espanol
* Names: nginx, hanekawa
* Technical words: mutex, xhtml
* Portmanteau words: autosave, nodelist
* Invented words: camelize, coachee
* Shortened words: attrs, repo
* Deliberate typos: hllo, hillo (used in code examples, etc)
* Lorem ipsum words: arcu, euismod
This is the [output](https://gist.github.com/chancancode/eb0b573d667dc31906f33f1fb0b22313)
of the script *after* fixing the typos included in this
commit. In theory, someone can run that command again in
the future and compare the output to catch new typos (i.e.
using my list to filter out known typos).
Limitations: the aspell dictionary could be wrong, I
could have miss things, and my script ignores words that
are less than 3 characters or longer than 10 characters.
In testing https://github.com/rails/rails/pull/38848 in the
GitHub monolith, we realized that we probably should only
be annotating HTML output with these comments, at least
in their current format. By passing `format` to
`erb_implementation`, we set ourselves up to eventually
support annotations for other formats as well.
Allowing templates with "." introduces some ambiguity. Is index.html.erb
a template named "index" with format "html", or is it a template named
"index.html" without a format? We know it's probably the former, but if
we asked ActionView to render "index.html" we would currently get some
combination of the two: a Template with index.html as the name and
virtual path, but with html as the format.
This deprecates having "." anywhere in the template's name, we should
reserve this character for specifying formats. I think in 99% of cases
this will be people specifying `index.html` instead of simply `index`.
This was actually once deprecated in the 3.x series (removed in
6c57177f2c) but I don't think we can rely
on nobody having introduced this in the past 8 years.
As a developer, when looking at a page in my web browser, it's sometimes
difficult to figure out which template(s) are being used to render the page.
config.action_view.annotate_template_file_names adds HTML comments to the
rendered output indicating where each template begins and ends.
Co-authored-by: Aaron Patterson <tenderlove@github.com>
* master:
Add a regression test that ActionText caught
[ci skip] Use yml extension for locale files
Fix `helper_method` in `ActionView::TestCase` to allow keyword arguments
Fix `delegate_missing_to` to allow keyword arguments
Dump the schema or structure of a database when calling db:migrate:name
Reset the `ActiveRecord::Base` connection after `rails db:migrate:name`
Fix `unscope` when an `eq` node which has no arel attribute
Remove unused argument
Disallow calling `connected_to` on subclasses of `ActiveRecord::Base`
More less and lazy allocation for `assign_attributes` and `_assign_attributes`
Tweak contributing_to_ruby_on_rails.md [ci skip]
Clarify the difference between (old) `spec_name` and `connection_specification_name`
Remove duplicate part from deprecation warning
Fix deprecation warnings in connection_handlers_sharding_db_test.rb
Fixup CHANGELOGs [ci skip]
`reset_column_information` does not reset @predicate_builder
Simplify FixtureResolver to reuse filtering logic
Mostly remove bad test
Use type attribute in ActionView::Helpers::JavaScriptHelper#javascript_tag example
Update some references to finder options [ci skip]
The local variable name in `as:` may not be a valid local variable name,
but if there is no object specified to be assigned to the parameter,
then why supply the `as:`? This commit adds an object for the as param
This test was attempting to test how cache keys work by modifying the
templates and seeing when that cache was fresh. This doesn't actually
work for real Resolvers, only FixtureResolver, and isn't desirable. We
absolutely want to share templates if they resolve to the same file.
Instead, this simplifies the test to only check that we get the correct
template for the locale we request.
We're moving away from using validations in our
component framework, and feel that it's better
to avoid prescribing their usage in these
example classes, which exist to serve as example
objects that are compatible with the render_in
API.
We need a more complete understanding of mime types for the asset tags
to render properly, so we need to load this rather than just the stubs
included by actionview by default.
This fixes running these tests in isolation.
As a follow-up to https://github.com/rails/rails/pull/37872,
this change introduces a class_names view helper
to make it easier to conditionally apply class names
in views.
Before:
<div class="<%= item.for_sale? ? 'active' : '' %>">
After:
<div class="<%= class_names(active: item.for_sale?) %>">
We've been using this helper in the GitHub monolith
since 2016.
Co-authored-by: Aaron Patterson <tenderlove@github.com>
- #37872 introduced a regression and you can't do
```html.erb
hidden_field_tag('token', value: [1, 2, 3])
```
This will result in a `<input type="hidden" value=""`>.
I chose `hidden_field_tag` and the `value` attribute as an example
but this issue applies to any tag helper and any attributes.
https://github.com/rails/rails/pull/37872#issuecomment-561806468
mention that the feature should only apply for "class" attribute.
This commit fix original intent of #37872
Adds support for conditional values to TagBuilder,
extracting logic we use in the GitHub application,
inspired by https://github.com/JedWatson/classnames.
It’s common practice to conditionally apply CSS classes
in Rails views. This can lead to messy string interpolation,
often using ternaries:
```ruby
content_tag(
"My username",
class: "always #{'sometimes' if current_user.special?} another"
)
```
By adding support for hashes to TagBuilder, we can instead write the following:
```ruby
content_tag(
"My username",
class: ["always", "another", { 'sometimes' => current_user.special? }]
)
```
cc @JedWatson
`OptimizedFileSystemResolver` builds a regular expression to match view
template paths. Prior to this patch, only file globbing special
characters were escaped when building this regular expression, leaving
other regular expression special characters unescaped.
This patch properly escapes all regular expression special characters,
and adds test coverage for paths that include these characters.
Fixes#37107.
This hack prevails everywhere in the codebase by being copy & pasted, and it's actually not a negative thing but a necessary thing for framework implementors,
so it should better have a name and be a thing.
And with this commit, activesupport/test/abstract_unit.rb now doesn't silently autoload AS::TestCase,
so we're ready to establish clearner environment for running AS tests (probably in later commits)
* Added a phone_to helper method, on the style of mail_to and sms_to.
It creates an anchor tag with the href set to tel: *here your number*
which, when clicked on a mobile phone, or on a desktop with a supported
application, lets the phone app kick in, and it prepopulates it with the
phone number specified.
[Pietro Moro + Rafael Mendonça França]
The source_extract method will return nil when it can't find the file name in
the backtrace, methods that consume this method expect an array and the nil ends
up causing type errors down the road like it happened here: #36341. This
patch refactors the source_extract method so that it returns an empty
array instead of nil when it can't find the source code.
Co-authored-by: Kasper Timm Hansen <kaspth@gmail.com>
It is sometimes expected of the `translate` methods to return a Hash,
for instance it's the case of the `number.format` key.
As such users might need to specify a Hash default, e.g.
`translate(:'some.format', default: { separator: '.', delimiter: ',' })`.
This works as expected with the `I18n.translate` methods,
however `TranslationHelper#translate` apply `Array()` on the default value.
As a result the default value end up as `[:separator, '.', :delimiter, ',']`.
We shouldn't modify fixtures (or any files which are checked-in). It
prevents us from parallelizing, and probably has other issues.
We could fix these tests by copying the file to a tmpdir and modifying
it there, but I don't think they are testing anything useful anymore.
Re-initializing a resolver isn't representative of "uncached" rendering
(either in dev-mode or using lookup_context.disable_cache).
Since #35709, `Response#conten_type` returns only MIME type correctly.
It is a documented behavior that this method only returns MIME type, so
this change seems appropriate.
39de7fac05/actionpack/lib/action_dispatch/http/response.rb (L245-L249)
But unfortunately, some users expect this method to return all
Content-Type that does not contain charset. This seems to be breaking
changes.
We can change this behavior with the deprecate cycle.
But, in that case, a method needs that include Content-Type with
additional parameters. And that method name is probably the
`content_type` seems to properly.
So I changed the new behavior to more appropriate `media_type` method.
And `Response#content_type` changed (as the method name) to return Content-Type
header as it is.
Fixes#35709.
[Rafael Mendonça França & Yuuji Yaginuma ]
The templates rendered in RenderTestCases tests will be cached by the
resolvers unexpectedly. And this will break other tests when executed in
certain order. (See https://github.com/rails/rails/issues/36154 for more
detail)
So to fix this issue, we just need to clear the caches on all resolvers.
Performance cops will be extracted from RuboCop to RuboCop Performance
when next RuboCop 0.68 will be released.
https://github.com/rubocop-hq/rubocop/issues/5977
RuboCop 0.67 is its transition period.
Since rails/rails repository uses Performance cops, This PR added
rubocop-performance gem to Gemfile.
And this PR fixes some offenses using the following auto-correct.
```console
% bundle exec rubocop -a
Offenses:
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:212:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator =
> should be surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb:239:26:
C: [Corrected] Layout/SpaceAroundOperators: Operator => should be
surrounded by a single space.
"primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" }
^^
actionview/test/template/resolver_shared_tests.rb:1:1: C: [Corrected]
Style/FrozenStringLiteralComment: Missing magic comment #
frozen_string_literal: true.
module ResolverSharedTests
^
actionview/test/template/resolver_shared_tests.rb:10:33: C: [Corrected]
Layout/SpaceAroundEqualsInParameterDefault: Surrounding space missing in
default value assignment.
def with_file(filename, source="File at #{filename}")
^
actionview/test/template/resolver_shared_tests.rb:106:5: C: [Corrected]
Rails/RefuteMethods: Prefer assert_not_same over refute_same.
refute_same a, b
^^^^^^^^^^^
2760 files inspected, 5 offenses detected, 5 offenses corrected
```
This adds a bit of complexity, but is necessary for now to avoid holding
extra copies of templates which are resolved from ActionView::Digestor
after disabling cache on the lookup context.
Previously it's possible to have multiple copies of the "same" Template.
For example, if index.html.erb is found both the :en and :fr locale, it
will return a different Template object for each. The same can happen
with formats, variants, and handlers.
This commit de-duplicates templates, there will now only be one template
per file/virtual_path/locals tuple.
We need to consider virtual_path because both `render "index"`, and
`render "index.html"` can both find the same file but will have
different virtual_paths. IMO this is rare and should be
deprecated/removed, but it exists now so we need to consider it in order
to cache correctly.
This commit introduces a new UnboundTemplate class, which represents a
template with unknown locals. Template objects can be built from it by
using `#with_locals`. Currently, this is just a convenience around
caching templates, but I hope it's a helpful concept that could have
more utility in the future.
We didn't previously have many tests directly against the
OptimizedFileSystemResolver or FileSystemResolver, though usually
failures would be exposed through other tests.
It's easier to test some specifics of the behaviour with unit tests.
This also lets us test FileSystemResolver (non-optimized) which I don't
think previously had much testing (other than from classses inheriting
it).
Previously, we would discard the template source after rendering, if we
had a virtual path, in hopes that the virtual path would let us find our
same template again going through the Resolver.
Previously we discarded the source as an optimization, to avoid keeping
it around in memory. By instead just reading the file every time source
is called, as FileTemplate does, this is unnecessary.
This is because we only use hash to maintain the result. So when the key
are the same, the result would be skipped. The solution is to maintain
an array for tracking every item's position to restructure the result.
Previously, when using `render file:`, it was possible to render files
not only at an absolute path or relative to the current directory, but
relative to ANY view paths. This was probably done for absolutely
maximum compatibility when addressing CVE-2016-0752, but I think is
unlikely to be used in practice.
Tihs commit removes the ability to `render file:` with a path relative
to a non-fallback view path.
Make FallbackResolver.new private
To ensure nobody is making FallbackResolvers other than "/" and "".
Make reject_files_external_... no-op for fallbacks
Because there are only two values used for path: "" and "/", and
File.join("", "") == File.join("/", "") == "/", this method was only
testing that the absolute paths started at "/" (which of course all do).
This commit doesn't change any behaviour, but it makes it explicit that
the FallbackFileSystemResolver works this way.
Remove outside_app_allowed argument
Deprecate find_all_anywhere
This is now equivalent to find_all
Remove outside_app argument
Deprecate find_file for find
Both LookupContext#find_file and PathSet#find_file are now equivalent to
their respective #find methods.
This has similar problems to render file:.
I've never seen this used, and believe it's a relic from when all
templates could be rendered from an absolute path.
The previous behaviour of render file: was essentially the same as
render template:, except that templates can be specified as an absolute
path on the filesystem.
This makes sense for historic reasons, but now render file: is almost
exclusively used to render raw files (not .erb) like public/404.html. In
addition to complicating the code in template/resolver.rb, I think the
current behaviour is surprising to developers.
This commit deprecates the existing "lookup a template from anywhere"
behaviour and replaces it with "render this file exactly as it is on
disk". Handlers will no longer be used (it will render the same as if
the :raw handler was used), but formats (.html, .xml, etc) will still be
detected (and will default to :plain).
The existing render file: behaviour was the path through which Rails
apps were vulnerable in the recent CVE-2019-5418. Although the
vulnerability has been patched in a fully backwards-compatible way, I
think it's a strong hint that we should drop the existing
previously-vulnerable behaviour if it isn't a benefit to developers.
Custom glob patterns tie the implementation (Using Dir.glob) to the API
we provide.
It also doesn't really work. extract_handler_and_format_and_variant
expects the handler, format, and variant to be at the end of the
template path, and in the same order as they are in the default pattern.
This deprecates specifying a custom path for FileSystemResolver and
removes the pattern argument of OptimizedFileSystemResolver#initialize,
which does not work with a custom pattern.
Many tests were using `render file:`, but were only testing the
behaviour of `render template:` (file: just allows more paths/ is less
secure then template:).
The reason for so many `render file:` is probably that they were the old
default.
This commit replaces `render file:` with `render template:` anywhere the
test wasn't specifically interested in using `render file:`.
Marek Kasztelnik