Closed (fixed)
Project:
Webform
Version:
6.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
24 Apr 2008 at 16:00 UTC
Updated:
30 Apr 2012 at 19:10 UTC
Jump to comment: Most recent file
Comments
Comment #1
quicksketchThis is largely intentional that the form should go to the main node page once the form has been started. I can see the problems that we're getting into here by using arg() checking though. Perhaps a better way would be to check $node->is_teaser (or whatever that flag is), meaning that this redirect would only occur when viewing the teaser of a webform. What do you think?
Comment #2
meeotch commentedThis fix seems to work (although I haven't tested teasers, since I'm not using them - but "is_teaser" does seem to be the right flag, based on my searching)... However, it does seem to "eat" arguments, as noted in the dupe issue http://drupal.org/node/266939. That is:
http://sitename.com/competition/upload?sid=9
becomes
http://sitename.com/competition/upload
or, in my current case (tabbed panel)
http://sitename.com/competition/more#tabs--competition-4
becomes
http://sitename.com/competition/more
Apparently $_SERVER["REQUEST_URI"] is only set to the base URL, without the arguments. Does anyone know a method of getting the full URL with args, so I can stuff that into the #action?
Comment #3
quicksketchmeeotch, did you mean to post a patch with your last comment? Drupal.org seems to be eating them recently.
Comment #4
meeotch commentedSorry - I should have been more specific. I meant the fix you mentioned in #1 works, in terms of causing webform not to redirect in the non-teaser case. No patch posted.
(However, as mentioned above, the URL arguments seem to be stripped when reloading to show validation errors. For now, I've moved my webform to a page that doesn't require arguments to function.)
Comment #5
ariflukito commentedhi what is the status of this one
attached is what I'm using right now
Comment #6
sfa commentedSubscribed
Comment #7
marcmerz commentedquicksketch, would it be a problem to change line 1464 in webform.module (v 1.124.2.94) like this:
This way one could easily call the webform like:
and when there is an error the user gets back to the same page instead of the webform url path setting.
Comment #8
iNade commentedIs the patch in #5 working ?
Comment #9
iNade commentedComment #10
quicksketchThis has been fixed in #377420: Playing more nicely with argument rewrites.
Comment #11
teamA commentedThis is not a duplicate of that issue quicksketch. I applied your patch and it did nothing for this issue.
Comment #12
teamA commentedIn addition, I'm getting these failures when trying to apply the patch in #5.
Hunk #1 succeeded at 1129 (offset 203 lines).
Hunk #2 FAILED at 1372.
Hunk #3 succeeded at 1464 with fuzz 2 (offset 241 lines).
Hunk #4 succeeded at 1480 (offset 243 lines).
Hunk #5 FAILED at 1862.
So- I would say this is still active.
Comment #14
quicksketchI'm not sure I want to change the current behavior. Setting the #action does exactly what it is intended to do: redirect the user to the dedicated Webform page after they have started filling out the form.
As the code comment states, this is what it's intending to do:
If you're somehow embedding Webform in another node or in a block, you'll have to use hook_form_alter() and remove this change. Webform itself does not support embedding of Webforms on other pages.
Comment #15
varr commentedThe following worked for me.
FYI, using the #theme array seemed to be the easy, most straight forward way to verify if we're dealing with a webform (using the $form array). If you're aware of a better way reply and let us know.
Also, keep in mind that you can insert the status/error messages into the same panel pane that the webform is on using the panel UI, which can create a more cohesive experience for the user.
Enjoy.
Comment #16
ndf commentedNice solution. It works as a charm, thank you!
Comment #17
Eronarn commentedJust a comment on the above for anyone else who finds this with Google - $_GET['q'] didn't work for the way I have my particular Drupal install setup. For me, $_SERVER['REQUEST_URI'] was instead the correct choice.
Comment #18
mike-michal commentedWhat about this condition: if($form_id == 'example_form_id'){...}
???
Comment #19
tustind commentedIf you want to return your errors on the same page that the webform is embedded in through Panels, you don't need to hack anything.
Comment #20
manuel garcia commentedLet me reopen this please, and hear me out :)
For those confused with this issue, Changing the famous line 1534 in 6.x-3.x to
$form['#action'] = base_path() . $_GET['q'];would break the intended workflow of multistep webforms where the first step is taken in a teaser display.So the way around it is to embed the webform as a block, I'm I right?
In my opinion we are damaging our possibilities, just to save that workflow of coming from a teaser display... I really feel like we are hard coding the action for webforms in nearly every case.
Let me propose a solution, which is to only hardcode the form's #action if the webform is being displayed as a teaser, and let the fapi take care of the rest of cases, find the patch attached - what do you think?
Comment #21
screenage commentedFor solution #19:
Also make sure if you have a multilangual site, and you have made translations for your webform,
you set the "Available as block" option for each translated version, and you add each webform block to your panel with the necessary visibility rules for the language.
Comment #22
quicksketch@Manuel Garcia that sounds like a good change to me. However your patch and what you've described are two different things. You described:
But what's really happening in your patch is, "only hardcode the form's #action if the webform has been configured to display the full form in the teaser". This means that the hard-coded #action would still take place if the user had checked the box for "Show complete form in teaser", even if they were showing the full version of the node. We need to check $node->is_teaser or $node->display_mode (in D7). I'm actually not sure what these variables are called, they may not be passed into webform_client_form() at all right now.
Comment #23
manuel garcia commented@quicksketch, you're right the variables are not available in the $node object right now, there's no way to check wether the node is being displayed as a teaster... can you think of a way to find that out?
Comment #24
quicksketchSomething else to consider here also: #1033090: Multiple step form - first in block the rest in node.
You could insert into $node->webform['is_teaser'] during hook_nodeapi($op = 'view') before the call to drupal_get_form('webform_client_form').
Comment #25
quicksketchThe changes committed in #1033090: Multiple step form - first in block the rest in node should make this patch a little easier to implement now. There is a single variable for $pages_in_node that currently defaults to TRUE. Once we get the teaser information into the form rendering process, we can set $pages_in_node = $node->webform['is_teaser'] by default instead of just the straight-up TRUE we're using now.
Comment #26
doublejosh commentedDirectly related?
When using Form Block (upon an error, or multi-step forms) the /action keeps the user on the same page the form was on. Can imagine this *might* be the intended behavior sometimes, but in all my cases I'd want the user to proceed to the webform node to finish things up. Hence thinking this is the right thread.
Perhaps there should be an admin setting for what /action to take when the form is begun anywhere other than the webform full node page? This would cover both teaser situations and Form Block uses.
Comment #27
manuel garcia commented@quicksketch those changes indeed make this patch a lot simpler
Find attached what you suggest as a patch, as far as I've tested it's working perfectly =)
Comment #28
quicksketch@Manuel Garcia: Your patch will only work for D6. Could you see what's needed in D7?
Comment #29
manuel garcia commentedSure thing, here it is. Pretty much the same thing.
Comment #30
quicksketchThe D7 patch uses is_page instead of is_teaser, looks like the two patches would have opposite effects. :\
Anyway I really just need to test them out and see what the overall effect is.
Comment #31
manuel garcia commentedErm... you're totally right... I'll just blame this on today being Monday if that's ok.
The attached patch actually makes sense now for d7.
Comment #32
quicksketchHa, thanks. In D7, you'll have to use $view_mode to determine if it's a teaser. Since there isn't a $teaser (or $page) variable in D7's hook_node_view().
Comment #33
manuel garcia commentedYup, but we're already doing that and setting up a $teaser variable earlier inside hook_node_view:
Comment #34
quicksketchUsing @Manuel Garcia's patch as inspiration, I've made this patch which removes loading of the block settings from the webform_client_form() function. Instead of adding an odd property called
$node->webform['is_teaser']that affects the #action property, this patch takes a direct approach of just letting code set$node->webform['action']to any arbitrary value. Then node_view() sets this property for teasers and webform_block_view() sets it for blocks. The approach is both direct and allows extra flexibility for other modules extending Webform.How does this look to you guys?
Comment #35
quicksketchUpdated version that only sets the $action property for teasers and not for full nodes.
Comment #36
quicksketchTested this out again today and it all works well. Ported to D6 in this patch and committed patches to both branches.
Comment #37
manuel garcia commentedAwesome, thanks quicksketch.
Sorry I never got around to testing the latest patch!