Add #anchors to modules administration page

Check out how the permissions page does it.. it also had to do unique IDs for each module, so you can probably re-use at least some of the code.

Nice idea. :) if this is done, it'd be nice to update text in places that refer to enabling/disabling a certain module to point directly to it via the #anchor.

Since catch is my new Drupal hero for testing the opt-out patch (http://drupal.org/node/178581) so I figured I would try and help with this one. :)

This seems to work... I used the same approach as the access control screen by adding an ID to the table row.

LOL. Wrong patch. :)

I love the concept. 2 problems with the results, though:

1) (minor) why "#module-book" instead of just "#book" for the anchors?

2) (serious) Apply the patch. Try it out. Go to /admin/build/modules#module-book -- When the page loads, guess what the first module you see is? color.module. Huh? That's right... thanks tableheader.js. :( It's a lovely UI improvement in general, but it sure screws up the potential goodness this patch would be providing. I have no idea what we could do about this, but it basically renders this otherwise lovely idea useless. :(

reroll
it works, and I like it

@Pasqualle: Please read my 2nd point in comment #6 above. This is a show-stopper problem with this approach.

Until there's a solution that gets tableheader.js out of the way in this case, I think this would be *more* confusing, not an improvement. :( Someone please create an issue about fixing tableheader.js (since yes, it affects other pages, not just this one) and post the link here, so we know what issue we're blocked on. Thanks.

@catch: Lovely, thanks for the link.

#184143: tableheader.js interferes with #anchors in table rows is still active.

Reroll.

Updated patch for 8.x, attached.

Update patch, to match path of the latest 8.x.

just want to point out #1440628: [Meta] javascript toolbar/tableheader with url fragment mess that is related to this.

patch in #17 no longer applies to 8.x.

the attached patch works for 8.x.

to test, visit the modules page using a module's name as the named anchor.

for example:

• /admin/modules#link
• /admin/modules#rdf
• /admin/modules#user

Makes #1440628: [Meta] javascript toolbar/tableheader with url fragment mess even more visible, but other than that it's fine with me :)

I really don't think #1440628: [Meta] javascript toolbar/tableheader with url fragment mess should block getting the anchors into this table. Since core doesn't link to the anchors, core can't mess up the display for users.

Can this be committed and backported to 7?

a d7 patch is attached, if and when it's needed.

#23: core-module-anchors-184010-23-D7.patch queued for re-testing.

sorry about the failed d7 patches. didn't realize d8 had to wrap up first. rookie mistake.

back to RTBC per comment #21

here's a patch for D7. is anything further needed to backport?

Related issue #1892872: Add title attribute to modules administration page

Another related issue. #1734534: Optionally show the machine names on the permission page

I tested #30. The patch looks good to me. The same tests described in #20 work in D7 with the patch applied. I'm marking it RTBC.

Moving back to D8, because this caused a regression: #1919564: Listing for Overlay module's layout changes when viewed within the Overlay

Due to this patch, System module violates the CSS namespaces of other modules.

To wit: User module uses #module-[name] CSS IDs on the Permissions page to avoid this trap. System module should do the same.

Attached patch uses #module-[name] HTML IDs, which also means that these IDs are identical to the Permissions page now.

Looks/sounds good to me. Bot's happy. Patch is (obviously) up to standards, etc. ;)

I can confirm the fix works, but do we have anything elsewhere in core that relies on these IDs? If so, those will all need to be updated. I'd suggest we either include that here, or at least somehow make note of it.

Last time I checked, nothing in core relied on it. It was recently.

Yeah, I thought maybe the Update Manager links at the end did, but they just link to the overall Modules page. Seems like a nice follow-up to fix them.

Committed and pushed to 8.x. Thanks!

...should we do the same in the permissions page (for the same usability reasons since that page gets really long too)?

Since that currently appears to work via JS, yes I think so. Want to open a new issue?

...sure: #1976152: Add #anchors to permissions administration page

There's a Drupal 7 patch in #30, but it needs work due to the subsequent followups.

See attached.

#46 is RTBC. It applies the same basic change from #35, adding a "#module-[name]" HTML ID to checkbox labels.

Before:

After:

This is a resuable theme function attached to an alterable form; I'm not totally convinced we know exactly what's being fed into this code well enough to dump it into the HTML unchecked... Shouldn't we be using drupal_html_id() here so we know we're getting a "good" ID in the end?

As an aside, that would probably prevent this from violating any Drupal standards for HTML ID names (for example, it would produce "module-field-ui" rather than "module-field_ui")?

I feel bad bumping this one back, but we can't change the IDs once we get it into Drupal 7... so I think this needs to be finalized first.

Attached patch fixes for D8.

BTW, wasn't even aware of that function; I'd rolled my own for themes, so nice to know there's already one I can use for that. ;-)

50: core-module-anchors-184010-50.patch queued for re-testing.

Needs reroll after #1825952: Turn on twig autoescape by default.

Ok, here is a reroll.

Note, parts of this patch have been committed already (#29, #40). This is the third follow-up as suggested by @David_Rothstein in #49.

+++ b/core/modules/system/system.admin.inc
@@ -227,7 +227,8 @@ function theme_system_modules_details($variables) {
+    $id = drupal_html_id('module-' . $key);

This has been deprecated - need to use Html::getUniqueId() instead.

Ok, here is a corrected one.
Also, do we need to include the DOMDocument helpers for parsing and serializing HTML strings as
use Drupal\Component\Utility\Html;

@stefank: Yes, you also need to add the new use statement.

Thanks. Lets see if that is going to pass.

Great.

This followup was committed as it is fixing a bug rather than a task. Back to backporting.

Committed 058ca31 and pushed to 8.0.x. Thanks!

D7 Port

+++ b/modules/system/system.admin.inc
@@ -2628,9 +2628,11 @@ function theme_system_modules_fieldset($variables) {
+    $id = drupal_html_id('module-' . $key);
...
     if (isset($module['enable']['#id'])) {

Move the call to drupal_html_id() into the if statement.

uh, my bad, you are absolutely right.