Forum OpenACS Development: ad_form and ad_page

1: ad_form and ad_page_contract integration

Posted by Tilmann Singer on 10/18/02 05:13 PM

When building pages with ad_form, both ad_form and ad_page_contract do input variable validation. It might be benefitial when the validation and thus parts of ad_form were part of ad_page_contract.

Here's an idea: Add an optional declaration block to ad_page_contract, called -form, that contains all widget declarations that the form will be built of, which were previously declared in ad_form. The same filters as for usual ad_page_contract variables can be applied, e.g. integer, trim, html, etc.

The main difference will be that upon initial request the existance of those variables will not be checked - only when the request is a submitted form.

The ability to specify the key variable in ad_page_contract needs to be added, either in the vars block or in -form. Example:

ad_page_contract {
   add or edit a note
} {
  note_id:key
} -form {
    { title:trim
        {html { size 20 }}
        {label "Title"}}

    { body:text(textarea),trim
        {label "Body"}
        {html { rows 10 cols 40 wrap soft }}}

}

This way ad_page_contract would already know if the form is a new or edit request, and can supply that knowledge by setting a variable like __new_p that will be defined for every request when a -form block is present, not only upon invalid form redisplays. No more need to add kludges like ad_form_new_p.

I don't know how dynamic form building with ad_form -extend would fit in here. Dynamically generated variables obviously can't be part of ad_page_contract (assuming no one wants to put code before it, which would be ugly).

To avoid clutter in the -form block there should be the possibility to refine the widgets declaration later in the page, and leave stuff like {html ...} completely out of ad_page_contract.

Also it would be nice if form element errors that will be displayed inline could be manually set in the code, e.g. in the -validate clause of ad_page_contract, and if there was the possibility to generate generic validation messages that are not associated to a specific element but still would be displayed inline, instead of the current "Please press back and correct ..." message.

This is a terribly chaotic pile of thoughts, but hopefully enough to start a discussion.

2: Response to ad_form and ad_page_contract integration (response to 1)

Posted by Don Baccus on 10/18/02 07:31 PM

I'd personally prefer extending ad_form to allow the use of ad_page_contract filters on submitted form variables. This would be relatively simple to do. I caught a bit of the conversation on IRC (lurking) and I think this is a reasonable thing to do. They'd be applied after form validation.

I guess it is worth asking if things like "trim" should be added to the acs-templating form handling code itself or not. ad_form currently is meant to be a wrapper around that, though it does do one important thing not found in acs-templating (automated key generation that defaults to the acs_objects sequence).

When I first thought about writing an improved form handling utility I started with the assumption that extending ad_page_contract would be the thing to do. The problem I ran into was that when considering all the different cases that need to be handled things rapidly become very complicated.

Consider the relatively trivial case of validation blocks. One could add the necessary bookkeeping to track whether or not the item being validated should result in an error embedded in the form or an error page ala ad_page_contract. Not the most complex thing in the world but ad_page_contract and ad_form are already fairly complex and I'm not eager to add to it. We could implement two types of validation blocks and simplify the megaproc but would this be any more clear, really, than two separate calls?

Also there is value in allowing dynamic code before building the form. This was what really led me to implement ad_form as a separate proc rather than extend ad_page_contract.

Having code before the ad_page_contract code would be worse than ugly - one of the reasons for ad_page_contract is to encourage people to write informative comment blocks that show up at the very top of the script, not buried after some initial code has been executed.

We'd still need ad_form to allow for extension of an already defined form, which is necessary. Check out the way I use Tcl library procs to build up common form elements in gp-multimedia, for instance. Think about building forms with a variable number of elements. One could build the form block dynamically then pass it into ad_form (or ad_page_contract) but using "-extend" is much simpler. I know because that's what I did when writing gp-multimedia before adding "-extend".

I personally dislike the idea of allowing the declaration of stuff like "html" separate from the form block defining the form element because I like having the form defined in one place. I can understand a form that's declared in place, I'm not sure I can understand one that's defined helter-skelter in pieces scattered throughout a script.

The form handling portion of acs-templating only gives you one way to define inline messages not associated with a visible form element - you can set an error on a hidden form element and I believe that error will be displayed in the form (haven't tested it, though).

But the paradigm I'd like to encourage for coordinating use of ad_page_contract and ad_form would be to only handle queryvars in ad_page_contract that are controlled by the programmer rather than entered by the user. In this case, then, failed validation should only be caused by a bug in the code or by someone spoofing form data by generating their own POST requests to the site. In either case I think an error page is OK ...

These are my thoughts, at least.

3: Response to ad_form and ad_page_contract integration (response to 1)

Posted by defunct defunct on 10/19/02 10:57 AM

Just a quick mention regarding frames.

although I'm sure we all share a reasonably common dislike for
them, is it worth considering making the form stuff a little more
frame friendly?

Ok, bascially if you have a site where you use <iframe> quite a
bit (funnily enough I'm doing one just now) then its pretty
common to want to set the <base target="_top" directive in the
header. i.e. all ways push to the top level whenever a target isn't
directly specified.

However the net effect of this is that if you hit 'submit' on a forms
api based page, that then fails validation, it will then jump
outside of its original containing frame, which is almost never
the situation you'd like.

I'd suggest it makes more sense to have the forms stuff assume
_self as the default target, as the api itself is intended to throw
back the entry-error page in exactly the same format as the
original.

Possibly there's something already in the api to cater for this,
and I'm not aware of it, but if not does this seem a sensible
suggestion? Its not going to have any negative affect when not
using frames (I don't think) but when you are it will exhibit what
I'd argue if the exepected behaviour.

Make sense?

Just while I'm on the topic, the whole acs is a little frame-
unfriendly. or example I'm having to put the /pvt/home page in an
inline frame for this current project. Of course by doing so
means that by implication I'd like all the related pages (pvt/
home-user, /register/* /user/* pages in acs-subsite) to remain
within the same frame. At least I would argue this is the best
default behaviour. Its a real nuisance to have to through all these
pages and change the links to ensure proper frame functioning,
where (if I'm right) assuming the _self target for these pages
would get around the problem?

Thought appreciated. If you're happy with this I don't mind doing
the work (as I've raised it) but I wanted to check I'm not forgetting
something about frames, or possible nasty side effects.

Forum OpenACS Development: ad_form and ad_page_contract integration