This project is archived and is in readonly mode.

#1974 ✓resolved
Sven Fuchs

Layout picked from wrong view_paths?

Reported by Sven Fuchs | February 14th, 2009 @ 08:56 PM

Still not too familiar with ActionPack's internals, so I'm not entirely sure on this but ...

In 2.2.x pick_layout returned a String like "layouts/default" which then would be resolved using the current view's view_paths (which could be changed on a per-request basis).

Now pick_layout already returns a Template instance which is returned untouched from view_paths' find_template. (I guess this is because in 2.3 view_paths are now instantiated as EagerPaths by default?)

Thus, when a new view_path has been added to the current view's view_paths pick_layout will ignore this and instead pick a template based on the controller class' view_paths.

The attached patch might point out a workaround for this problem but obviously this is not a good solution. I wonder why pick_layout/active_layout do not use the current view's view_path anyway.

Comments and changes to this ticket

  • Pratik

    Pratik February 14th, 2009 @ 09:08 PM

    • Assigned user set to “josh”
  • Sven Fuchs

    Sven Fuchs February 14th, 2009 @ 09:14 PM

    Ok, obviously that workaround would need to take into account that there might not be a layout present (changed that). Anyway, it's only there for pointing into the right direction ;)

  • Repository

    Repository February 15th, 2009 @ 12:27 AM

    • State changed from “new” to “resolved”

    (from [0cb020b4d6d838025859bd60fb8151c8e21b8e84]) workaround for picking layouts based on wrong view_paths [#1974 state:resolved]

    Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...

  • Sven Fuchs
  • Sven Fuchs

    Sven Fuchs February 19th, 2009 @ 05:32 PM

    Actually this did not really resolve the issue. It obviously only works for layouts that are already present in the class' view_path - they will then be loaded from the location given in the instance's view_path.

    When the layout is not present in the class' view_path an exception is raised even though the layout is present in the instance's view_path.

    Is there any reason not to use the instance's view_path for looking up the layout? I might have a look into a patch.

  • thedarkone

    thedarkone February 19th, 2009 @ 07:57 PM

    Sven, class-level view_paths are probably used due to how inheritable layouts are implemented.

  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 10:57 AM

    thedarkone,

    unless i'm mistaken that only affects the "layout name" but not necessarily the selection of an actual template (which now happens because of the recent introduction of EagerPath).

    I'll have a closer look into this and try to come up with another patch.

  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 02:25 PM

    • State changed from “resolved” to “open”

    Ok, patch is attached. I'll reopen the ticket.

  • josh

    josh February 20th, 2009 @ 05:04 PM

    Awesome, it looks a bit cleaner now.

    I think some rjs related layout fix got applied after you created this patch. Could you please rebase yours with master and I'll give it another shot.

  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 06:35 PM

    Whooops. I thought I would have patched against master but apparently I was on v2.3.0

    Hopefully I've got it right now. New patch is attached.

  • Sven Fuchs

    Sven Fuchs February 20th, 2009 @ 06:38 PM

    Hmmm, i just notice that there's one failing test in the actionmailer tests. Doesn't seem to be related to this patch though. Is that some known bug?

  • Repository

    Repository February 20th, 2009 @ 08:20 PM

    • State changed from “open” to “resolved”

    (from [8c5cc66a831aadb159f3daaffa4208064c30af0e]) make action_controller/layouts pick templates from the current instance's view_paths instead of the class view_paths [#1974 state:resolved]

    Signed-off-by: Joshua Peek josh@joshpeek.com http://github.com/rails/rails/co...

  • Andrew White

    Andrew White February 21st, 2009 @ 04:23 AM

    Josh, you can close #1919 as well since this ticket is essentially the same as mine. You may want to add the extra tests from my patch in light of the regressions we've been having.

    One thing to note is that automatic layout assignment doesn't work with per-request view paths with either this patch or my patch. The auto_layout variable is set in the alias chained inherited method and searches the default view path so any automatic layouts that only exist in the per-request view path are not detected.

    I'll investigate further to see if I can get it to work.

  • Repository

    Repository April 13th, 2009 @ 11:58 PM

    (from [906aebceedb95d8caa6db6314bc90f605bdfaf2b]) Bring abstract_controller up to date with rails/master

    Resolved all the conflicts since 2.3.0 -> HEAD. Following is a list of commits that could not be applied cleanly or are obviated with the abstract_controller refactor. They all need to be revisited to ensure that fixes made in 2.3 do not reappear in 3.0:

    2259ecf368e6a6715966f69216e3ee86bf1a82a7 AR not available * This will be reimplemented with ActionORM or equivalent

    06182ea02e92afad579998aa80144588e8865ac3 implicitly rendering a js response should not use the default layout [#1844 state:resolved] * This will be handled generically

    893e9eb99504705419ad6edac14d00e71cef5f12 Improve view rendering performance in development mode and reinstate template recompiling in production [#1909 state:resolved] * We will need to reimplement rails-dev-boost on top of the refactor;

    the changes here are very implementation specific and cannot be
    cleanly applied. The following commits are implicated:
    
      199e750d46c04970b5e7684998d09405648ecbd4
      3942cb406e1d5db0ac00e03153809cc8dc4cc4db
      f8ea9f85d4f1e3e6f3b5d895bef6b013aa4b0690
      e3b166aab37ddc2fbab030b146eb61713b91bf55
      ae9f258e03c9fd5088da12c1c6cd216cc89a01f7
      44423126c6f6133a1d9cf1d0832b527e8711d40f
    
    

    0cb020b4d6d838025859bd60fb8151c8e21b8e84 workaround for picking layouts based on wrong view_paths [#1974 state:resolved] * The specifics of this commit no longer apply. Since it is a two-line

    commit, we will reimplement this change.
    
    

    8c5cc66a831aadb159f3daaffa4208064c30af0e make action_controller/layouts pick templates from the current instance's view_paths instead of the class view_paths [#1974 state:resolved] * This does not apply at all. It should be trivial to apply the feature

    to the reimplemented ActionController::Base.
    
    

    87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallback for explicit templates [#2052 state:resolved] * There were a number of patches related to this that simply compounded

    each other. Basically none of them apply cleanly, and the underlying
    issue needs to be revisited. After discussing the underlying problem
    with Koz, we will defer these fixes for further discussion.
    
    

    http://github.com/rails/rails/co...

  • Repository

    Repository April 13th, 2009 @ 11:58 PM

    (from [906aebceedb95d8caa6db6314bc90f605bdfaf2b]) Bring abstract_controller up to date with rails/master

    Resolved all the conflicts since 2.3.0 -> HEAD. Following is a list of commits that could not be applied cleanly or are obviated with the abstract_controller refactor. They all need to be revisited to ensure that fixes made in 2.3 do not reappear in 3.0:

    2259ecf368e6a6715966f69216e3ee86bf1a82a7 AR not available * This will be reimplemented with ActionORM or equivalent

    06182ea02e92afad579998aa80144588e8865ac3 implicitly rendering a js response should not use the default layout [#1844 state:resolved] * This will be handled generically

    893e9eb99504705419ad6edac14d00e71cef5f12 Improve view rendering performance in development mode and reinstate template recompiling in production [#1909 state:resolved] * We will need to reimplement rails-dev-boost on top of the refactor;

    the changes here are very implementation specific and cannot be
    cleanly applied. The following commits are implicated:
    
      199e750d46c04970b5e7684998d09405648ecbd4
      3942cb406e1d5db0ac00e03153809cc8dc4cc4db
      f8ea9f85d4f1e3e6f3b5d895bef6b013aa4b0690
      e3b166aab37ddc2fbab030b146eb61713b91bf55
      ae9f258e03c9fd5088da12c1c6cd216cc89a01f7
      44423126c6f6133a1d9cf1d0832b527e8711d40f
    
    

    0cb020b4d6d838025859bd60fb8151c8e21b8e84 workaround for picking layouts based on wrong view_paths [#1974 state:resolved] * The specifics of this commit no longer apply. Since it is a two-line

    commit, we will reimplement this change.
    
    

    8c5cc66a831aadb159f3daaffa4208064c30af0e make action_controller/layouts pick templates from the current instance's view_paths instead of the class view_paths [#1974 state:resolved] * This does not apply at all. It should be trivial to apply the feature

    to the reimplemented ActionController::Base.
    
    

    87e8b162463f13bd50d27398f020769460a770e3 fix HTML fallback for explicit templates [#2052 state:resolved] * There were a number of patches related to this that simply compounded

    each other. Basically none of them apply cleanly, and the underlying
    issue needs to be revisited. After discussing the underlying problem
    with Koz, we will defer these fixes for further discussion.
    
    

    http://github.com/rails/rails/co...

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Referenced by

Pages