Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
menus without dhtml enabled (so should be no dhtml effects) are not fully dhtml-less.
I only want dhtml for my 'navigation' menu, so all the other menus (primary and secondary) are checkboxed. But I still have dhtml effects on those two menus: i have to click twice on menu items with subitems to go to the page, and very strange, the subitems appear and disappear immediately...
-
Comments
Comment #1
cburschkaConfirmed by two other reports (and me). Annoying regression. It clearly happened in 3.1 or 3.2, likely the latter.
Comment #2
wretched sinner - saved by grace CreditAttribution: wretched sinner - saved by grace commented(Just notice that my previous issue was logged and marked as duplicate of this issue - didn't show in my issue queue!)
I have the same regression, but only on primary links. I don't have secondary links in my theme to test.
The Book navigation blocks that I have respect the checkbox settings.
Comment #3
cburschkaWhoa, major logic issues in that code. I wonder how I pulled that off.
The gist is that I hook into the menu theming in two places, theme_menu_item_link and theme_menu_item, to track the recursion levels and load the "missing" parts of the menu tree that are not expanded. This tracking happens with a "stack" variable to which each recursion level is added.
Now in theme_menu_item_link, I had a condition that did *not* try to stack the item if it was something like the "View" and "Edit" tabs, which would cause errors because they're completely different from normal items.
Then I had a condition that added a "disabled" flag to the item if it was inside a non-dhtml menu.
So I merged these two conditions, which meant that a disabled item did not even get added to the recursion-tracking stack. Problem was, that was how I tracked whether an item was disabled in the first place.
In short, the issue should be fixed by this patch. Please test to see if you still get issues.
Comment #4
extect CreditAttribution: extect commentedPatch is working quite well here! Thank you!
Comment #5
wretched sinner - saved by grace CreditAttribution: wretched sinner - saved by grace commentedWorks for me perfectly. Thanks for the quick response!
Comment #6
trogie CreditAttribution: trogie commentedWorks perfectly for me too!
Comment #7
jeffschulerThis does not seem to be a patch against the 6.x-3.2 release.
Patching fails with:
The code that the patch should be remove doesn't appear to exist in the module file.
Does this patch apply to a development snapshot instead of the 6.x-3.2 release?
Comment #8
fred0 CreditAttribution: fred0 commentedI just applied it against the dev snapshot and it worked (and fixed the problem) so, I would say yes.
Comment #9
mustardman CreditAttribution: mustardman commentedAny chance we can get a beta release for 6.x with this fix? I would really like to use DHTML Menu but I can't until this selectable menus issue is resolved. I'd rather not use patched or developer versions.
Comment #10
cburschkaSorry, sorry. I'd completely forgotten about this. I've committed this fix now.
Comment #12
cburschkaProblem woke up to rear its cute little head again in the D7 version. Looks like the logic should be changed there too.
Comment #13
cburschkaHere is a neat fix for this logic. It is based on a part of the big "next-generation" patch already submitted, but it slightly improves on its logic (for example, array_merge_recursive would have merged the ID attribute into an array rather than overwriting the old value).
Comment #14
cburschkaNot bad, but we're still applying a "no-dhtml" class to menus without DHTML. As #352005: Ubercart DHTML conflict - use module-specific selector uses a positive ID (".dhtml-menu") in the selector, the ".no-dhtml" is redundant. With this patch, we just stop messing with the non-DHTML menu, no class, no ID, period.
Comment #15
cburschkaNaturally, testing for syntax errors is still a good idea most of the time.
Comment #16
cburschkaStill more bugs. :/
Comment #17
cburschkaThis update just adds a sanity check to make sure there is a stacked link at all.
Comment #18
cburschkaCommitted, tagged, released! DHTML Menu 8.x-1.0-alpha2 is now available for download!
Comment #19
cburschkaEr, 7.x even. Drupal 8 sounds like something from Science Fiction right now. :P
Comment #21
j0rd CreditAttribution: j0rd commentedSame problem with 6.x-3.5 version. DHTML is overriding my menu's custom IDs, even for menus for which it is disabled.
Ideally, for disabled menu's DHTML Menu would do nothing.
Comment #22
j0rd CreditAttribution: j0rd commentedHere's a patch I've used to disable DHTML from messing with my menu's for which it is disabled.
Comment #23
vuilI revert the issue to 7.x-1.x branch for update, test, and contribution if need. Thank you!