Download & Extend

Hidden, required modules cause a bogus confirmation message to appear when the modules page is submitted

Project:Drupal core
Version:8.x-dev
Component:system.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work
Issue tags:Needs tests

Issue Summary

Steps to reproduce:

  1. Add code like this to your install profile:

    <?php
    /**
    * Implements hook_system_info_alter().
    */
    function standard_system_info_alter(&$info, $file, $type) {
      if (
    $file->name == 'field_sql_storage') {
       
    $info['hidden'] = TRUE;
      }
    }
    ?>
  2. Go to the admin/modules page and submit the form.
  3. Notice how a nonsensical confirmation message appears on the screen, telling you that you need to enable the Field SQL Storage module to proceed (even though it's a hidden, required module and therefore already enabled).

This bug affects e.g. Drupal distributions which might want to hide certain core required modules from appearing on the modules page in order to simplify it.

I'm attaching two patches, one which I think is correct and one which I almost thought was correct until I realized it causes a regression in certain edge cases. This patch will need tests either way, but I'm uploading both to see exactly what kinds of tests it will need; I'm hoping the second patch will fail existing tests, but I have a feeling it won't :)

AttachmentSizeStatusTest resultOperations
hidden-required-modules.patch1016 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 32,776 pass(es).View details | Re-test
hidden-required-modules-WRONG-FIX.patch864 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 32,774 pass(es).View details | Re-test

Comments

#1

The first patch had a typo in the code comment - reuploading.

AttachmentSizeStatusTest resultOperations
hidden-required-modules.patch1019 bytesIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hidden-required-modules_0.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test
hidden-required-modules-WRONG-FIX.patch864 bytesIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch hidden-required-modules-WRONG-FIX_0.patch. Unable to apply patch. See the log in the details link for more information.View details | Re-test

#2

Status:needs review» needs work

The last submitted patch, hidden-required-modules-WRONG-FIX.patch, failed testing.

#3

Status:needs work» needs review

#1: hidden-required-modules-WRONG-FIX.patch queued for re-testing.

#4

#5

#1: hidden-required-modules.patch queued for re-testing.

#6

Status:needs review» needs work

The last submitted patch, hidden-required-modules-WRONG-FIX.patch, failed testing.

#7

Status:needs work» needs review

Rerolled against 8.x HEAD at the London Ontario Drupal Users Group.

AttachmentSizeStatusTest resultOperations
1205684-7.patch1.01 KBIdlePASSED: [[SimpleTest]]: [MySQL] 41,482 pass(es).View details | Re-test
1205684-7-WRONG-FIX.patch884 bytesIdleFAILED: [[SimpleTest]]: [MySQL] 41,485 pass(es), 1 fail(s), and 0 exception(s).View details | Re-test

#8

Status:needs review» needs work

The last submitted patch, 1205684-7-WRONG-FIX.patch, failed testing.

#9

This is a tricky one test test I think. We want a test module that relies on a hidden module and shows up on the modules page for testing. But we don't want this to show up normally. My approach was to make a test module that relies on another test module and then have the first module override the info of the second so that it sets it to hidden = FALSE. Then we just need to have the test enable the first module and try to enable the second module through the interface. So far this hasn't gone well in my experiments as I am not successfully able to override the hidden property.

#10

This is a decent start for the test but needs some work still.

AttachmentSizeStatusTest resultOperations
hidden-required-modules-1205684-10.patch2.99 KBIdleFAILED: [[SimpleTest]]: [MySQL] 41,716 pass(es), 3 fail(s), and 3 exception(s).View details | Re-test

#11

Status:needs work» needs review

Thanks @lliss, that definitely looks like a good start!

Setting to needs review so testbot can run it. Can you please upload a patch combining the test along with the first patch in #7? See #13 in http://drupal.org/node/1468170.

#12

Status:needs review» needs work

The last submitted patch, hidden-required-modules-1205684-10.patch, failed testing.

#13

Test and patch from #7 rolled into one.

AttachmentSizeStatusTest resultOperations
hidden-required-modules-1205684-13.patch0 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 41,698 pass(es).View details | Re-test

#14

Status:needs work» needs review

#15

Doh. It helps to have something in the file.

AttachmentSizeStatusTest resultOperations
hidden-required-modules-1205684-15.patch4 KBIdleFAILED: [[SimpleTest]]: [MySQL] 41,708 pass(es), 3 fail(s), and 3 exception(s).View details | Re-test

#16

Status:needs review» needs work

The last submitted patch, hidden-required-modules-1205684-15.patch, failed testing.

nobody click here