Posted by amateescu on August 7, 2011 at 9:11am
3 followers
| Project: | Menu attributes |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
In 6.x-1.x the menu attributes form is not displayed because a value FAPI element cannot contain our fieldset.
Comments
#1
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.
#2
6.x-2.x branch created. Not sure why I messed that up originally...
#3
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 isvalue.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.
#4
Finally, here's a patch with the approach from #3 :)
#5
Hrm, I don't like the idea of not using 'options'. I think there should be another way around this.
#6
Then #1 should do the job.
#7
Yeah #1 seems ok for now. Does this need to be applied to D7 as well?
#8
I can't reproduce the problem in D7 but I think it should be applied, just to be on the safe side.
#9
The last submitted patch, 1241828-menu_attributes_form_not_displayed-4.patch, failed testing.
#10
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 :(
#11
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? :)