fixed layout bugs in Mojolicious::Renderer

mojolicious · May 10, 2013 · 0f3f6d8 · 0f3f6d8
1 parent 5ef8683
commit 0f3f6d8
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 36 deletions.
diff --git a/Changes b/Changes
@@ -26,6 +26,7 @@
     managing more than 10k concurrent connections.
   - Improved Mojolicious to be able to detect the current operating mode from
     the PLACK_ENV environment variable.
+  - Improved renderer performance.
   - Improved Mojo::DOM::HTML performance.
   - Improved html_unescape performance in Mojo::Util.
   - Improved Mojolicious::Plugin::EPLRenderer to cache templates more
@@ -34,6 +35,7 @@
   - Improved tests.
   - Fixed Perl 5.17.11+ compatibility.
   - Fixed a few authority bugs in Mojo::URL.
+  - Fixed layout bugs in Mojolicious::Renderer.
   - Fixed support for HEAD request method in Mojo::Server::CGI and
     Mojo::Server::PSGI.
 

diff --git a/lib/Mojolicious/Renderer.pm b/lib/Mojolicious/Renderer.pm
@@ -66,69 +66,57 @@ sub render {
   # Merge stash and arguments
   @{$stash}{keys %$args} = values %$args;
 
-  # Extract important stash values
   my $options = {
     encoding => $self->encoding,
     handler  => $stash->{handler},
     template => delete $stash->{template}
   };
   my $data   = delete $stash->{data};
-  my $format = $options->{format} = $stash->{format} || $self->default_format;
-  my $inline = $options->{inline} = delete $stash->{inline};
   my $json   = delete $stash->{json};
-  my $text   = delete $stash->{text};
+  my $inline = $options->{inline} = delete $stash->{inline};
+  $options->{format} = $stash->{format} || $self->default_format;
   $options->{handler} //= $self->default_handler if defined $inline;
 
   # Text
   my $output;
-  my $content = $stash->{'mojo.content'} ||= {};
-  if (defined $text) {
+  if (defined(my $text = delete $stash->{text})) {
     $self->handlers->{text}->($self, $c, \$output, {text => $text});
-    $content->{content} = $output
-      if ($c->stash->{extends} || $c->stash->{layout});
   }
 
   # Data
   elsif (defined $data) {
     $self->handlers->{data}->($self, $c, \$output, {data => $data});
-    $content->{content} = $output
-      if ($c->stash->{extends} || $c->stash->{layout});
   }
 
   # JSON
   elsif (defined $json) {
     $self->handlers->{json}->($self, $c, \$output, {json => $json});
-    $format = 'json';
-    $content->{content} = $output
-      if ($c->stash->{extends} || $c->stash->{layout});
+    $options->{format} = 'json';
   }
 
   # Template or templateless handler
-  else {
-    return undef unless $self->_render_template($c, \$output, $options);
-    $content->{content} = $output
-      if ($c->stash->{extends} || $c->stash->{layout});
-  }
+  else { return unless $self->_render_template($c, \$output, $options) }
 
-  # Extendable content
-  if (!$json && !defined $data) {
-
-    # Extends
-    while ((my $extends = $self->_extends($c)) && !defined $inline) {
-      $options->{handler}  = $stash->{handler};
-      $options->{format}   = $stash->{format} || $self->default_format;
-      $options->{template} = $extends;
-      $self->_render_template($c, \$output, $options);
-      $content->{content} = $output
-        if $content->{content} !~ /\S/ && $output =~ /\S/;
-    }
+  # Data and JSON can't be extended
+  return $output, $options->{format} if defined $data || $json;
 
-    # Encoding
-    $output = encode $options->{encoding}, $output
-      if !$partial && $options->{encoding} && $output;
+  # Extends
+  my $content = $stash->{'mojo.content'} ||= {};
+  $content->{content} = $output if $stash->{extends} || $stash->{layout};
+  while ((my $extends = $self->_extends($stash)) && !defined $inline) {
+    $options->{format}   = $stash->{format} || $self->default_format;
+    $options->{handler}  = $stash->{handler};
+    $options->{template} = $extends;
+    $self->_render_template($c, \$output, $options);
+    $content->{content} = $output
+      if $content->{content} !~ /\S/ && $output =~ /\S/;
   }
 
-  return $output, $format;
+  # Encoding
+  $output = encode $options->{encoding}, $output
+    if !$partial && $options->{encoding} && $output;
+
+  return $output, $options->{format};
 }
 
 sub template_name {
@@ -187,8 +175,7 @@ sub _detect_handler {
 }
 
 sub _extends {
-  my ($self, $c) = @_;
-  my $stash  = $c->stash;
+  my ($self, $stash) = @_;
   my $layout = delete $stash->{layout};
   $stash->{extends} ||= join('/', 'layouts', $layout) if $layout;
   return delete $stash->{extends};

diff --git a/t/mojolicious/layouted_lite_app.t b/t/mojolicious/layouted_lite_app.t
@@ -30,6 +30,8 @@ app->defaults(layout => 'default');
 
 get '/works';
 
+get '/mixed';
+
 get '/doesnotexist';
 
 get '/dies' => sub {die};
@@ -107,16 +109,23 @@ my $t = Test::Mojo->new;
 
 # Template with layout
 $t->get_ok('/works')->status_is(200)
+  ->content_type_is('text/html;charset=UTF-8')
   ->content_is("DefaultJust worksThis <template> just works!\n\n");
 
 # Different layout
 $t->get_ok('/works?green=1')->status_is(200)
+  ->content_type_is('text/html;charset=UTF-8')
   ->content_is("GreenJust worksThis <template> just works!\n\n");
 
 # Extended
 $t->get_ok('/works?blue=1')->status_is(200)
+  ->content_type_is('text/html;charset=UTF-8')
   ->content_is("BlueJust worksThis <template> just works!\n\n");
 
+# Mixed formats
+$t->get_ok('/mixed')->status_is(200)->content_type_is('text/plain')
+  ->content_is("Mixed formats\n\n");
+
 # Missing template
 $t->get_ok('/doesnotexist')->status_is(404)
   ->content_is("DefaultNot found happenedNot found happened!\n\n");
@@ -231,6 +240,9 @@ Default<%= title %><%= content %>
 @@ layouts/green.html.ep
 Green<%= title %><%= content %>
 
+@@ layouts/mixed.txt.ep
+Mixed <%= content %>
+
 @@ blue.html.ep
 Blue<%= title %><%= content %>
 
@@ -240,6 +252,10 @@ Blue<%= title %><%= content %>
 % extends 'blue' if param 'blue';
 This <template> just works!
 
+@@ mixed.html.ep
+% layout 'mixed', format => 'txt';
+formats
+
 @@ exception.html.ep
 % title 'Exception happened';
 % layout 'green' if param 'green';