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...
-

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cburschka’s picture

Assigned: Unassigned » cburschka
Priority: Normal » Critical

Confirmed by two other reports (and me). Annoying regression. It clearly happened in 3.1 or 3.2, likely the latter.

wretched sinner - saved by grace’s picture

(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.

cburschka’s picture

Status: Active » Needs review
FileSize
1.43 KB

Whoa, 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.

extect’s picture

Patch is working quite well here! Thank you!

wretched sinner - saved by grace’s picture

Works for me perfectly. Thanks for the quick response!

trogie’s picture

Status: Needs review » Reviewed & tested by the community

Works perfectly for me too!

jeffschuler’s picture

This does not seem to be a patch against the 6.x-3.2 release.

Patching fails with:

patching file dhtml_menu.module
Hunk #1 FAILED at 48.
1 out of 1 hunk FAILED -- saving rejects to file dhtml_menu.module.rej

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?

fred0’s picture

I just applied it against the dev snapshot and it worked (and fixed the problem) so, I would say yes.

mustardman’s picture

Any 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.

cburschka’s picture

Status: Reviewed & tested by the community » Fixed

Sorry, sorry. I'd completely forgotten about this. I've committed this fix now.

Status: Fixed » Closed (fixed)

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

cburschka’s picture

Version: 6.x-3.2 » 7.x-1.x-dev
Status: Closed (fixed) » Patch (to be ported)

Problem woke up to rear its cute little head again in the D7 version. Looks like the logic should be changed there too.

cburschka’s picture

Status: Patch (to be ported) » Needs review
FileSize
2.23 KB

Here 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).

cburschka’s picture

Title: menus without dhtml not working ok » Do not touch menus where DHTML is disabled.
FileSize
3.5 KB

Not 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.

cburschka’s picture

Naturally, testing for syntax errors is still a good idea most of the time.

cburschka’s picture

Still more bugs. :/

cburschka’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
3.49 KB

This update just adds a sanity check to make sure there is a stacked link at all.

cburschka’s picture

Committed, tagged, released! DHTML Menu 8.x-1.0-alpha2 is now available for download!

cburschka’s picture

Status: Reviewed & tested by the community » Fixed

Er, 7.x even. Drupal 8 sounds like something from Science Fiction right now. :P

Status: Fixed » Closed (fixed)

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

j0rd’s picture

Version: 7.x-1.x-dev » 6.x-3.5
Status: Closed (fixed) » Needs work

Same 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.

j0rd’s picture

Status: Needs work » Needs review
FileSize
1.63 KB

Here's a patch I've used to disable DHTML from messing with my menu's for which it is disabled.

vuil’s picture

Version: 6.x-3.5 » 7.x-1.x-dev
Issue summary: View changes
Status: Needs review » Needs work

I revert the issue to 7.x-1.x branch for update, test, and contribution if need. Thank you!