525606 - template-before_process hook needs to run *whenever* a template is loaded, not just for $template->process

Status: RESOLVED FIXED

(Bugzilla :: Extensions, enhancement, P1)

Description

[Max Kanat-Alexander]

I've been using the template-before_process hook a lot, and it's really fragile and a pain to have to know what exactly Bugzilla is going to load. For example, if I want to do something before bug/edit.html.tmpl is loaded, I have to know every single template that could possibly load that template, and catch *those* templates.

It would be much better if before_process happened whenever a template was loaded, which is possible by creating a custom Template::Context object.

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, here we go! This involves quite a bit of cleverness as far as working with Template Toolkit goes. The comments explain things pretty well.

This requires the two patches from the blockers.

Comment 2

[Frédéric Buclin]

bitrotten

Comment 3

[Max Kanat-Alexander]

Attachment: v2

Okay, this updates the patch to work against the latest extension code. It also modifies the behavior slightly so that it no longer calls the hook when we're processing a BLOCK, only when we're processing a file.

Comment 4

[Max Kanat-Alexander]

Oh, and the patch still requires the patch from bug 523762.

Comment 5

[Max Kanat-Alexander]

dkl: Out of all the patches that I currently have up for review, I think that this one is the most important one to get in before the freeze this Friday.

Comment 6

[David Lawrence [:dkl]]

Comment on attachment 415313 [details] [diff] [review]
v2

Sorry for the delay in review. I am having a problem with getting this to work as described. If my extension throws an error inside of template_before_process I get the following:

 Bugzilla has suffered an internal error. Please save this page and send it to [e-mail] with details of what you were doing at the time this message appeared.

URL: http://10.211.55.2/bugzilla/enter_bug.cgi

Template->process() failed twice.
First error: file error - recursion into 'global/initialize.none.tmpl'
Second error: file error - recursion into 'global/initialize.none.tmpl'

Without the patch applied, I get the normal error message, header and footer properly. I have not had enough time to track down where this is happening. If I comment out the PRE_PROCESS part in Bugzilla/Template.pm I get the proper error page. I will try to track it down some more tomorrow unless you have any ideas why this is occurring.

Dave

Comment 7

[Max Kanat-Alexander]

Attachment: v3

Okay, the problem was that it's not safe to call the hook during PRE_PROCESS. (I don't know why this wasn't causing trouble during checksetup.pl--that's still a mystery, but I've fixed it now.)

As part of this I've changed PRE_PROCESS to an arrayref, because if a template_before_create implementor wanted to add another PRE_PROCESS template, they'd be doing that anyway, and I wanted to just be always consistent with the type of PRE_PROCESS.

Comment 8

[David Lawrence [:dkl]]

Hmm this is still failing for me. If I insert a ThrowUserError in template_before_create() and then run checksetup.pl I get:

Removing existing compiled templates...
Precompiling templates...Deep recursion on subroutine "Bugzilla::Template::create" at Bugzilla.pm line 180.
Deep recursion on subroutine "Bugzilla::Hook::process" at Bugzilla/Template.pm line 788.
Deep recursion on subroutine "Bugzilla::Extension::Example::template_before_create" at Bugzilla/Hook.pm line 33.
Deep recursion on subroutine "Bugzilla::Error::ThrowUserError" at ./extensions/Example/Extension.pm line 448.
Deep recursion on subroutine "Bugzilla::Error::_throw_error" at Bugzilla/Error.pm line 136.
Deep recursion on subroutine "Bugzilla::template" at Bugzilla/Error.pm line 91.
Killed

Eventually the kernel kills it off. The same happens from the web UI.

On the plus side inserting a ThrowUserError in template_before_process() shows the error screen properly now in the webUI. 

Dave

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> Hmm this is still failing for me. If I insert a ThrowUserError in
> template_before_create() and then run checksetup.pl I get:

  That's a completely different bug. This bug is about template_before_process, so if there's a template_before_create bug, file it and I'll get to it.

Comment 10

[David Lawrence [:dkl]]

Comment on attachment 417806 [details] [diff] [review]
v3

Ok understood. Will file a separate bug for the other. The rest works correctly now and the code/docs look good
as well. r=dkl

Comment 11

[Max Kanat-Alexander]

Checking in Bugzilla/Hook.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Hook.pm,v  <--  Hook.pm
new revision: 1.47; previous revision: 1.46
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.123; previous revision: 1.122
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template/Context.pm,v
done
Checking in Bugzilla/Template/Context.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template/Context.pm,v  <--  Context.pm
initial revision: 1.1
done
Checking in Bugzilla/Template/Plugin/Hook.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template/Plugin/Hook.pm,v  <--  Hook.pm
new revision: 1.16; previous revision: 1.15
done
Checking in extensions/Example/Extension.pm;
/cvsroot/mozilla/webtools/bugzilla/extensions/Example/Extension.pm,v  <--  Extension.pm
new revision: 1.5; previous revision: 1.4
done