System: admin/build/modules missing label

boombatower - August 9, 2008 - 01:52
Project:Drupal
Version:7.x-dev
Component:system.module
Category:bug report
Priority:normal
Assigned:boombatower
Status:closed
Description

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.

AttachmentSize
system_module_label.patch850 bytes

#1

obsidiandesign - August 13, 2008 - 21:01

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.

#2

boombatower - August 14, 2008 - 03:15
Status:needs review» reviewed & tested by the community

I assume that means. Thanks.

#3

Dries - August 14, 2008 - 09:09
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.

#4

boombatower - August 14, 2008 - 22:32

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.

#5

kscheirer - August 14, 2008 - 22:39

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.

#6

catch - August 15, 2008 - 00:06

#7

boombatower - August 15, 2008 - 00:39
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.

#8

boombatower - August 19, 2008 - 01:44

ping.

#9

boombatower - August 23, 2008 - 20:48

Applied with offset so re-rolled.

AttachmentSize
system_module_label.patch 852 bytes

#10

boombatower - August 29, 2008 - 19:14

Still applies, and ready to go.

#11

Damien Tournoud - August 29, 2008 - 21:29

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

<?php
-        $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.

#12

boombatower - August 29, 2008 - 21:31

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

#13

Dries - August 30, 2008 - 09:49
Status:reviewed & tested by the community» fixed

Committed. Thanks!

#14

Anonymous (not verified) - September 13, 2008 - 09:52
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.