developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 12 years ago

Closed 12 years ago

#592 closed bug (fixed)

form plugin create always a form with method post

Reported by: laurentj Owned by: Julien
Priority: normal Milestone: Jelix 1.0.4
Component: jelix:forms Version: 1.0.3
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

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

Attachments (3)

592-jForms-GET-method.diff (1.1 KB) - added by Julien 12 years ago.
592-jForms-GET-method.2.diff (2.3 KB) - added by Julien 12 years ago.
good patch
592-jForms-GET-method.3.diff (3.0 KB) - added by Julien 12 years ago.
the third one is always the good one ;)

Download all attachments as: .zip

Change History (14)

comment:1 Changed 12 years ago 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

comment:2 Changed 12 years ago 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 ?

comment:3 Changed 12 years ago 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.

Changed 12 years ago by Julien

comment:4 Changed 12 years ago 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 ;)

comment:5 Changed 12 years ago 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....

Changed 12 years ago by Julien

good patch

Changed 12 years ago by Julien

the third one is always the good one ;)

comment:6 Changed 12 years ago 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'.

comment:7 Changed 12 years ago by laurentj

  • Owner changed from laurentj to Julien
  • review changed from review? to review+
  • Status changed from assigned to new

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)

comment:8 Changed 12 years ago 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';

comment:9 Changed 12 years ago 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.

comment:10 Changed 12 years ago by Julien

Oh yes, you're absolutely right.

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

comment:11 Changed 12 years ago by Julien

  • Resolution set to fixed
  • Status changed from new to closed

Commit done in the trunk and in branch 1.0.x

Note: See TracTickets for help on using tickets.