I consider this to be a bug as it used to have the label and the uninstall page still does. (One of the two is wrong.)

You should be able to click on the module name and have the checkbox be selected.

Patch fixes that.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

obsidiandesign’s picture

Patch works correctly here; both select and deselect of the checkbox occur by clicking on the module name repeatedly, in admin/build/modules and the uninstall page.

boombatower’s picture

Status: Needs review » Reviewed & tested by the community

I assume that means. Thanks.

Dries’s picture

Status: Reviewed & tested by the community » Needs review

The for-attribute that we generate for those labels are a bit long/verbose: edit-modules-Core---optional-contact-enable. Also, given that they include strings from the .info file, have we check that these are escaped properly? Let's talk about this a bit and mark it back to RTBC if that is considered to be best.

boombatower’s picture

Is that out of scope (or change in scope)? Since that is generated based on the field names and such and this patch simply uses it for the label? It is already sent to the page.

I noticed that is was rather long as well and I agree it would seem overly long.

Should we either:
Create a separate issue or change this one.
Commit this issue and then work on id.

kscheirer’s picture

patch applies cleanly (7 line offset) and works for me.

The checkbox IDs are kinda long, but that's not this patch's fault. If that gets fixed along with the escaping issue, the labels will use the new values.

catch’s picture

boombatower’s picture

Status: Needs review » Reviewed & tested by the community

That issue doesn't contain anything related to this and this is a very small patch.

I think the id issue should be moved to that thread.

Per #5 marking as RTBC.

boombatower’s picture

ping.

boombatower’s picture

FileSize
852 bytes

Applied with offset so re-rolled.

boombatower’s picture

Still applies, and ready to go.

Damien Tournoud’s picture

If I understand correctly, this restores a feature lost by #229129: System module page *seriously* broken:

-        $row[] = '<strong><label for="' . $form['status'][$key]['#id'] . '">' . drupal_render($form['name'][$key]) . '</label></strong>';

As such, it is a (very small) but good first step toward restoring the functionality of the modules admin page.

boombatower’s picture

Yes, very small, but as I found this issue before with the uninstall page it would be nice to keep them both working.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed. Thanks!

Anonymous’s picture

Status: Fixed » Closed (fixed)

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