360626 - ThrowTemplateError() used in Util.pm

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Frédéric Buclin]

While working on a patch, I reached ThrowTemplateError() in Util::get_text():

    $template->process('global/message.txt.tmpl', $vars, \$message)
        || ThrowTemplateError($template->error());

But this routine isn't defined:

Undefined subroutine &Bugzilla::Util::ThrowTemplateError called at Bugzilla/Util.pm line 498.

Util.pm cannot |use Bugzilla::Error;| because Error.pm already uses Util.pm. I remember we moved all code throwing errors out of Util.pm. Maybe should we do the same with get_text() and move it into Template.pm. After all, get_text() is a template thing.

Comment 1

[Max Kanat-Alexander]

I seem to recall that get_text doesn't work inside Bugzilla::Template, for dependency reasons or something. I think I originally put it there, and I had some problem. Maybe not, though.

Comment 3

[Thommie Rother]

Confirmed for 3.02. It seems as if this also inhibits the presentation of non-english localisation email templates: If "defaultlanguage" is set to the default "en" and "languages" to a different locale ("de"), mails are still sent in "en" although translated templates are present in ../de/default/email/. Wouldn`t it make more sense to connect the mail template language to the default "languages" instead (unless users have individual settings) ?

Comment 4

[Max Kanat-Alexander]

(In reply to comment #3)
> It seems as if this also inhibits the presentation of
> non-english localisation email templates:

  No, that would be unrelated to this bug.

Comment 6

[Reed Loden [:reed]]

ThrowUserError() is used in SqlifyDate() inside of Util.pm... Thoughts?

Comment 7

[Max Kanat-Alexander]

require Bugzilla::Error inside the relevant subroutines.

Comment 8

[Reed Loden [:reed]]

Attachment: patch - v1

something like this?

Comment 9

[Frédéric Buclin]

Comment on attachment 412738 [details] [diff] [review]
patch - v1

>+    require Bugzilla::Error;
>     $template->process('global/message.txt.tmpl', $vars, \$message)
>         || ThrowTemplateError($template->error());

|require| doesn't automatically import ThrowTemplateError(). You have to use the full "path" Bugzilla::Error::ThrowTemplateError. Now, as this error isn't going to happen very often, I wonder if it's a good idea to waste cycle require'ing Bugzilla::Error everytime get_text() is called. I think something like this would be better:

if (!$template->...) {
    require Bugzilla::Error;
    Bugzilla::Error::ThrowTemplateError()
}

Comment 10

[Reed Loden [:reed]]

Attachment: patch - v2

Neat.

Comment 11

[Reed Loden [:reed]]

tip:

Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.97; previous revision: 1.96
done

BUGZILLA-3_4-BRANCH:

Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.86.2.4; previous revision: 1.86.2.3
done

Comment 12

[Max Kanat-Alexander]

By the way, "require" is very fast it doesn't have to load the module--it's just checking %INC, which is just a hash-key check.