Problem/Motivation

This came up while working on #2369981: Not found templates are displayed literally instead of throwing an Exception due to string loader.

When twig_render_template() catches a \Twig_Error_Loader exception (for example when a Twig template is not found), the $output variable and its array keys are not yet defined, but twig_render_template() carries on trying to manipulate them and you get some excessive notices:

Notice: Undefined variable: output in twig_render_template() (line 66 of core/themes/engines/twig/twig.engine).
Notice: Undefined index: debug_prefix in twig_render_template() (line 66 of core/themes/engines/twig/twig.engine).
Notice: Undefined index: debug_info in twig_render_template() (line 101 of core/themes/engines/twig/twig.engine).
Notice: Undefined index: debug_suffix in twig_render_template() (line 104 of core/themes/engines/twig/twig.engine).

These notices don't affect functionality at all, but they are annoying.

Proposed resolution

Minor internal refactor: Initialize $output and its array keys early so that only the actual \Twig_Error_Loader exception (via drupal_set_message()) is shown.

Remaining tasks

  • Patch
  • Tests (no test additions/changes made, see #4)
  • Review

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because the patch fixes PHP notices.
Issue priority Normal, just a bug fix for annoying PHP notices when Twig templates are not found.
Prioritized changes Bug fix
Disruption Not disruptive at all, minor internal refactor to remove PHP notices.

User interface changes

Gets rid of ugly notices when, for example, a Twig template is not found.

API changes

n/a

Files: 
CommentFileSizeAuthor
#3 interdiff.txt782 bytesCottser
#3 2430981-3.patch972 bytesCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,447 pass(es). View

Comments

Cottser’s picture

Status: Active » Needs review
FileSize
988 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,313 pass(es). View

Initial patch.

joelpittet’s picture

Nice catch, little nit and I don't think this needs tests it's not changing the functionality just initializing the variables but your call:

+++ b/core/themes/engines/twig/twig.engine
@@ -51,13 +51,14 @@ function twig_init(Extension $theme) {
+    'debug_prefix'    => '',
+    'debug_info'      => '',
+    'rendered_markup' => '',
+    'debug_suffix'    => '',

Can you remove the space alignment here?

Cottser’s picture

FileSize
972 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 87,447 pass(es). View
782 bytes

Pondering a test for this but you are probably right. Here is that change + short array syntax :) Interdiff is probably not too useful in this case.

Cottser’s picture

Issue tags: -Needs tests

Coming up short on how to test this and don't want to spend all night on it. The closest I got was a thought to try and test twig_debug output but PHP powers through all the undefined arrays and strings and the twig_debug output still gets rendered in the end in the exact same way.

Cottser’s picture

Issue summary: View changes

Updated the issue summary, added beta evaluation.

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

RTBC, looks good.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed f153619 and pushed to 8.0.x. Thanks!

  • alexpott committed f153619 on 8.0.x
    Issue #2430981 by Cottser: Unnecessary notices when twig_render_template...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.