Skip to content

I18n locale fallbacks for localized views don't work for mailers #11884

Closed
benedikt opened this Issue Aug 14, 2013 · 21 comments
@benedikt

Pull request #7368 recently fixed issues #3512 and #840 related to fallbacks for localized views for views rendered by controllers.

However the locale fallbacks are still broken for mailer views.

Having the locale set to de-at, and a file called app/views/mailer/demo.de.erb, trying to build the mail results in an ActionView::MissingTemplate exception:

ActionView::MissingTemplate: Missing template mailer/demo with {:locale=>[:"de-at"], :formats=>[:text], :handlers=>[:erb, :builder, :coffee]}. Searched in:
  * "/Users/benedikt/demo/app/views"

The issue is related to ActionView::LookupContext#skip_default_locale!, which is only called from within ActionMailer::Base:

I18n.locale = 'de-at'
context = Mailer(:new).lookup_context
context.find_all "mailer/demo" # => [app/views/mailer/demo.de.markerb]
context.skip_default_locale!
context.find_all "mailer/demo" # => []

However: The issue is not related to the value I18n.default_locale is set to, changing this to something else (even nonsense), doesn't affect this issue.

@benedikt

The ActionView::LookupContext#skip_default_locale! method was introduced to fix this test: https://github.com/rails/rails/blob/271d622a4ddc35c21bc8b853e4e2901a9ede43bd/actionmailer/test/base_test.rb#L279:L289

Reading it leaves me wondering whether this described behavior is actually the desired one. Should the implicit_with_locale.html.erb file really take precedence over the implicit_with_locale.en.html.erb file?

@alekseyg

I'm having this issue as well on a multilingual web app. Are there any workarounds for now?

@benedikt

This monkey patch solved the issue for me. I haven't noticed any issues related to the missing skip_default_locale! functionality, yet.

# config/initializers/lookup_context.rb
class ActionView::LookupContext
  def skip_default_locale!
  end
end
@neckhair
neckhair commented Nov 7, 2013

+1

@jmuheim
jmuheim commented Nov 7, 2013

+1

@zinkkrysty

+1

@robin850 robin850 added actionview and removed i18n labels Feb 24, 2014
@cerebroso

+1

@robin850 robin850 added this to the 4.0.6 milestone May 3, 2014
@robin850
Ruby on Rails member
robin850 commented May 3, 2014

@carlosantoniodasilva : I took the liberty to assign you to this one as you mentioned on Basecamp ; I tried to fix this issue but without any luck, here is my stash hoping it can help you!

@benedikt
benedikt commented May 3, 2014

@robin850 @carlosantoniodasilva Removing ActionView::LookupContext#skip_default_locale! would fix this issue entirely. Looking at the commit logs, it was introduced to fix a failing test, which might describe undesired behavior (See my first comment above).

@robin850
Ruby on Rails member
robin850 commented May 3, 2014

@benedikt : I tried too but if we remove this line, this assertion fails. 😢 But you are right ; the mentioned commit is 3fc609e.

@benedikt
benedikt commented May 3, 2014

@robin850 Yes. That's the test that fails… but I think it doesn't make sense in the first place. However, I might be wrong about this, because I don't know anything about the intention it was written with :)

@ereslibre

Also having this issue. @benedikt's monkey patch works perfectly. The commit where this method was introduced is: 3fc609e. Didn't run the tests, but there are chances that from this commit on fallbacks stopped working since it was called on process method at ActionMailer::Base back then too.

@rails-bot rails-bot added the stale label Aug 19, 2014
@rails-bot

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@rafaelfranca rafaelfranca modified the milestone: 4.0.10, 4.1.7 Aug 21, 2014
@benedikt

To me it looks like the only way to move forward on this issue, is to decide whether the failing assertion from 3fc609e makes sense or not. I'm saying it does not.

How can we move forward on this? I'm happy to provide a pull request, that fixes the issue and removes the assertion.

@carlosantoniodasilva: As you're assigned for this issue, what's your opinion on this?

@rafaelfranca rafaelfranca removed the stale label Sep 10, 2014
@rafaelfranca rafaelfranca modified the milestone: 4.1.9, 4.0.13 Nov 19, 2014
@tasaif
tasaif commented Nov 24, 2014

+1

@robinboening

I also don't see any reason why skip_default_locale! is getting called or why its even there.

@benedikt's patch helped out for now, thanks.

Btw: The gem-patching gem is a good choice if you want to remember your patch later: https://github.com/ingoweiss/gem_patching

# https://github.com/rails/rails/issues/11884
Gem.patching('actionview', '4.1.8') do
  class ActionView::LookupContext
    def skip_default_locale!
    end
  end
end
@rafaelfranca rafaelfranca removed this from the 4.0.13 milestone Dec 30, 2014
@rafaelfranca
Ruby on Rails member

I fixed this issue removing skip_default_locale! logic from Action Mailer so it should work like Action View now, but, give it is a breaking change it will be present only at Rails 5.

Thank you for reporting.

@rafaelfranca rafaelfranca added a commit that closed this issue Dec 30, 2014
@rafaelfranca rafaelfranca Template lookup now respect default locale and I18n fallbacks.
Given the following templates:

    mailer/demo.html.erb
    mailer/demo.en.html.erb
    mailer/demo.pt.html.erb

Before this change for a locale that doesn't have its related file
the `mailer/demo.html.erb` will
be rendered even if `en` is the default locale.

Now `mailer/demo.en.html.erb` has precedence over the file without
 locale.

Also, it is possible to give a fallback.

    mailer/demo.pt.html.erb
    mailer/demo.pt-BR.html.erb

So if the locale is `pt-PT`, `mailer/demo.pt.html.erb` will be
 rendered given the right I18n fallback configuration.

Fixes #11884.
ecb1981
@benedikt

Thanks a lot for taking care of this, @rafaelfranca! 👍

@robinboening

💃 Thank you for fixing @rafaelfranca. And @benedikt thank you for reporting. 💃

@ccmcbeck
ccmcbeck commented Mar 5, 2016

@robinboening, thanks for the tip on gem-patching. There is a newer version at this link https://github.com/ingoweiss/gem-patching (notice the dash vs underscore) where the author responded to ingoweiss/gem_patching#2. The new version supports Gem::Requirement restriction operators so you can do this:

Gem.patching('actionview', '< 5') do
  class ActionView::LookupContext
    def skip_default_locale!
    end
  end
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.