This is a semi-backport of the auto-hiding of modules that have hidden = TRUE in their .info files. It is confusing for end-users when there are Drupal 6 modules only used for SimpleTests but shouldn't ever be enabled. Why should they see them in the first place? We should backport this behaviour from Drupal 7.

CommentFileSizeAuthor
#7 764548-hide-modules-D6.patch5.31 KBdave reid
#2 764548-hide-modules-D6.patch787 bytesdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Original issue is #293223: Roll back "Hide required core modules"

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
StatusFileSize
new 764548-hide-modules-D6.patch787 bytes
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Looks reasonable, can we have a tester or two? :)

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Manual testing probably means:

1) Enable, disable, uninstall, re-install some modules and ensure it works properly. Also ensure that dependencies are taken into account.

2) Comment out a hidden = TRUE declaration, and enable that module.

3) Uncomment that declaration, and enable, disable, ... some modules. The hidden one should stay enabled.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Hidden modules should not be enabled by end-users, so they should not stay enabled. I'm not sure why we'd want to test that or dependencies?

EDIT: I guess to make sure that *normal* module functionality works, so makes sense. It's been working so far for me on lots of enables and disables. Testing is also working fine.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Anyone care to RTBC? I've been using this on some client sites with no issues.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 764548-hide-modules-D6.patch5.31 KB

Reviewing the code in D7 I actually found a bug because the hidden modules I had didn't have dependencies[] in their .info files. If I added dependencies[] = xmlsitemap to xmlsitemap_test.info, I got errors on the module page. Revamped the patch to use the code in D7's system_modules(). This should be ready for final review.

damien tournoud’s picture

damien tournoud commented

Looks good to me from a code review perspective, but need some actual testers.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

I've manually tested this patch and it seems to be working fine.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)

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