Download & Extend
Issues

Remove "edit-[name]-wrapper" ID from form element wrappers

Posted by quicksketch on June 5, 2009 at 4:24am
Project:Drupal core
Version:7.x-dev
Component:forms system
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

This patch is a continuation of #43493: FAPI: Add name/type as CSS class for form elements, which adds classes to form element wrappers. Since the form element wrapper class and ID will be essentially interchangeable (the class has slight advantages), it makes sense to remove the ID attribute entirely, since it will serve no purpose that the class does not.

AttachmentSizeStatusTest resultOperations
drupal_form_element_remove_wrapper_ids.patch5.16 KBIdleFailed: Invalid PHP syntax in modules/openid/openid.css.View details
Login or register to post comments

Comments

#1

Posted by quicksketch on June 5, 2009 at 4:27am
Status:needs review» needs work

Note that the ID attribute needs to actually be removed from the wrapper div in theme_form_element() also, so this still needs work that can be corrected once #43493: FAPI: Add name/type as CSS class for form elements is done.

#2

Posted by sun on June 13, 2009 at 10:51pm

Awesome, subscribing

#3

Posted by Dries on June 14, 2009 at 8:05am

#43493: FAPI: Add name/type as CSS class for form elements was committed.

#4

Posted by quicksketch on June 15, 2009 at 11:30pm
Status:needs work» needs review

Re-rolled with the key lines removed from form.inc. After this patch, "edit-$id-wrapper" IDs are dead. The changes are all really straight-forward, but another set of eyes and searches are in order to make sure I didn't miss any and that I properly updated these.

AttachmentSizeStatusTest resultOperations
drupal_form_element_remove_wrapper_ids.patch6.04 KBIdlePassed: 11576 passes, 0 fails, 0 exceptionsView details

#5

Posted by System Message on June 30, 2009 at 10:15am
Status:needs review» needs work

The last submitted patch failed testing.

#6

Posted by quicksketch on July 4, 2009 at 5:27am
Status:needs work» needs review

This patch is still ready to go.

#7

Posted by sun on July 4, 2009 at 10:15am
Status:needs review» reviewed & tested by the community

Testbot confirms.

#8

Posted by Dries on July 4, 2009 at 2:57pm
Status:reviewed & tested by the community» fixed

Committed to CVS HEAD. Thanks!

#9

Posted by System Message on July 18, 2009 at 3:00pm
Status:fixed» closed (fixed)

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

#10

Posted by c960657 on August 16, 2009 at 9:39pm

I think the fix for this issue introduced some regressions - see #550572: CSS+JS regressions related to form-item-[name].

-#edit-type-wrapper, #edit-severity-wrapper {
+.form-item-type-wrapper, .form-item-severity-wrapper {

It seems that these should have been replaced using the following pattern instead, i.e. without the form-item prefix:

-#edit-type-wrapper, #edit-severity-wrapper {
+.type-wrapper, .severity-wrapper {