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

Last modified 12 years ago

#710 closed enhancement (fixed)

support external absolute url as jform action

Reported by: bibo Owned by: Julien
Priority: normal Milestone: jelix 1.1
Component: jelix:forms Version: trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

If someone wants to post data to a web service, it would be cool to still do it through jforms. At now, jforms throws an exception if you give an absolute url as jform action.

Note that, although it will be cool, you should take care of cleaning jform session data when the service pings you back. Otherwise, you'll pile up unused session data...

Attachments (6)

#710-trunk-jforms-externalurl.patch (4.8 KB) - added by bibo 12 years ago.
like this ?
#710-trunk-jforms-externalurl.2.patch (6.4 KB) - added by bibo 12 years ago.
fixes warnings
#710-trunk-jforms-externalurl.3.patch (6.9 KB) - added by bibo 12 years ago.
another try
#710-trunk-jforms-externalurl.4.patch (5.4 KB) - added by bibo 12 years ago.
patch updated to tip
#710-trunk-jforms-externalurl.5.patch (5.2 KB) - added by bibo 12 years ago.
simpler
710-jForms-support-external-absolute-url-as-action.diff (13.8 KB) - added by Julien 12 years ago.
support of https and unit tests

Download all attachments as: .zip

Change History (24)

Changed 12 years ago by bibo

like this ?

comment:1 Changed 12 years ago by bibo

  • Owner set to bibo
  • review set to review?
  • Status changed from new to assigned

Changed 12 years ago by bibo

fixes warnings

comment:2 Changed 12 years ago by laurentj

+            if (count($this->_actionParams)>0)
+                $url = jUrl::appendToUrlString($this->_action, $this->_actionParams, true);
+            echo '<form action="',$url,'" method="'.$this->options['method'].'" id="', $this->_name,'"';

I think that, in the case of method="get", _actionParams value should be inserted into <input type="hidden">.

comment:3 Changed 12 years ago by bibo

You mean method="*post*", I presume, no ?

comment:4 Changed 12 years ago by laurentj

non, "get". In a form, the parameters of the url should be into fields.

Changed 12 years ago by bibo

another try

Changed 12 years ago by bibo

patch updated to tip

comment:5 Changed 12 years ago by laurentj

  • Milestone set to jelix 1.1
  • review changed from review? to review-
+        if( ($externalUrl && count($this->_actionParams)>0) )
+           $params = $this->_actionParams;
+        else
+           $params = $url->params;

In the case of external url, $url is a string, not an object. So if we give an external url without parameters, we enter in the second part of the condition, and we will have an error. I think that you should remove the test about the count.

+        if ($externalUrl = preg_match('/^http:\/\//',$this->_action)) {
+            $url = $this->_action;

And then I think the assignation of $url is useless here. I think also that the assignation of the $params could be done in this if statement, and not in a separate if statement.

+        foreach (params as $p_name => $p_value) {

a $ is missing

Same comments for htmllight.jformsbuilder.php + you forgot to change the foreach($url->params..)

Changed 12 years ago by bibo

simpler

comment:6 Changed 12 years ago by bibo

  • review changed from review- to review?

and perhaps correct :)

comment:7 Changed 12 years ago by laurentj

  • review changed from review? to review+

It's ok :-)

comment:8 Changed 12 years ago by bibo

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

comment:9 Changed 12 years ago by Julien

Hello,

shouldn't we support httpS too ?

so we could write this :

if (preg_match('#^https?://#',$this->_action)) { 

comment:10 Changed 12 years ago by laurentj

Yes, I agree.

comment:11 Changed 12 years ago by Julien

I'm doing it right now + unit tests

comment:12 Changed 12 years ago by Julien

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:13 Changed 12 years ago by Julien

  • Owner changed from bibo to Julien
  • review review+ deleted
  • Status changed from reopened to new

Changed 12 years ago by Julien

support of https and unit tests

comment:14 Changed 12 years ago by Julien

  • review set to review?
  • Status changed from new to assigned

comment:15 Changed 12 years ago by laurentj

  • review changed from review? to review+

it's ok, thank you :-)

comment:16 Changed 12 years ago by Julien

dammit ! wrong commit message ! switched 2 tickets in my mind ! (any way to change this ?) sorry for that.

commit done in the trunk, svn r1176

comment:17 Changed 12 years ago by Julien

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

comment:18 Changed 12 years ago by laurentj

I changed the log message :-)

Note: See TracTickets for help on using tickets.