This variable makes all templates require per page caching (or at least per (bool) $is_front_page caching) in theory, even if it's never used.

Let's see if moving it to template_preprocess_page() breaks anything.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Fabianx’s picture

Good catch!

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

RTBC, makes a lot of sense, we need to be more careful about this now.

jibran’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/includes/theme.inc
@@ -1773,6 +1762,16 @@ function template_preprocess_page(&$variables) {
+  // drupal_is_front_page() might throw an exception.
...
+    $variables['is_front'] = drupal_is_front_page();

It should be \Drupal::service('path.matcher')->isFrontPage() now.

catch’s picture

FileSize
2.26 KB

The last submitted patch, is_front_0.patch, failed testing.

catch’s picture

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the fix.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

We need a change record for this.

almaudoh’s picture

Small nit...

+++ b/core/includes/theme.inc
@@ -1773,6 +1762,16 @@ function template_preprocess_page(&$variables) {
+    $variables['is_front'] = Drupal::service('path.matcher')->isFrontPage();

Should be \Drupal::service() right?

lauriii’s picture

Status: Needs work » Needs review
FileSize
2.26 KB
471 bytes
davidhernandez’s picture

Issue tags: -Needs change record

Change draft added. Am I correct in thinking that because this is in template_preprocess_page now instead of template_preprocess that the variable isn't accessible everywhere now, by default?

Fabianx’s picture

That is correct, but needed else every cache item is potentially 'PER_PAGE' suddenly ...

davidhernandez’s picture

Just modifying some comments.

Otherwise, I'm fine with the change. I played with it some and it seems to work fine for page templates. On the negative side, this reduces the availability of the variable, but the performance improvement outweighs that concern. Any module/theme depending on the variable is easily fixed by checking for the front page directly.

While searching I noticed there are still a number of references to drupal_is_front_page(). Since that function is deprecated now, shouldn't those be changed? (out of scope for this issue) I didn't see an issue for it.

davidhernandez’s picture

Forgot to upload the patches.

lauriii’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 90a26b4 and pushed to 8.0.x. Thanks!

  • alexpott committed 90a26b4 on 8.0.x
    Issue #2370147 by catch, davidhernandez, lauriii: Move is_front variable...
Fabianx’s picture

Any module/theme depending on the variable is easily fixed by checking for the front page directly.

Yes, but as soon as they do that, they will need to ensure that the cache granularity of all objects on that page has the CACHE_PER_PAGE cache context.

davidhernandez’s picture

Fabianx, how do you do that? I want to add that information to the change record, but I'm not finding documentation for it or an implementation to use as an example.

Fabianx’s picture

I honestly don't know how to do that.

I only know that core is setting #pre_render and #cache keys and that that contains cache contexts.

Wim: ^^
Nat: ^^

Status: Fixed » Closed (fixed)

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

stefan.korn’s picture

I am currently trying to figure out a good way to check for front page in a template other than page.html.twig.

Going to preprocess functions for each template that is needed?

I also tried to make a custom Twig Extension that checks with \Drupal::service('path.matcher')->isFrontPage(); for the front page but this is not working, probably due to caching.

Is there still an easy way to check for frontpage like in D7?

Would be great if someone can give me a hint.

Fabianx’s picture

#22:

So whenever you check the front page you will also need to add a cache context so that the caching layer knows that your block or entity or whatever is encompassed is varied by 'route'.

However per route would be a lot of granularities.

Therefore dawehner proposed that in your contrib twig extension you also register a new cache context:

route.is_front

which just returns two states: TRUE if its the front-page or FALSE otherwise.

Your code in the extension would then need to add the cache context and the easiest is to just render a render array with that, like:

$build = [];
$build['#cache']['contexts'][] = 'route.is_front';
\Drupal::service('renderer')->render($build);

And thats it.

markhalliwell’s picture