In 6.x-1.x the menu attributes form is not displayed because a value FAPI element cannot contain our fieldset.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

amateescu’s picture

Status: Active » Needs review
FileSize
530 bytes

And the patch.

Btw, I think that our branches are kinda messed up ATM. I propose that we create a 6.x-2.x branch before applying this patch.

Dave Reid’s picture

6.x-2.x branch created. Not sure why I messed that up originally...

amateescu’s picture

Title: Menu attributes not displayed » Menu attributes form not displayed
Priority: Normal » Major
Status: Needs review » Needs work

I investigated this a bit so you won't have to waste your time with it. The problem was introduced in #1200932: Menu Attributes overwrites menu link option set by other modules. I know, my bad. In my defense, the tests I wrote for 7.x-1.x will prevent us from doing this mistake again :)

Before that patch, $form['options'] was being overridden and it's #type was FAPI's default markup. After the patch, $form['options'] keeps it's previous type, which is value.

The problem with setting $form['options'] to item or markup is that it gives us a nasty 'Array' printed just above our fieldset, so I see no other option than moving the whole structure outside of $form['options'] and transfer our values over there in a validate handler.

I'll think about it some more and come up with a patch in a day or two.

amateescu’s picture

Status: Needs work » Needs review
FileSize
3.99 KB

Finally, here's a patch with the approach from #3 :)

Dave Reid’s picture

Hrm, I don't like the idea of not using 'options'. I think there should be another way around this.

amateescu’s picture

Then #1 should do the job.

Dave Reid’s picture

Status: Needs review » Reviewed & tested by the community

Yeah #1 seems ok for now. Does this need to be applied to D7 as well?

amateescu’s picture

I can't reproduce the problem in D7 but I think it should be applied, just to be on the safe side.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1241828-menu_attributes_form_not_displayed-4.patch, failed testing.

amateescu’s picture

Priority: Major » Critical
Status: Needs work » Needs review
FileSize
728 bytes

After trying various things using the patch from #1 as the starting point, the fix is even simpler, we just need to set the #type to 'form', and there's no naggy 'Array' printed anywhere. #facepalm for wasting a few hours on this during the Drupalcon London code sprint :(

amateescu’s picture

After inspecting the document source (not just FIrebug), this actually puts another
element in our existing form, so disregard the patch from #10.

The patch from #1 brings us back to the situation from #1200932: Menu Attributes overwrites menu link option set by other modules and I could find no other alternatives with D6's Form API. Dave, can you reconsider #5? :)

joelpittet’s picture

@amateescu Thank you #1 worked like a charm. Seems a bit strange but did the trick.

joelpittet’s picture

Issue summary: View changes
Status: Needs review » Fixed

#1 was RTBC in #7 So committing that thanks @amateescu.

Status: Fixed » Closed (fixed)

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