Views tries to render contextual links when Contextual Links module is disabled

However, the call to _contextual_links_to_id() happens in views_preprocess_html(), and if somehow $variables['page']['#views_contextual_links'] get set to TRUE (which should only happen by a call to views_add_contextual_links(), but who knows), this bug will still occur.

So should we add another call to \Drupal::moduleHandler()->moduleExists('contextual') in views_preprocess_html() too?

Yes I would do that as it makes it more robust. At the moment the only place where #views_contextual_links is set is in views_add_contextual_links(), but this might get used in contrib and custom modules / themes. Looking at the history of this code in #914382: Contextual links incompatible with render cache this was originally only running when the Contextual Links module was enabled, in views_contextual_links_view_alter().

This also needs a test.

the fixes done here are looking perfect.

Are you sure we need to do more than creating a test which enables views but not enable contextual.
Once done, we create a page view, or rather reuse some of the existing test views and it should not break.

Given that I kinda think that this issue is more important than just normal, given that its a fatal error for the admin user.
Login as admin user is not hard in a test, just do a User::load(1) and then use $this->drupalLogin with it.

You can easy JUST enable the standard module, disable context and check that the frontpage is broken for the admin.
There is no way that this is not critical, sorry ...

1. +++ b/core/modules/node/src/Tests/Views/NodeContextualLinksTest.php
   @@ -34,7 +34,7 @@ class NodeContextualLinksTest extends NodeTestBase {
   -  public function testNodeContextualLinks() {
   +  public function ptestNodeContextualLinks() {
   

   Leftover debugging cruft.

2. +++ b/core/modules/node/src/Tests/Views/NodeContextualLinksTest.php
   @@ -94,4 +94,21 @@ protected function renderContextualLinks($ids, $current_path) {
   +    // Ensure that contextual links don't get called for admin users.
   +    $admin_user = User::load(1);
   

   Hmm well this is obviously correct when you have intimate knowledge of the actual bug that we are fixing here, but it is not clear how this comment is related to what the test code is actually doing: simply logging in and retrieving the node overview.

   Maybe we can add a docblock explaining that this is a regression test for a very particular bug, and link to this issue? Something like this:

     /**
      * Tests if the node page works if Contextual Links is disabled.
      *
      * This is a regression test for a bug that occurred on installations that had
      * Views enabled but Contextual Links disabled.
      *
      * @see https://www.drupal.org/node/2379811
      */
   

   Apart from these two minor remarks this is looking good.

Could we open a follow-up to not have views module calling out to private contextual module functions? That looks really odd.

For the quick fix this is looking good though.

1. +++ b/core/modules/views/views.module
   @@ -307,7 +307,7 @@ function views_page_display_pre_render(array $element) {
   +  if (!\Drupal::currentUser()->hasPermission('access contextual links') && \Drupal::moduleHandler()->moduleExists('contextual')) {
   

   We should reverse the condition I think.

   The permissions check could one day throw an exception if the permission doesn't exist in the system, it doesn't at the moment but there have been attempts to fix that before.

2. +++ b/core/modules/views/views.module
   @@ -392,8 +392,8 @@ function views_preprocess_html(&$variables) {
   +  if ($view->getShowAdminLinks() && \Drupal::moduleHandler()->moduleExists('contextual')) {
   

   Same here with the reversal I think.

Was trying to figure out how critical this is.

So it only affects user 1, however it's a fatal error just on visiting any page where this code runs, so if you had a view in a block in both front end and admin themes , it'd be completely unrecoverable - can't even log out to log in as a different admin user.

You'd have to 1. already have a different admin user in the system to log in as - if you don't then you're down to drush 2. have to use another browser or clear cookies to log in as that user.

So it's a bit borderline since you have to do some work to get into this situation, but could leave people with no mitigation once they do. Also new users are more likely to be browsing around as user/1 since that's how we set them up in the installer.

Oh, good idea with the OR.

For me this is now looking fine.

Opened #2392469: views_preprocess_html() calls private contextual module function.

Committed/pushed to 8.0.x thanks!

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

Followup to address the call to the private function _contextual_links_to_id() is #2392469: views_preprocess_html() calls private contextual module function.