Ticket #592 (closed bug: fixed)

Opened 6 months ago

Last modified 6 months ago

form plugin create always a form with method post

Reported by: laurentj Assigned to: Julien
Priority: normal Milestone: Jelix 1.0.4
Component: jelix:forms Version: 1.0.3
Severity: normal Keywords:
Cc: Php version:
Review: review+ Hosting Provider:
Documentation needed: 0 Blocking:

Description

When we indicates 'get' as a parameter of the plugin, the generated "method" attribute has the "post" value.

Attachments

592-jForms-GET-method.diff (1.1 kB) - added by Julien on 05/22/08 13:22:11.
592-jForms-GET-method.2.diff (2.3 kB) - added by Julien on 05/22/08 14:01:58.
good patch
592-jForms-GET-method.3.diff (3.0 kB) - added by Julien on 05/22/08 14:17:59.
the third one is always the good one ;)

Change History

05/21/08 16:53:30 changed by Julien

I took a look at this problem, and the explanation is that when you write something like :

{form $form, 'myaction', array(), "", "", 'get'}

then you get :

echo $params[5];
// will print "'get'" (notice single quotes) instead of just "get"

so the following code :

if($method!='get' && $method!='post')
    $method='post';

will always set the method to POST

05/21/08 17:20:32 changed by Julien

So the fix is quite easy, but I'm wondering if we shouldn't do a global treatment instead, for all jTpl plugins' arguments.

I mean strippring starting and ending quotes in args by doing something like :

foreach($args as $k=>$arg){
   $arg = preg_replace('#^\'([\']*)\'$#','$1',$arg);
   $args[$k] = preg_replace('#^"(["]*)"$#','$1',$arg);
}

I don't really know where to do this (I think in jTpl main class), I don't know if this is a good idea, I don't know if regexp replacing is the best way, I don't know if this will broke something somewhere.

Back to form plugin, if we do such a change, we would write :

if(isset($params[3]) && $params[3] !== ''){

instead of

if(isset($params[3]) && $params[3] != "''" && $params[3] != '""'){

if we don't want/need such a change, the fix for this ticket would be :

$method = strtolower(isset($params[5])?str_replace(array('\'','"'),array('',''),$params[5]):'post');

instead of

$method = strtolower(isset($params[5])?$params[5]:'post');

In fact, it could rather be improved that way :

$method = isset($params[5])?strtolower(str_replace(array('\'','"'),array('',''),$params[5])):'post';

Laurent, would you like me to patch the needed files "the easy and fast way" or should I look for arguments escaping in jTpl ?

05/21/08 21:56:23 changed by laurentj

  • owner set to laurentj.
  • status changed from new to assigned.

No, you cannot doing escape or whatever. Imagine for example I write :

{form $form, 'myaction', array(), "", "", $method}

;-)

So the easier solution is to generate a PHP test (the form plugin is a bloc, so it generate php code to include in the compiled template).

I take this bug.

05/22/08 13:22:11 changed by Julien

  • attachment 592-jForms-GET-method.diff added.

05/22/08 13:26:50 changed by Julien

I saw you've taken this bug too late, as I already wrote the simple patch yesterday.

I don't know if this solves the problem the way you want it, so I post it "just in case".

NB : about stripping starting/ending (double)quotes in args, this seems quite complicated, as I took a look at jTpl code and this will need a lot of changes in various files that test arguments values. So I forget about that idea ;)

05/22/08 13:32:01 changed by Julien

forget about my previous post and patch, I just noticed you last exemple :

{form $form, 'myaction', array(), "", "", $method}

and I see this is another case causing the bug....

05/22/08 14:01:58 changed by Julien

  • attachment 592-jForms-GET-method.2.diff added.

good patch

05/22/08 14:17:59 changed by Julien

  • attachment 592-jForms-GET-method.3.diff added.

the third one is always the good one ;)

05/22/08 14:20:30 changed by Julien

  • review set to review?.

Ok, this one is good.

It accepts literal or jTpl var as method arg.

If the method is set to anything but 'get', then the method is set to 'post'.

05/27/08 00:23:23 changed by laurentj

  • owner changed from laurentj to Julien.
  • status changed from assigned to new.
  • review changed from review? to review+.

Thanks for the patch.

$method = isset($param[5])?strtolower($param[5]):'\'post\;

Don't do a strtolower !!

r+ (but remove the call of strtolower before to commit the patch)

05/27/08 01:03:27 changed by Julien

Hello,

strtolower was there before, and I still need it, because of lib/jelix/plugins/jforms/html/html.jformsbuilder.php :

$method = ($params[2]=='get')?'get':'post';

if the user sets the method to 'GET' (in uppercase, thus not 'get'), then the method will be post, because comparison is case sensitive, right ?

Maybe I can remove the strtolower call in form and formfull, and apply it only in jformsbuilder ? Like this :

lib/jelix/plugins/jforms/html/html.jformsbuilder.php

$method = (strtolower($params[2])=='get')?'get':'post';

05/27/08 15:36:50 changed by laurentj

Imagine you write this :

 {form $form, 'myaction', array(), "", "", $meThoD}
or
 {form $form, 'myaction', array(), "", "", $foo['BaR']}

Then, your code is equivalent to

$method = isset($params[5])?strtolower('$foo[\'BaR\']'):'\'post\''; 

so the parameter become $foobar?, which is not equivalent to the given parameter.

Transformation should not be done during the code generation, but in the generated code or in the builder.

05/27/08 16:41:26 changed by Julien

Oh yes, you're absolutely right.

I'm moving the strtolower call in the builder right now.

05/27/08 17:31:19 changed by Julien

  • status changed from new to closed.
  • resolution set to fixed.

Commit done in the trunk and in branch 1.0.x

Download in other formats: Comma-delimited Text Tab-delimited Text RSS Feed