Using the latest Drupal (7.36) and menu_attributes together, I cannot add attributes to menu items (the <li> elements). I can add them to the links just fine, and they appear. The attributes persist when the menu item editor form is saved, but do not appear in the generated HTML output.


(EDIT @diqidoq) Issue Summary: Read the comments #7-#11. It seems that this is mostly caused by themes overriding the menu rendering and we need reports from users, who have this issue, especially their Drupal core 7.x? version and the (unmodified!) theme and additional modules they use. By following the rules of Drupal core to respect the theme as the last element in the chain, what I would strongly recommend, it momentary is to be feared that this issue runs into "works as designed" if not reported differently.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ubermichael’s picture

Issue summary: View changes
jboeger’s picture

Same issue for me.

joelpittet’s picture

Priority: Normal » Major

Hmm I see this too with simplytest.me Bumping to major.

matsjacobsson’s picture

I can confirm the same problem for me too...

Tregonia’s picture

I just encountered this on 7.37. What i just noticed is that, if you are outputting the menu via a menu block, the menu item attributes are attached. However, if you are using a theme function in your template, the attributes are not included in the html. The menu link attributes are applied appropriately, just not the menu item attributes.

dqd’s picture

@Tregonia: can you get a lil' bit more specific on your case? Because I am not sure if you try to say that you were about to add attributes or menu items with attributes via theme function. I know, last makes less sense, but it is possible to do that and that's why I need to ask. It makes a huge difference in this issue. And if you try to add attributes to menu items (first case) while menu_attributes module is on and you can't get your overrides wokring, we probably have another issue going on here. A module should never speak the last word to markup, it should always be theme layer. If menu attributes prevents overriding on theme layer in your case, this is an important issue to report, of course. But it is not the issue we discuss here. Thanks for any more input on this.

dqd’s picture

Running several simplytest.me D7 installations with latest menu_attributes beta and latest dev and with/without additional modules (like views, ctools, panels, token, entity, rules, devel, jquery_update, bootstrap (theme), etc.), doesn't show me the reported effect here, not on <a> element, nor on <li> elements (screenshots and status reports below)

A) 2 tests with latest beta rc3 release (Stock D7 & extended)

ATM: I sadly cannot reproduce the issue / Attributes show up.
The simplytest.me report shows me:

Drupal	7.38   OK
Access to update.php	Protected   OK
CTools CSS Cache	Exists   OK
Configuration file	Protected   OK
Cron maintenance tasks	Last run 1 hour 21 sec ago    You can run cron manually.    OK
Database system	MySQL, MariaDB, or equivalent  OK
Database system version	5.5.43-MariaDB-1ubuntu0.14.04.2   OK
Database updates	Up to date    OK
No update information available. Run cron or check manually.   OK
File system	Writable (public download method)   OK
GD library rotate and desaturate effects	2.1.1-dev   OK
Node Access Permissions	Disabled    OK
PHP	5.5.12-2ubuntu4.4 (more information)   OK
PHP extensions	Enabled    OK
PHP memory limit	256M   OK
PHP register globals	Disabled    OK
Unicode library	PHP Mbstring Extension    OK
Update notifications	Enabled    OK
Upload progress	 Not enabled    OK
Web server	Apache/2.4.7 (Ubuntu)   OK

Screenshot of test installation settings and result: http://pasteboard.co/1GIQDIWS.jpg

B) 2 tests with latest 7.x dev (Stock D7 & extended)

ATM: I sadly cannot reproduce the issue / Attributes show up.

Access to update.php   Protected   OK
CTools CSS Cache	Exists   OK
Configuration file	Protected   OK
Cron maintenance tasks	Last run 2 min 15 sec ago   OK
Database system	MySQL, MariaDB, or equivalent   OK
Database system version	5.5.43-MariaDB-1ubuntu0.14.04.2   OK
Database updates	Up to date   OK
Drupal core update status	Up to date   OK
File system	Writable (public download method)   OK
GD library rotate and desaturate effects	2.1.1-dev   OK
Node Access Permissions	Disabled   OK
PHP	5.5.12-2ubuntu4.4 (more information)   OK
PHP extensions	Enabled   OK
PHP memory limit	256M   OK
PHP register globals	Disabled   OK
Unicode library	PHP Mbstring Extension   OK
Update notifications	Enabled   OK
Upload progress	Not enabled   OK
Web server	Apache/2.4.7 (Ubuntu)   OK
jQuery Update	jQuery 1.4.4 (configure) and jQuery UI 1.8.7   OK

Screenshot of test installation settings and result: http://pasteboard.co/1GOcj67O.jpg


Summary: I would love to see some more test results from other users to confirm that the latest Drupal 7 Core 7.38 update has maybe fixed this menu_attributes issue. And if we all come to the same result, we may should compare the D7 core updates to make sure not to run in this issue again. :)
dqd’s picture

Priority: Major » Normal
Status: Active » Postponed (maintainer needs more info)
joelpittet’s picture

Status: Postponed (maintainer needs more info) » Active
FileSize
168.71 KB
155.88 KB

I can still reproduce this error on simplytest.me


dqd’s picture

@joelpittet: what you reproduce is not the assumed issue, it's rather the Bartik theme override behaviour touching your settings. This can be considered as "works as designed". A theme should always speak the last word in the process. As I already have mentioned above to @Tregonia, a theme is the last point where hooks can do anything in the system before it gets out to the wild, and a module should never try to overcome this. Bartik uses primary and secondy links directly in page.tpl.php which makes menu_attributes settings ineffective.

That's why I wanted to get more details from @Tregonia about, what he tries to achieve in his case, what overrides what. I would assume that some "reproduced" cases here are rather connected to my concerns ;-) that's why I would love to see more user reports.

dqd’s picture

Files added for comparision (same as my links from above in #7). Bootstrap doesn't render menu links directly into page.tpl.php like Bartik does, as mentioned above.

ashleysman’s picture

The problem seems to stem from the theme_menu_link()'s preprocess function never being called, meaning the <li> elements don't get their shiny new attributes.

Simple debugging with injecting an exit(__FUNCTION__); as the first line in Menu Attribute's menu_attributes_preprocess_menu_link() function proves that it never fires.

I'm suspecting that theme_menu_link() and any preprocess/overrides are not being called if the menu is rendered viatheme('links', ...) or alternatives like theme('links__system_main_menu', ...).

Next up is to nail down the differences between this standard way of rendering the menu in a template, and how it's done in the blocks.

UPDATE:

It all works fine if I use this method in the template or adapt it for a preprocess:

$main_menu = menu_tree('main-menu');
$print render($main_menu);

So it appears if a menu is renedered via theme_links() it's not going to add <li> attributes.

dqd’s picture

@ashleysman: did you read the last comments in the issue queue? Sorry, I don't want to cross your efforts here and thanks for your report. But this issue has already another state and I tackled it down the last days and had many conversations with Joel on IRC about it: While there are probably circumstances where your situation fits, we rather have here a conceptual situation where we leave the rules how Drupal should work, if we try to overcome theme situations from the module layers point. I know, that's not what you say, but you may confuse followers of the issue because you post a solution for a situation of you, where we don't no the details from, while we already know that it mostly depends on things where menu_attributes shouldn't do anything: themes. I reported a long test above and explained why menu_attributes "works as designed" because it keeps following the rules of Drupal not to overcome situations caused by themes, since themes should always be the last element in the chain for many good reasons. To make sure that this is not the case with you, we need more details. Renderung menus is very very theme specific and we need ALL details of the issue followers, especially which Drupal core version and which theme (+ version) they use.

But adart from that, we already know that this issue will run into the "don't touch it" status, what we can only prevent by proofing, that it happens with most of the themes without the theme is doing anything with menu rendering. @joelpittet: I strongly recommend to set the status to postponed, as mentioned on IRC to prevent confusion. If it is ok, I'll do that for now until we meet on IRC.

dqd’s picture

Issue summary: View changes
Status: Active » Postponed (maintainer needs more info)
joelpittet’s picture

Thank you both @diqidoq and @ashleysman for tracking down the issue.

theme('links') is definitely the culprit in the matter. Though I'd still like @ubermichael or @jboeger to confirm this is what they are using to generate the links.

theme_links() doesn't deal and/or care about menu item attributes, it can be a list of links for any random generated list.

We could alter theme_links so that it takes more than just link attributes but that may open up a can of worms as @diqidoq mentioned about modifying the theme_ function and the theme itself being the last word.

dqd’s picture

conciliatory measure / modest proposal: let's put sth in the help text of the module and a notice message by first activation to let people know that the module won't override theme settings and theme functions, since this would be a reverse over-engineering of how Drupal works. Menu attributes tries to service a system wide solution of something you usually do on theming level. But since in many use cases many of the menu items want to be present in several themes with same attributes (i.e. multilang-sites or multi-domain installs), menu attributes delivers a central place to add them.

joelpittet’s picture

Plan to try to add something to the front page once one of those two confirms that is the case.

jboeger’s picture

I was not using theme('links')

I'm sorry I can't remember how I fixed it but I think it's just something stupid I did or forgot to do. apologies for the flaky memory.