confirm_form() shouldn't assume that it is the only one on the form to set the '#attributes' element. Please merge the array instead of assigning it.

CommentFileSizeAuthor
#17 system.module.confirm-form.patch647 bytesadityakg
#15 system.module.confirm-form.patch664 bytesadityakg
#12 system.module.confirm-form.patch664 bytesadityakg
#9 system.module.confirm-form.patch634 bytesadityakg
#8 system.module.patch634 bytesadityakg
#3 system.module.confirm_form.attributes.patch713 bytescha0s
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cha0s’s picture

cha0s CreditAttribution: cha0s commented

bump...

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Version: 6.x-dev » 7.x-dev

Why not suggesting a patch? It will have to be against the 7.x code base (HEAD tag in CVS).

Please read http://drupal.org/patch/create for more information.

cha0s’s picture

cha0s CreditAttribution: cha0s commented
FileSize
system.module.confirm_form.attributes.patch713 bytes

Sure thing. After playing I realized we can just concat the string... which leads me to my next issue:

$form['#attributes']['class'] should be an array. What would I file that under, a feature request..?

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Active » Needs review

@cha0s, let's open a task, if you are willing to take the lead on this.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Needs work

The last submitted patch failed testing.

casey’s picture

casey CreditAttribution: casey commented
Status: Needs work » Fixed

fixed for D7

http://api.drupal.org/api/function/confirm_form/7

casey’s picture

casey CreditAttribution: casey commented
Status: Fixed » Needs work
Issue tags: +Novice

Oops to quick of judgement; no its not.

Currently can't provide patches.

  $form['#attributes'] = array('class' => array('confirmation'));

Should become

  $form['#attributes']['class'][] = 'confirmation';
adityakg’s picture

adityakg CreditAttribution: adityakg commented
FileSize
system.module.patch634 bytes

Hi, I am new here. I am trying to learn on how to apply patches :)

adityakg’s picture

adityakg CreditAttribution: adityakg commented
FileSize
system.module.confirm-form.patch634 bytes

I'm sorry for the previous not-working patch. The previous patch is patched from the 'modules/system' folder instead of drupal '.' folder. 2nd try:

adityakg’s picture

adityakg CreditAttribution: adityakg commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, system.module.confirm-form.patch, failed testing.

adityakg’s picture

adityakg CreditAttribution: adityakg commented
FileSize
system.module.confirm-form.patch664 bytes
adityakg’s picture

adityakg CreditAttribution: adityakg commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, system.module.confirm-form.patch, failed testing.

adityakg’s picture

adityakg CreditAttribution: adityakg commented
Status: Needs work » Needs review
FileSize
system.module.confirm-form.patch664 bytes

sorry for the spamming :(

Status: Needs review » Needs work

The last submitted patch, system.module.confirm-form.patch, failed testing.

adityakg’s picture

adityakg CreditAttribution: adityakg commented
Status: Needs work » Needs review
FileSize
system.module.confirm-form.patch647 bytes
casey’s picture

casey CreditAttribution: casey commented
Status: Needs review » Reviewed & tested by the community

Some trouble with windows style newlines I see, but you fixed that.

Patch is good to go.

adityakg’s picture

adityakg CreditAttribution: adityakg commented

#18: thanks! still getting a grip of it. so, sorry for the spamming :)

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Novice

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

Issue tags: +Novice

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.