[jifty-devel] Form fields and output

Gaal Yahas gaal at forum2.org
Sat Nov 25 07:16:47 EST 2006


On Sat, Nov 25, 2006 at 02:00:11AM -0500, Jesse Vincent wrote:
> > A form field object stringifies to "" but triggers writing its HTML to
> > the web output. Has it been considered to just return the HTML? 
> 
> Short answer: I'm very, very sorry and would like to improve matters.
> Patches are most welcome.  It was an early and expedient hack that never
> got cleaned up.

Here's a patch; not committing since a discussion on IRC brought up some
issues.

First of all, the historical reason for ->out()ing fields and not
returning their text turns out to be that <% $ff %> escapes HTML by
default. Applying this patch directly would break a lot of existing
apps! And adding |n everywhere in the templates is a bit of a hassle.

Alex proposed a patch <http://paste.husk.org/7240> that overrides <% %>
blocks to not apply escapes to Jifty::Web::Form::Elements. If the code
below is to be taken in, it should be used in conjunction with Alex's.
But even then it doesn't Just Work yet; e.g. it breaks Wifty's title,
perhaps because of _("").


=== lib/Jifty/Web/Form/Element.pm
==================================================================
--- lib/Jifty/Web/Form/Element.pm	(revision 579)
+++ lib/Jifty/Web/Form/Element.pm	(local)
@@ -369,7 +369,7 @@
 
 =head2 render_key_binding
 
-Renders the javascript from L</key_binding_javscript> in a <script>
+Returns the javascript from L</key_binding_javscript> in a <script>
 tag, if needed.
 
 =cut
@@ -377,13 +377,12 @@
 sub render_key_binding {
     my $self = shift;
     return unless $self->key_binding;
-    Jifty->web->out(
+    return
         '<script type="text/javascript"><!--' .
         "\n" .
         $self->key_binding_javascript .
         "\n" .
-        "--></script>");
-    return '';
+        "--></script>";
 }
 
 1;
=== lib/Jifty/Web/Form/Field/Button.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Button.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Button.pm	(local)
@@ -37,8 +37,7 @@
         ' />',
         "\n"
     );
-    Jifty->web->out($field);
-    return '';
+    return $field;
 }
 
 1;
=== lib/Jifty/Web/Form/Field/Checkbox.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Checkbox.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Checkbox.pm	(local)
@@ -37,8 +37,7 @@
     $field .= $self->javascript;
     
     $field .= qq! />\n!;
-    Jifty->web->out($field);
-    '';
+    return $field;
 }
 
 =head2 render_value
@@ -58,8 +57,7 @@
     $field .= qq! disabled="disabled" readonly="readonly"!;
     $field .= qq! />\n!;
 
-    Jifty->web->out($field);
-    return '';
+    return $field;
 }
 
 =head2 javascript_preempt
=== lib/Jifty/Web/Form/Field/Combobox.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Combobox.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Combobox.pm	(local)
@@ -14,7 +14,7 @@
 sub render_widget {
     my $self  = shift;
 
-my $field = <<"EOF";
+    my $field = <<"EOF";
 <nobr>
 <span id="@{[ $self->element_id ]}_Container" class="combobox">
 <input name="@{[ $self->fallback_name ]}" 
@@ -34,7 +34,6 @@
 <option style="display: none" value=""></option>
 EOF
 
-
     for my $opt (@{ $self->action->available_values($self->name) }) {
         my $display = $opt->{'display'};
         my $value   = $opt->{'value'} ||'' ;
@@ -45,9 +44,7 @@
         $field .= qq!>$display</option>\n!;
     } 
     
-
-
-$field .= <<"EOF";
+    $field .= <<"EOF";
 </select>
 <script language="javascript"><!--
 ComboBox_InitWith('@{[ $self->element_id ]}');
@@ -55,10 +52,7 @@
 </nobr>
 EOF
 
-
-
-        Jifty->web->out($field);
-    '';
+    return $field;
 }
 
 =head2 render_autocomplete
=== lib/Jifty/Web/Form/Field/Hidden.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Hidden.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Hidden.pm	(local)
@@ -21,7 +21,6 @@
 
 sub render {
     my $self  = shift;
-    $self->render_widget();
-    return '';
+    return $self->render_widget();
 }
 1;
=== lib/Jifty/Web/Form/Field/Password.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Password.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Password.pm	(local)
@@ -41,8 +41,7 @@
 
 
 sub render_value {
-    Jifty->web->out('-');
-    return '';
+    return '-';
 }
 
 =head2 classes
=== lib/Jifty/Web/Form/Field/Radio.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Radio.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Radio.pm	(local)
@@ -13,10 +13,12 @@
 
 sub render_widget {
     my $self  = shift;
+    my $out;
 
     for my $opt (@{ $self->action->available_values($self->name) }) {
-        $self->render_option($opt);
+        $out .= $self->render_option($opt);
     }
+    return $out;
 }
 
 =head2 render_label
@@ -28,11 +30,8 @@
 
 sub render_label {
     my $self = shift;
-    Jifty->web->out(
-        qq!<span class="label @{[$self->classes]}">@{[_($self->label) ]}</span>\n!
-    );
-
-    return '';
+    return
+        qq!<span class="label @{[$self->classes]}">@{[_($self->label) ]}</span>\n!;
 }
 
 =head2 render_option option
@@ -60,8 +59,7 @@
     $field .= qq! /><label for="@{[ $id ]}"!;
     $field .= $self->_widget_class;
     $field .= qq!>$display</label>\n!;
-    Jifty->web->out($field);
-    '';
+    return $field;
 }
 
 1;
=== lib/Jifty/Web/Form/Field/ResetButton.pm
==================================================================
--- lib/Jifty/Web/Form/Field/ResetButton.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/ResetButton.pm	(local)
@@ -25,8 +25,7 @@
         ' />',
         "\n"
     );
-    Jifty->web->out($field);
-    return '';
+    return $field;
 }
 
 
=== lib/Jifty/Web/Form/Field/Select.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Select.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Select.pm	(local)
@@ -30,8 +30,7 @@
         $field .= qq!</option>\n!;
     } 
     $field .= qq!</select>\n!;
-    Jifty->web->out($field);
-    '';
+    return $field;
 }
 
 
@@ -55,8 +54,7 @@
     }
     $field .= HTML::Entities::encode_entities(_($value)) if defined $value;
     $field .= qq!</span>\n!;
-    Jifty->web->out($field);
-    return '';
+    return $field;
 }
 
 1;
=== lib/Jifty/Web/Form/Field/Textarea.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Textarea.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Textarea.pm	(local)
@@ -34,8 +34,7 @@
     $field .= qq! >!;
     $field .= Jifty->web->escape($self->current_value) if $self->current_value;
     $field .= qq!</textarea>\n!;
-    Jifty->web->out($field);
-    '';
+    return $field;
 }
 
 1;
=== lib/Jifty/Web/Form/Field/Upload.pm
==================================================================
--- lib/Jifty/Web/Form/Field/Upload.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field/Upload.pm	(local)
@@ -29,8 +29,7 @@
     my $field = qq!<input type="file" name="@{[ $self->input_name ]}" !;
     $field .= $self->_widget_class();
         $field .= qq!/>!;
-    Jifty->web->out($field);
-    '';
+    return $field;
 }
 
 =head2 render_value
=== lib/Jifty/Web/Form/Field.pm
==================================================================
--- lib/Jifty/Web/Form/Field.pm	(revision 579)
+++ lib/Jifty/Web/Form/Field.pm	(local)
@@ -296,11 +296,10 @@
 
 =head2 render
 
-Outputs this form element in a span with class C<form_field>.  This
+Returns this form element in a span with class C<form_field>.  This
 outputs the label, the widget itself, any hints, any errors, and any
 warnings using L</render_label>, L</render_widget>, L</render_hints>,
-L</render_errors>, and L</render_warnings>, respectively.  Returns an
-empty string.
+L</render_errors>, and L</render_warnings>, respectively.
 
 This is also what C<Jifty::Web::Form::Field>s do when stringified.
 
@@ -308,29 +307,30 @@
 
 sub render {
     my $self = shift;
-    $self->render_wrapper_start();
-    $self->render_preamble();
+    my $out = join "",
+        $self->render_wrapper_start,
+        $self->render_preamble,
+        $self->render_label;
 
-
-    $self->render_label();
     if ($self->render_mode eq 'update') { 
-        $self->render_widget();
-        $self->render_autocomplete_div();
-        $self->render_inline_javascript();
-        $self->render_hints();
-        $self->render_errors();
-        $self->render_warnings();
-        $self->render_canonicalization_notes();
+        $out .= join "",
+            $self->render_widget,
+            $self->render_autocomplete_div,
+            $self->render_inline_javascript,
+            $self->render_hints,
+            $self->render_errors,
+            $self->render_warnings,
+            $self->render_canonicalization_notes;
     } elsif ($self->render_mode eq 'read'){ 
-        $self->render_value();
+        $out .= $self->render_value();
     }
-    $self->render_wrapper_end();
-    return ('');
+    $out .= $self->render_wrapper_end();
+    return $out;
 }
 
 =head2 render_inline_javascript
 
-Render a <script> tag (if neceesary) containing any inline javascript
+Render a <script> tag (if necessary) containing any inline javascript
 that should follow this form field. This is used to add an
 autocompleter, placeholder, or keybinding to form fields where needed.
 
@@ -351,12 +351,11 @@
         )
     );
     
-    if($javascript =~ /\S/) {
-        Jifty->web->out(qq{<script type="text/javascript"><!--
+    return $javascript =~ /\S/ ? 
+        qq{<script type="text/javascript"><!--
     $javascript
 --></script>
-});
-    }
+    } : "";
 }
 
 =head2 classes
@@ -375,7 +374,7 @@
 
 =head2 render_wrapper_start
 
-Output the start of div that wraps the form field
+Return the start of div that wraps the form field
 
 =cut
 
@@ -384,69 +383,57 @@
     my @classes = qw(form_field);
     if ($self->mandatory) { push @classes, 'mandatory' }
     if ($self->name)      { push @classes, 'argument-'.$self->name }
-    Jifty->web->out('<div class="'.join(' ', @classes).'">' ."\n");
+    return '<div class="' . join(' ', @classes).'">' ."\n";
 }
 
 =head2 render_wrapper_end
 
-Output the div that wraps the form field
+Return the div that wraps the form field
 
 =cut
 
 sub render_wrapper_end {
     my $self = shift;
-    Jifty->web->out("</div>"."\n");
+    return "</div>\n";
 }
 
 =head2 render_preamble
 
-Outputs the preamble of this form field, using a <span> HTML element
-with CSS class C<preamble> and whatever L</class> specifies.  Returns an
-empty string.
+Returns the preamble of this form field, using a <span> HTML element
+with CSS class C<preamble> and whatever L</class> specifies.
 
-Use this for sticking instructions right in front of a widget
+Use this for sticking instructions right in front of a widget.
 
 =cut
 
 
 sub render_preamble {
     my $self = shift;
-    Jifty->web->out(
-qq!<span class="preamble @{[$self->classes]}" >@{[_($self->preamble) || '' ]}</span>\n!
-    );
-
-    return '';
+    return <<".";
+<span class="preamble @{[$self->classes]}" >@{[_($self->preamble) || '' ]}</span>
+.
 }
 
 =head2 render_label
 
-Outputs the label of this form field, using a <label> HTML element
-with the CSS class C<label> and whatever L</class> specifies.  Returns
-an empty string.
+Return the label of this form field, using a <label> HTML element
+with the CSS class C<label> and whatever L</class> specifies.
 
 =cut
 
 sub render_label {
     my $self = shift;
-    if ( $self->render_mode eq 'update' ) {
-        Jifty->web->out(
+    return $self->render_mode eq 'update' ?
 qq!<label class="label @{[$self->classes]}" for="@{[$self->element_id ]}">@{[_($self->label) ]}</label>\n!
-        );
-    } else {
-        Jifty->web->out(
-            qq!<span class="label @{[$self->classes]}">@{[_($self->label) ]}</span>\n!
-        );
-    }
-
-    return '';
+    :
+qq!<span class="label @{[$self->classes]}">@{[_($self->label) ]}</span>\n!;
 }
 
 
 =head2 render_widget
 
-Outputs the actual entry widget for this form element.  This defaults
+Returns the actual entry widget for this form element.  This defaults
 to an <input> element, though subclasses commonly override this.
-Returns an empty string.
 
 =cut
 
@@ -461,8 +448,7 @@
     $field .= qq! size="@{[ $self->length() ]}"! if ($self->length());
     $field .= " " .$self->other_widget_properties;
     $field .= qq!  />\n!;
-    Jifty->web->out($field);
-    return '';
+    return $field;
 }
 
 =head2 other_widget_properties
@@ -516,31 +502,28 @@
     # XXX: force stringify the value because maketext is buggy with overloaded objects.
     $field .= HTML::Entities::encode_entities(_("@{[$self->current_value]}")) if defined $self->current_value;
     $field .= qq!</span>\n!;
-    Jifty->web->out($field);
-    return '';
+    return $field;
 }
 
 
 
 =head2 render_autocomplete_div
 
-Renders an empty div that /__jifty/autocomplete.xml can fill
-in. Returns an empty string.
+Returns an empty div that /__jifty/autocomplete.xml can fill
+in.
 
 =cut
 
 sub render_autocomplete_div { 
     my $self = shift;
-    return unless($self->autocompleter);
-    Jifty->web->out(
-qq!<div class="autocomplete" id="@{[$self->element_id]}-autocomplete" style="display: none;"></div>!);
-
-    return '';
+    return '' unless($self->autocompleter);
+    return
+qq!<div class="autocomplete" id="@{[$self->element_id]}-autocomplete" style="display: none;"></div>!;
 }
 
 =head2 render_autocomplete
 
-Renders the div tag and javascript necessary to do autocompletion for
+Returns the div tag and javascript necessary to do autocompletion for
 this form field. Deprecated internally in favor of
 L</render_autocomplete_div> and L</autocomplete_javascript>, but kept
 for backwards compatability since there exists external code that uses
@@ -550,12 +533,12 @@
 
 sub render_autocomplete {
     my $self = shift;
-    return unless $self->autocompleter;
-    $self->render_autocomplete_div;
-    Jifty->web->out(qq{<script type="text/javascript"><!--
+    return '' unless $self->autocompleter;
+    my $out = $self->render_autocomplete_div;
+    $out .= qq{<script type="text/javascript"><!--
     @{[$self->autocomplete_javascript]}
---></script>});
-    return '';
+--></script>};
+    return $out;
 }
 
 
@@ -569,7 +552,7 @@
 
 sub autocomplete_javascript {
     my $self = shift;
-    return unless($self->autocompleter);
+    return '' unless($self->autocompleter);
     return qq{new Jifty.Autocompleter('@{[$self->element_id]}','@{[$self->element_id]}-autocomplete')};
 }
 
@@ -583,7 +566,7 @@
 
 sub placeholder_javascript {
     my $self = shift;
-    return unless $self->placeholder;
+    return '' unless $self->placeholder;
     my $placeholder = $self->placeholder;
     $placeholder =~ s{(['\\])}{\\$1}g;
     $placeholder =~ s{\n}{\\n}g;
@@ -609,25 +592,21 @@
 
 =head2 render_hints
 
-Renders any hints for using this input.  Defaults to nothing, though
-subclasses commonly override this.  Returns an empty string.
+Returns any hints for using this input.  Defaults to nothing, though
+subclasses commonly override this.
 
 =cut
 
 sub render_hints { 
     my $self = shift;
-    Jifty->web->out(
-qq!<span class="hints @{[$self->classes]}">@{[_($self->hints) || '']}</span>\n!
-    );
-
-    return '';
-
+    return
+qq!<span class="hints @{[$self->classes]}">@{[_($self->hints) || '']}</span>\n!;
 }
 
 
 =head2 render_errors
 
-Outputs a <div> with any errors for this action, even if there are
+Returns a <div> with any errors for this action, even if there are
 none -- AJAX could fill it in.
 
 =cut
@@ -635,17 +614,15 @@
 sub render_errors {
     my $self = shift;
 
-    return unless $self->action;
+    return '' unless $self->action;
 
-    Jifty->web->out(
-qq!<span class="error @{[$self->classes]}" id="@{[$self->action->error_div_id($self->name)]}">@{[  $self->action->result->field_error( $self->name ) || '']}</span>\n!
-    );
-    return '';
+    return
+qq!<span class="error @{[$self->classes]}" id="@{[$self->action->error_div_id($self->name)]}">@{[  $self->action->result->field_error( $self->name ) || '']}</span>\n!;
 }
 
 =head2 render_warnings
 
-Outputs a <div> with any warnings for this action, even if there are
+Returns a <div> with any warnings for this action, even if there are
 none -- AJAX could fill it in.
 
 =cut
@@ -653,12 +630,10 @@
 sub render_warnings {
     my $self = shift;
 
-    return unless $self->action;
+    return '' unless $self->action;
 
-    Jifty->web->out(
-qq!<span class="warning @{[$self->classes]}" id="@{[$self->action->warning_div_id($self->name)]}">@{[  $self->action->result->field_warning( $self->name ) || '']}</span>\n!
-    );
-    return '';
+    return
+qq!<span class="warning @{[$self->classes]}" id="@{[$self->action->warning_div_id($self->name)]}">@{[  $self->action->result->field_warning( $self->name ) || '']}</span>\n!;
 }
 
 =head2 render_canonicalization_notes
@@ -671,12 +646,10 @@
 sub render_canonicalization_notes {
     my $self = shift;
 
-    return unless $self->action;
+    return '' unless $self->action;
 
-    Jifty->web->out(
-qq!<span class="canonicalization_note @{[$self->classes]}" id="@{[$self->action->canonicalization_note_div_id($self->name)]}">@{[$self->action->result->field_canonicalization_note( $self->name ) || '']}</span>\n!
-    );
-    return '';
+    return
+qq!<span class="canonicalization_note @{[$self->classes]}" id="@{[$self->action->canonicalization_note_div_id($self->name)]}">@{[$self->action->result->field_canonicalization_note( $self->name ) || '']}</span>\n!;
 }
 
 1;
=== lib/Jifty/Web/Form/Link.pm
==================================================================
--- lib/Jifty/Web/Form/Link.pm	(revision 579)
+++ lib/Jifty/Web/Form/Link.pm	(local)
@@ -103,18 +103,17 @@
     $tooltip = Jifty->web->escape( $tooltip )
         if ( $tooltip and $self->escape_label );
 
-    Jifty->web->out(qq(<a));
-    Jifty->web->out(qq( id="@{[$self->id]}"))         if $self->id;
-    Jifty->web->out(qq( class="@{[$self->class]}"))   if $self->class;
-    Jifty->web->out(qq( title="@{[$self->tooltip]}")) if $tooltip;
-    Jifty->web->out(qq( target="@{[$self->target]}")) if $self->target;
-    Jifty->web->out(qq( accesskey="@{[$self->key_binding]}")) if $self->key_binding;
-    Jifty->web->out(qq( href="@{[Jifty->web->escape($self->url)]}"));
-    Jifty->web->out( $self->javascript() );
-    Jifty->web->out(qq(>$label</a>));
-    $self->render_key_binding();
-
-    return ('');
+    my $out = qq(<a);
+    $out .= qq( id="@{[$self->id]}")         if $self->id;
+    $out .= qq( class="@{[$self->class]}")   if $self->class;
+    $out .= qq( title="@{[$self->tooltip]}") if $tooltip;
+    $out .= qq( target="@{[$self->target]}") if $self->target;
+    $out .= qq( accesskey="@{[$self->key_binding]}") if $self->key_binding;
+    $out .= join "",
+        qq( href="@{[Jifty->web->escape($self->url)]}"),
+        $self->javascript,
+        qq(>$label</a>),
+        $self->render_key_binding;
 }
 
 1;

-- 
Gaal Yahas <gaal at forum2.org>
http://gaal.livejournal.com/


More information about the jifty-devel mailing list