In 2abf6ca0c8 @cache_hit got introduced.
This was renamed to @cache_hits and revised in the subsequent commit
8240636bed but it seems one assignment was
overlooked.
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`.
Previously we were using module_eval to manually add some helpers in
ActionView::TestCase, which assumes a lot about what _helpers allows.
Instead, we can use a standard helper block to define these methods only
one time, and add a method to the new uniq controller class to tie back
to the test instance.
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>
We should avoid overriding name if possible because it is confusing to
developers and it also can causes bugs in code which deal with modules
but aren't using the Module.instance_method(:name).bind workaround.
I believe we can achieve the desired "method missing" behaviour by just
redefining inspect.
Refs https://github.com/rails/rails/pull/39819
All branches that use translated_text are covered so we can remove this
method call.
Also apply some whitespaces around conditionals to make them explicit.
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
This change swaps the CommonJS require() syntax in the Webpacker
application.js pack template file and in documentation examples with ES
module import syntax.
Benefits of this change include:
Provides continuity with the larger frontend community: Arguably, one of
the main draws in adopting Webpacker is its integration with Babel to
support ES module syntax. For a fresh Rails install with Webpacker, the
application.js file will be the first impression most Rails developers
have with webpack and Webpacker. Most of the recent documentation and
examples they will find online for using other libraries will be based
on ES module syntax.
Reduces confusion: Developers commonly add ES imports to their
application.js pack, typically by following online examples, which means
mixing require() and import statements in a single file. This leads to
confusion and unnecessary friction about differences between require()
and import.
Embraces browser-friendliness: The ES module syntax forward-looking and
is meant to be supported in browsers. On the other hand, require()
syntax is synchronous by design and not browser-supported as CommonJS
originally was adopted in Node.js for server-side JavaScript. That
webpack supports require() syntax is merely a convenience.
Encourages best practices regarding optimization: webpack can statically
analyze ES modules and "tree-shake", i.e., strip out unused exports from
the final build (given certain conditions are met, including
`sideEffects: false` designation in package.json).
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.
* master-sec:
Check that request is same-origin prior to including CSRF token in XHRs
HMAC raw CSRF token before masking it, so it cannot be used to reconstruct a per-form token
activesupport: Avoid Marshal.load on raw cache value in RedisCacheStore
activesupport: Avoid Marshal.load on raw cache value in MemCacheStore
Return self when calling #each, #each_pair, and #each_value instead of the raw @parameters hash
Include Content-Length in signature for ActiveStorage direct upload
Eugene Kenny