Deprecate $variables['page'] for node.html.twig

Please see https://www.drupal.org/node/3334622 → and 📌 Provide a mechanism for deprecation of variables used in twig templates Fixed for how to deprecate Twig variables.

I realised this wasn't documented in the deprecation policy so also opened 📌 Document how to deprecate Twig variables Active to fix that.

ankitv18 → changed the visibility of the branch 3458589-deprecate-variablespage-for to hidden.

Hi

It looks much better. The patch is applied cleanly and I even searched in the Drupal code base for $variables[‘page]. I did not find it. So for me, it looks good.

RTBC ++

There are failing tests due to get deprecation messages from this deprecation.

The variable is still in use and it looks like it needs to be set to TRUE or FALSE. Can this really be deprecated or do I just not know something?

Updated the issue summary.

Thanks for updating IS @catch

The 'page' template variable in node templates duplicates 'view_mode' = full.

But 'page' is set to a boolean, how can that be the same as 'view_mode' = full?

Seems to cause valid test failures in tests.

@quietone it's duplicating in the sense that the $page boolean means the same thing as the full view mode. There is some additional logic in setting it, but that logic should not exist at all, just makes it confusing.

There are two failing tests. In both of those the page variable is 'false' and the view_mode is 'full'. So, the $page boolean is not the same thing as the full view mode as stated in #15.

The tests are

• core/modules/taxonomy/tests/src/Functional/Views/TermTranslationViewsTest.php
• core/modules/node/tests/src/Functional/NodeTitleTest.php

So, the $page boolean is not the same thing as the full view mode as stated in #15.

Yes it's not identical 100% of the time, it's identical about 95-99% of the time but that's what makes it even more confusing.

Right, so the MR is completely wrong. Is this is about changing a confusing name or a need to introduce more variables?

The key 'page' is documented for node and clearly states that is is not the same as full mode. And taxonomy also uses 'page' is a similar, though simpler way. It uses $variables['page'] = $variables['view_mode'] == 'full' && taxonomy_term_is_page($term);. Would a change be needed there as well?

I tried to debugged why : core/modules/taxonomy/tests/src/Functional/Views/TermTranslationViewsTest.php its faling and that is because the method : taxonomy_term_is_page($term) is checking for the route \Drupal::routeMatch()->getRawParameter('taxonomy_term') and the test is runing on a view that has a contextual filter argument as : arg_0 .

So , the above check $variables['page'] = $variables['view_mode'] == 'full' && taxonomy_term_is_page($term); will always be false , so some additionla logic i needed to that covers the scenario of view.

@deepakkm and @vakulrai, did you read comment, #18, which starts with, "the MR is completely wrong"? The work to do here is to decide on the resolution, which is shown in the issue summary as 'unknown'.

The history here is that Drupal started of with only hard-coded 'teaser' and 'page' template variables.

At some point, I think in Drupal 7, configurable view modes were introduced. However, the template variables were never updated nor clarified.

We didn't remove 'teaser' until 📌 Deprecate $variables['teaser'] Fixed even though it was 100% a duplicate of the view mode.

'page' is slightly more complicated because the logic for 'page' vs 'full' was never brought fully into sync with each other, so there are subtle differences.

However, I think we should be deprecating $variables['page'] and adjusting any necessary logic to rely on view mode = 'full' - e.g. the separation between 'page' and 'full' is because this has just never been cleaned up properly in 15 years, it is not because there was a well thought out discussion about keeping them similar but subtly different concepts.

If someone had a use case for a further view mode, that has the same configuration as 'full', but avoids any logic around the full mode specifically, that can be achieved by configuring 'full' the same as 'default' and then adding an additional view mode that is not overridden.

Adding related issue as I think replacing this variable with the view mode check would fix that issue as well? Right now we only set page to true if we're on the canonical route (or if we're in preview and also viewing the full/default view mode).

@acbramley yes it would fix that, would just be able to rely on the full view mode.