Hi guys,
I''m working on additions to what I submitted in ticket #3049
[http://dev.rubyonrails.org/ticket/3049]. Someone suggested that
redirect_to :back should allow for an alternate redirect if no referer
is set.
I''ve looked at the code for a while and allowing for:
redirect_to :back, :alternate => { OPTIONS HERE }
and such seems like it will produce fairly messy code in def redirect_to.
I do have a working implementation where you just place the alternate
redirect after the :back:
redirect_to :back, :action => ''alternative_action''
Does this seem acceptable? Is the :alternate syntax needed? Can
someone think of a clean way to allow for the :alternate syntax given
redirect_to''s prototype:
redirect_to(options = {}, *parameters_for_method_reference))
Kev
I''m bumping this and asking for someone (anyone really) to give me
some feedback on whether this would be acceptable syntax for allowing
for fallback with redirect_to :back.
redirect_to :back, :controller =>
''alternative_controller'',
:action =>''alt_action''
redirect_to :back,
''http://alternative.url.test.host/blah''
If this is something that shouldn''t be in the core at all, someone
atleast respond and tell me so. Regardless, I think the first half of
the patch (throwing a helpful exception is there is no request.env and
redirect_to :back is called) should be included.
I''ve added my latest diff to the ticket
[http://dev.rubyonrails.org/ticket/3049] and included docs.
Kev
On 12/4/05, Kevin Clark <kevin.clark@gmail.com>
wrote:> Hi guys,
> I''m working on additions to what I submitted in ticket #3049
> [http://dev.rubyonrails.org/ticket/3049]. Someone suggested that
> redirect_to :back should allow for an alternate redirect if no referer
> is set.
>
> I''ve looked at the code for a while and allowing for:
> redirect_to :back, :alternate => { OPTIONS HERE }
> and such seems like it will produce fairly messy code in def redirect_to.
>
> I do have a working implementation where you just place the alternate
> redirect after the :back:
> redirect_to :back, :action =>
''alternative_action''
>
> Does this seem acceptable? Is the :alternate syntax needed? Can
> someone think of a clean way to allow for the :alternate syntax given
> redirect_to''s prototype:
> redirect_to(options = {}, *parameters_for_method_reference))
>
> Kev
>
On Jan 8, 2006, at 8:10 PM, Kevin Clark wrote:> I''m bumping this and asking for someone (anyone really) to give me > some feedback on whether this would be acceptable syntax for allowing > for fallback with redirect_to :back. > redirect_to :back, :controller => ''alternative_controller'', > :action =>''alt_action'' > redirect_to :back, ''http://alternative.url.test.host/blah'' >I wonder if some other method would work better? redirect_to :back sounds so un-english. Suggestions: redirect_to :previous, :controller => ''alternative_controller'', :action => ''alt_action'' redirect_to :referring_url, :controller => ''alternative_controller'', :action => ''alt_action'' redirect_back_otherwise :controller => ... sound better to me. I''d really like to see this in the core. Duane Johnson (canadaduane) http://blog.inquirylabs.com/
> I wonder if some other method would work better? redirect_to :back > sounds so un-english. > > Suggestions: > redirect_to :previous, :controller => > ''alternative_controller'', :action => ''alt_action'' > redirect_to :referring_url, :controller => > ''alternative_controller'', :action => ''alt_action'' > redirect_back_otherwise :controller => ... > > sound better to me. I''d really like to see this in the core.redirect_to :back is already in the core, but its not a large change to make the keyword different. If others agree.
redirect_to :previous reads very well. On 1/8/06, Duane Johnson <duane.johnson@gmail.com> wrote:> > On Jan 8, 2006, at 8:10 PM, Kevin Clark wrote: > > > I''m bumping this and asking for someone (anyone really) to give me > > some feedback on whether this would be acceptable syntax for allowing > > for fallback with redirect_to :back. > > redirect_to :back, :controller => ''alternative_controller'', > > :action =>''alt_action'' > > redirect_to :back, ''http://alternative.url.test.host/blah'' > > > I wonder if some other method would work better? redirect_to :back > sounds so un-english. > > Suggestions: > redirect_to :previous, :controller => > ''alternative_controller'', :action => ''alt_action'' > redirect_to :referring_url, :controller => > ''alternative_controller'', :action => ''alt_action'' > redirect_back_otherwise :controller => ... > > sound better to me. I''d really like to see this in the core. > > Duane Johnson > (canadaduane) > http://blog.inquirylabs.com/ > > > _______________________________________________ > Rails-core mailing list > Rails-core@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails-core >-- Tobi http://jadedpixel.com - modern e-commerce software http://typo.leetsoft.com - Open source weblog engine http://blog.leetsoft.com - Technical weblog
> > I wonder if some other method would work better? redirect_to :back > > sounds so un-english.It makes more sense when you read the documentation: :back: Back to the page that issued the request. Useful for forms that are triggered from multiple places. Short-hand for redirect_to(request.env["HTTP_REFERER"]) The key part is that last sentence. :back is an alias, a symlink, a stand-in for request.env["HTTP_REFERER"]. I was even hesitant bringing that in because of the feature loops it leads to. I don''t think we should go further in core. I''d much welcome a plugin, though. For people who needs advanced redirecting to back. -- David Heinemeier Hansson http://www.loudthinking.com -- Broadcasting Brain http://www.basecamphq.com -- Online project management http://www.backpackit.com -- Personal information manager http://www.rubyonrails.com -- Web-application framework
On 1/8/06, David Heinemeier Hansson <david.heinemeier@gmail.com> wrote:> :back: Back to the page that issued the request. Useful for forms that > are triggered from multiple places. Short-hand for > redirect_to(request.env["HTTP_REFERER"])Looking at the code, it doesn''t appear to check that request.env["HTTP_REFERER"] isn''t also the current page, which could result in an infinite loop (I''ve triggered one before using request.env["HTTP_REFERER"] directly). Should that be considered a bug or just something to keep in mind when coding?
> Looking at the code, it doesn''t appear to check that > request.env["HTTP_REFERER"] isn''t also the current page, which could > result in an infinite loop (I''ve triggered one before using > request.env["HTTP_REFERER"] directly). Should that be considered a > bug or just something to keep in mind when coding?:back should be considered a 5-letter version of request.env["HTTP_REFERER"] -- nothing more. And it''s only there because request.env["HTTP_REFERER"] is hard to remember in the first place. Any additional logic, including safety features, should be delegated to a more advanced plugin for it. -- David Heinemeier Hansson http://www.loudthinking.com -- Broadcasting Brain http://www.basecamphq.com -- Online project management http://www.backpackit.com -- Personal information manager http://www.rubyonrails.com -- Web-application framework
> :back should be considered a 5-letter version of > request.env["HTTP_REFERER"] -- nothing more. And it''s only there > because request.env["HTTP_REFERER"] is hard to remember in the first > place. > > Any additional logic, including safety features, should be delegated > to a more advanced plugin for it. > -- > David Heinemeier HanssonI think it''s clear I disagree with David on this point. If :back exists then people who don''t know any better (and some that do) will use it which means mistakes will happen which cause their apps (and tests) to error out. I think that either: 1) :back should remain but should throw an exception on errors or 2) remove :back. Throwing an exception is not complicated (adds very little code) but can be a great help to those who would otherwise get a non-helpful error and then waste our time in IRC or the general mailing list. This is NOT advanced logic. Anyway, I''m working on a plugin to add the features that exist in my patch. Hope to have it out sometime this week. Kev
On Jan 9, 2006, at 2:18 PM, Kevin Clark wrote:>> :back should be considered a 5-letter version of >> request.env["HTTP_REFERER"] -- nothing more. And it''s only there >> because request.env["HTTP_REFERER"] is hard to remember in the first >> place. >> >> Any additional logic, including safety features, should be delegated >> to a more advanced plugin for it. >> -- >> David Heinemeier Hansson > > I think it''s clear I disagree with David on this point. If :back > exists then people who don''t know any better (and some that do) will > use it which means mistakes will happen which cause their apps (and > tests) to error out. > > I think that either: > 1) :back should remain but should throw an exception on errors > or > 2) remove :back. > Throwing an exception is not complicated (adds very little code) but > can be a great help to those who would otherwise get a non-helpful > error and then waste our time in IRC or the general mailing list. This > is NOT advanced logic. > > Anyway, I''m working on a plugin to add the features that exist in my > patch. Hope to have it out sometime this week. > KevI agree. Is it possible, David, that you kind of regret having put it in there? The way you talk about it sounds like that''s the case. If you use :back because you can''t remember request.env [''HTTP_REFERER''], and then find that your application redirects infinitely, you''re going to have to pull the ugly details out and fix it anyway. Doesn''t seem to really save anyone from pain except superficially. Duane Johnson (canadaduane) http://blog.inquirylabs.com/
> I agree. Is it possible, David, that you kind of regret having put > it in there? The way you talk about it sounds like that''s the case.The flip side of this, is that if we remove it, we break existing applications because on the off chance that an infinite redirect may occur. Also, there''s actually nothing wrong with redirecting to your current URL, consider an action like: def details if request.post? # do stuff redirect_to :action=>"details" else # something else end end not the prettiest thing, but certainly valid. -- Cheers Koz
On 1/9/06, Michael Koziarski <michael@koziarski.com> wrote:> The flip side of this, is that if we remove it, we break existing > applications because on the off chance that an infinite redirect may > occur.I agree that redirect_to :back shouldn''t be removed. Backwards compatability is good. I just really think the only responsible thing to do without going any further into logic that shouldn''t be in the core is to throw an exception when there is a problem. Is it really that big of a problem? Kev
> I agree that redirect_to :back shouldn''t be removed. Backwards > compatability is good. I just really think the only responsible thing > to do without going any further into logic that shouldn''t be in the > core is to throw an exception when there is a problem. > > Is it really that big of a problem?If you can reliably detect that we have a problem, then no it''s not a big deal at all. In fact it''s a great enhancement. But how are you going to do that? -- Cheers Koz
Sorry guys, reply sent this to just Koz instead of this list... stupid gmail. On 1/10/06, Kevin Clark <kevin.clark@gmail.com> wrote:> > If you can reliably detect that we have a problem, then no it''s not a > > big deal at all. In fact it''s a great enhancement. But how are you > > going to do that? > > Koz, > I''m not suggesting we solve the redirecting back to self issue; that > is a whole different can of worms. My intial patch simply added an > exception so if someone called redirect_to :back with a nil > request["HTTP_REFERER"] it would display a helpful error instead of > the other error (I don''t remember at this point, my code is patched) > which didn''t really say what the problem was. I agree with David that > anything more complex should be left out of core, but do like good > error messages. > > Kev >
> > I''m not suggesting we solve the redirecting back to self issue; that > > is a whole different can of worms. My intial patch simply added an > > exception so if someone called redirect_to :back with a nil > > request["HTTP_REFERER"] it would display a helpful error instead of > > the other error (I don''t remember at this point, my code is patched) > > which didn''t really say what the problem was. I agree with David that > > anything more complex should be left out of core, but do like good > > error messages.Sure, I''ll buy an exception patch to guard against nil. -- David Heinemeier Hansson http://www.loudthinking.com -- Broadcasting Brain http://www.basecamphq.com -- Online project management http://www.backpackit.com -- Personal information manager http://www.rubyonrails.com -- Web-application framework