"Secondary menu" exists but is no longer the default source for the secondary links

Duplicate of
http://drupal.org/node/503810

I was just trying to group all issues related to the same problem. Since there's a patch there I marked this as a duplicate.
Anyway we can keep this open. I'lll keep an eye open and close this when we'll find a proper solution.

I posted a patch here:
http://drupal.org/node/503810#comment-2545392

I was just having a play with D7 head, and I found an bug which seems to relate to this one (although, not quite sure if I understand what the description of this issue is really getting at).

There is a "User menu" Menu which has these links (on a clean install):
- user
- logout

And a "User menu" Block

However despite the "User menu" Block being disabled, the links from the "User menu" Menu display in the footer under the Heading "Secondary menu"

So, there is a copy of the "User menu" that displays in the footer region (with Stark theme), and its display can NOT be controlled by the "Secondary menu" or "User menu" blocks. The only way to get rid of it is to disable the individual links in the "User menu" menu.

I hope this isn't a 'feature'!

Just thought I would add that this annoying duplicate user menu that calls itself "Secondary menu" is not in fact the Secondary menu (menu or block). The REAL "Secondary menu" works fine and is independent from it.

Someone else has spotted the problem and uploaded a screenshot:
http://www.flickr.com/photos/jacine-rodriguez/4350108345/

Sorry for spamming this issue. I have now realised that it IS a feature! (bah!)

The display of the so-called "Secondary menu" (that is really the User menu), is controlled by the Theme settings under "Toggle display".

Same thing applies with the "Main menu" (really Primary links navigation)

Using the same names for different things like this is just confusing.

I came across this issue (well, I got here from #273137: Split Navigation to User and Administration menu) while testing out Bartik. I really do think that this is critically broken atm. We currently ship with a menu called "Secondary menu" that is not, in fact, the secondary menu. The secondary menu is "User menu" only because Garland prints the secondary links in the upper right corner, which is where we want those links. This utterly breaks on a theme like Bartik that prints the secondary links in the footer (which is a fine location for secondary links).

Seems to me we need a $user_menu variable, and instruct themes to print it in the upper-right corner as a convention? Or..?

See also #698014: Theme settings for main/secondary variables mismatch with menu settings.

Yeah, jamming the user menu into the secondary menu slot messes up the HTML semantically too. Instead of being marked user-menu as a class (or id) it's marked secondary-menu. :(

It seems like terminology is part of the problem. So, I did the following google search to help clarify: "secondary links" -drupal and "secondary menu" -drupal

shows that most usage of "secondary links" refers to education. As in "elementary and secondary education links" or "secondary and post-secondary education links". I think "secondary links" is a drupalism. It doesn't appear to be well-known term in web development.

"secondary navigation" <-- a known web development term
"secondary menu" <-- lesser known web dev term
"secondary links" <-- education-related term

In Drupalese, we use 'links' to mean 'a horizontal list of links' as opposed to 'menu' which means 'a vertically arranged list which can expand into a tree'.

Whereas everyone else uses 'menu' for both.

If we add a 'user links' variable, then that makes this a duplicate of #408142: Create a 'user links' menu + page template variable, but no-one really responded to that issue at the time, so this one now has more information despite originally being more about killing off the 'secondary menu'. I still think a new theme variable is the way to deal with this issue though.

I spent some time in confusion about this and finally found the DR7 manual entry ( http://drupal.org/node/788984 ) which, with the explanation about the difference between 'menus' and 'links' above, shows how these primary and secondary links can be bent to your will. It is still very confusing out of the box but the solution depends on whether you view this as a documentation problem or a design problem.

If the former, more documentation is needed on the 'menu' administrative pages. If the latter, then these conceptual differences between 'menu' and 'links' may need to be further abstracted (why not tertiary or 'named' links?) including in the administrative panels.

I may not be the best example, but until I found the 'right' info, I found this nomenclature very confusing and frustrating.

Having drill-down primary/secondary links by setting both menu settings to the same value is a relatively obscure core feature, which IMO themes can visually support out of the box, or not. To me, it makes a whole lot more sense for secondary links to appear in the footer by default, and in fact Stark works this way (meaning default core page template works this way), as does Bartik. The right way to handle this is indeed to put menu_block module into core so that any menu can work that way. Drupal 8 material.

Likewise, I don't understand how moving all menus to blocks is a less invasive change to themers porting their stuff to Drupal 7 than the addition of a single new variable, that works the same way as other "primary" menus do in core, for better or worse, right now (and have since time immemorial). I really would love that menus-as-blocks-and-not-weird-one-off-variables refactoring thing to happen, but it's also clearly Drupal 8 material.

So given where we are in the release cycle, #408142: Create a 'user links' menu + page template variable seems the right way to handle this. If you'd rather I "won't fix" this one and mark the other one critical, I can do that, but I'm curious of your thoughts.

Since it seems late to convert these variables to block (There's a patch here, by the way), what if we simply print the user menu in a block and assign the secondary menu to secondary links (like in D6)?

I agree with David. The reason the user menu/page variable issue has stalled is because the Garland secondary link styling is in exactly the same place as where you'd expect to find user links, so without adding a new design element to garland, which no-one has volunteered to work on, there's no way to do the other patch.

I'm fine with the current patch.

With a 'footer menu' we don't currently have any links to show in it, so it wouldn't show up at the bottom of the page by default anyway.

I wonder if the user links where there (in the top right corner) before the toolbar went in. Cause I feel like they overlap with the toolbar user links.
I guess that if I want my users to have user links in the top right corner (like in most sites) I give them access to the toolbar. Otherwise everything is in the sidebar like it was. I believe it makes somewhat easier to understand why there are those duplicated links (for authenticated users without access to the toolbar). But these are just details ...

Let's defer the footer menu to another issue - I think that'd be a good idea for the dummy content profile that's being discussed if not the default one. Otherwise I see nothing to complain about here.

I got bitten by the issue described as #5 in this thread here. It really felt like a ux WTF to me. Using stark to get familiar with the default themes I didn't expect this oddness
The code that's ramming this into the footer in page.tpl.php is rude and unconfigurable.

#16: remove-secondary-menu-410646-16.patch queued for re-testing.

I support the removal of black holes like this (see #16). So, current patch should be committed as is and core themes be handled in followups?

Preamble: I know there are a few open issues that deal with this, but I'm not sure which one is the "primary" issue right now. Let me know if I should be cross-posting somewhere.

Having drill-down primary/secondary links by setting both menu settings to the same value is a relatively obscure core feature

I think it's important that the core themes DO support this drill-down option; however, I don't see how to fix this in Bartik without a huge design bike shed argument. (Big rant deleted.) People who want to show sub-nav options can still: (1) Put Primary Navigation block in Sidebar first, (2) turn off Primary links under theme settings and have "all" of the Primary links show up sub-nav options. I agree it's not elegant. Let's fix it in D8.

My summary of the problem that really does need to be solved is as follows (I'll call everything "links" to avoid the menu/link debate):

• Primary links is now the "main menu."
• Secondary links the page.tpl.php variable are now "footer navigation."
• User links are now the source for Secondary links under Menu -> Settings
• There is no page.tpl.php variable to place both (this is where I start to twitch) Secondary links and Secondary links.

My recommendations:

1. Update the description text on Menu -> Settings (top of the page text) to:

   The menu module allows on-the-fly creation of menu links in the content authoring forms.

   An advanced option allows you to use the same source for both Main links (currently Main menu) and Secondary links: if your source menu has two levels of hierarchy, the top level menu links will appear in the Main links, and the children of the active link will appear in the Secondary links.

   Display location of the Primary and Secondary links is handled by the theme.

2. Update the help text on Menu -> Settings -> Source for Primary links to read, "Select the source for the Primary links which are typically displayed at the top of the page by the theme. Used in content editing forms as the default "Parent item" for Menu settings."
3. Update the help text on Menu -> Settings -> Source for Secondary links to read ONLY, "Select the source for the Secondary links. Typically at the bottom of the page."
4. Add a new page.tpl.php variable for $user_links so that themes can (optionally) provide support for the default Menu settings of Main links/User links AND the drill-down Primary/Secondary links.
5. Bartik: Leave as-is. (Do not place $user_links into the page.tpl.php file.)
6. Garland: move $user_links to the footer. Put $secondary_links at the top (just below $primary_links). Leave the default menu for Secondary links set to User links.
7. Fix the rest of the brokenness in D8. ;)

Just my two cents which hopefully is worth more than just throwing "subscribe" onto the issue which is what I really wanted to do. ;)

> I think it's important that the core themes DO support this drill-down option; however, I don't see how to fix this in Bartik without a huge design bike shed argument.

Where were you when I was making that point for the list of core theme requirements? ;)

IMO #503810: Convert primary and secondary menus to blocks is the correct solution to the problem and I really don't see any good reason why this needs to be a D8 issue.

Of course I would prefer the menu_block module in core, but it's too late for that.

Just my 2 cents.

@jacine. Agreed that getting rid of those two page.tpl.php variables is the right solution eventually, but it's not what the UI text describes right now and it does change the established behaviour of how those links have been used up to now. :( Getting rid of the functionality also makes it (virtually) impossible for point-click Drupal installs to do anything vaguely nice for sub-nav. This is a step BACKWARDS.

@emmajane, I'm not saying to remove that functionality. I agree it is a step backwards. I just think it should be in a block (maybe a new one would be best).

It just really pains me to see these in Drupal 7. Users should be able to move these menus around, like everything else. I just wish we could move past these hacks instead of forcing themes to deal with them, or creating other less crappy hacks in place of crappier ones.

Also, I don't buy the argument that Drupal 7 themers are too far along with porting their themes to deal with such a change. I think most themers would welcome this change, and at this point the list of changes is so long, what difference does one more make?

EDIT: Also, if this is done nicely Bartik can probably make drop-down menus (wouldn't that be nice)...

At this stage I'm going to back the proposal in #18, I think this is the most consistent with D6 and not a big deal to roll with right now. Yes Bartik and Stark will need to change - Stark easy, Bartik - we'll just have to put them right up the top - seems easy also.

Documentation changes proposed in #30 seem to make sense, but cannot agree with points #4, #5 or #6 - these seem to bring in unnecessary inconsistencies for core themes.

-1 to a $user_links sort of variable, please no more un-movable variables (blocks are the way to go for end users, we all know that, so lets not take a step backwards).

I agree also that doing all as blocks is the right way to go, but most likely a Menu Block in core/lets do it right in D8 sort of thing.

After a good night's sleep and appreciating that this is BROKEN in gnarly ways and that we are not going to be able to solve it elegantly for D7 I've attached a revised plan. Jeff's right. No new page variables. That's going backwards. I don't like that it's broken, but I think we screwed up and missed out on the opportunity to nuke these page variables and make menus into blocks.

For now: Let's make the help text accurate. Leave the drill-down as an Easter Egg for themes that choose to apply it and fix the whole mess in D8.

I have attached a patch which "fixes" the text on Menu -> Settings to reflect what these options actually do.

New text is as follows:

Drupal provides two configurable menu areas within the page template. Themes may display the menus in non-conventional locations or may also choose to omit them completely.

Source for the Main links
Typically displayed at the top of the page.

Source for the Secondary links
Typically displayed in the footer or as a complementary menu to the Main links.

I think thats probably gonna be fine for now, indeed not the "best" solution but lets revisit that in D8.

Not sure if we are now fixing the actual bug?

The description of Secondary menu has already been changed. In the current version of head it reads, "The Secondary menu is used on many sites for legal notices and contact pages."

Out of all the other options that have been proposed, I think @emmajane's solution in #37 is best.

BTW, I really hope to see everyone here pushing for this in D8: #474004: Add options to system menu block so primary and secondary menus can be blocks rather than variables

Moving over to Bartik theme so that we can get Jen's attention on this issue.

I just don't see any way to get #503810: Convert primary and secondary menus to blocks done and in in a short enough period of time, and furthermore it's a HUGE change for themers this late in the game: templates would need to be adjusted from expecting the main menu to be a simple bulleted list of links to being a block with potentially 2, 3, 4, 27 nested levels of links. I do agree that UX-wise it's a WTF since that's the only part of the page (apart from site name I guess) that you don't get a hover-configure link on, but unless someone points out the flaw in my thinking here, I don't see any way this can possibly be D7 material.

After reading all the pros/cons/etc. of this issue (thanks a lot for the detailed discussion!), I agree with David that making Bartik's (and Stark's) secondary links location conform to that of Garland's is the simplest way forward in D7 (and removing "Secondary menu" from new installations). I realize this is a regression from what Jen wanted to do with the design, but we have a critical issue with the user menu appearing in the lower-*left* instead of the upper-*right* that needs to be fixed somehow, and solving it this way also solves this other critical, release-blocking UX wtf (Secondary menu = user links, not "Secondary menu".. ugh.).

@webchick, as jacine mentioned in #35 it is more likely to be a welcome change to have #503810: Convert primary and secondary menus to blocks implemented. I'll try and round up some additional opinions on this part.

Ok, we (emmajane, JohnAlbin, jarek, bleen18, et al) just had a LONG discussion in #drupal-contrib about this whole tangled web of issues. We basically have the following:

1. The new "User menu" in Drupal 7 should, by UX convention, appear in the upper-right corner.
2. Because Garland doesn't have a region for "upper-right corner", but it does have its $secondary_links variable appearing in *almost* the upper-right corner, we made a horrible decision at some point to make the default "Secondary menu" source the "User links" menu to get the general idea of how it was supposed to work. This was a horrible decision, and resulted in several critical fall-out issues.
3. The first critical issue is that we have this "Secondary menu" menu that doesn't actually appear anywhere, least of all in the "Secondary menu" variable, where one would expect. This is a complete and total WTF disaster, and I see this nailing people new to Drupal 7 on a daily basis.
4. The second critical issue is that Bartik's maintainers made a design decision to put secondary links in the footer. There's nothing wrong with this design decision, per-se, except that with the way things are currently wired up, instead of User menu appearing in the upper right, it now appears in the lower left. That could not possibly be more wrong.
5. System module's page.tpl.php also has this "print $secondary_menu in the footer" design decision, so that means any themes that try and sub-theme from core get this "user links in the complete opposite place" applied to them as well. Nast-o-riffic.

Things we've ruled out:

• #503810: Convert primary and secondary menus to blocks is definitely Drupal 8 material. This comes from "The Man" himself (JohnAlbin). In addition to there being no workable code in that issue, changing this is going to require lots and lots of follow-up work to themes to account for the new paradigm shift. So let's consider that one formally scratched off the list of possibilities, but a high priority in D8.
• In some issue somewhere (possibly this one), we suggested making a new $user_links variable, that basically works the same as $main_menu and $secondary_menu. This would involve Yet Another Toggle to the admin menu settings page, and further spreading this unpopular and confusing idea of "magic" menu variables in tpl.php files. Understandably un-popular with themers, and especially if moving these all to blocks is where we ultimately want to go, adding a new one of these nasties is not desired. So let's scratch that from the list, too.

So, it appears what we need to do to move forward here is to adjust all core themes (and system module's page.tpl.php) to provide a block region for "upper-right hand corner" for placing menus such as the user menu block. And then adjust default.install to put it there, instead of secondary links, and restore secondary menu to the secondary links so all is harmonious again.

@webchick I'm pretty excited about this solution. It will be our first instance of "menu in a region by default" instead of "menu in a hard coded page variable." I am +1 on your recommendations.

oops. Cross post killed tags.

I went back and took a look at Garland's top right. is there a reason why we can't turn on the "Admin" toolbar for authenticated users? That would put the User links in the "correct" top right according to the original UX issue: #382890: Move 'My account, Help & Logout' links to separate menu and place it top right.

With the permission turned on (and no other changes made) this makes Garland's top banner look like the attached screenshot.

If there are performance issues with this, why not replicate the visual black bar and get the Utility links (aka User links) into the "right" place according to the original UX issue. #382890: Move 'My account, Help & Logout' links to separate menu and place it top right.

In terms of the page template, the term "secondary menu" ($secondary_nav variable) is a semantic one that describes the importance and maybe hints as to positioning. It's totally fine to put the user links into the "secondary menu".

The only reason we retain the old custom menu named "secondary menu" is probably for D6 updates - can we just remove it from menu module?

admin toolbar is likely to be disabled in many install profiles. I don't think we shoudl consider that an option to fix the UX for normal users.

My opinion is that this sounds like a Bartik bug at this point, plus the task maybe to not install this confusing custom menu.

@pwolanin If it's a bug in Bartik it's a bug in Stark as well. See #17. I'm not sure which menu you're referring to with, "plus the task maybe to not install this confusing custom menu."

> It's totally fine to put the user links into the "secondary menu".

But then to get the menu coupling feature, you lose the convenience of the user links...

Well, Stark actually has no page template - it uses the one from system module.

So let me reiterate this way: if we are displaying the user links into the page element known as $vars['secondary_menu'], then all core page.tpl.php files should emit this at the upper right if that's what the generic core UX is supposed to entail.

I put the "User menu" block into the header region of both Bartik and Garland. In Stark the header region is at the top of the page (after site name and before the Primary nav). They appear as follows:

Bartik:

Garland:

Obviously both will need theming; however, while Garland's baked in header region does potentially support Utility links, I don't think the Bartik header region is true to the intention of the new menu created by #382890: Move 'My account, Help & Logout' links to separate menu and place it top right.

Comments from jensimmons on the intention for the header region would be much appreciated.

It seems like system/stark and Bartik ignored the changes that were put into Garland in #382890: Move 'My account, Help & Logout' links to separate menu and place it top right ?

#44 - Doesn't this still not solve the fundamental issue that coupled menus are miles apart - when the Primary Menu is set as source for the Secondary Links - only Garland makes sense in this configuration - is the plan just to live with this for D7 and admit defeat here - which seems odd since it would be rather trivial to fix this in both Bartik and core (core very easily solved).

re #57
Number 3 is the ugliest part (to me when I hit it)

System module's page.tpl.php also has this "print $secondary_menu in the footer"

system/page.tpl.php

    <div id="footer"><div class="section">
      <?php print theme('links__system_secondary_menu', array('links' => $secondary_menu, 'attributes' => array('id' => 'secondary-menu', 'class' => array('links', 'clearfix')), 'heading' => t('Secondary menu'))); ?>
      <?php print render($page['footer']); ?>
    </div></div> <!-- /.section, /#footer -->

The (meaningless) heading is hard-coded text in the theme! And that sort of function calling is out-of-place in a tpl.php
Granted, nobody is expected to actually use Stark, but we are expected to refer to it for best-practice base examples.

I still believe converting those variables to blocks is the proper solution.
Look at the patch and following discussion here.

Re #59, posted new issue #878772: Primary and Secondary links headings are hard coded

@dodazzi - above webchick indicates that such a conversion is not on the table for Drupal 7. I agree. Let's fix the broken Bartik/system page templates as David suggests.

As someone new to drupal I certainly was confused by the 'secondary links/secondary menu' terminology and it took a while to get it straight. But it is equally confusing and frustrating reading all the discussion here and under other related issues. It look like you are going to discuss yourself into doing nothing, at least in D7. Without knowing how difficult it might be it seems the appropriate thing to do are:

1) Rename 'secondary menu' in blocks/menus to something else relevant to the described function. I think the UI cue should stay there. I really don't think the described use for that menu qualifies as a secondary menu.

2) You already have 'main menu' and 'user menu' defined in blocks/menus. Place them by default in the header region. If people want a more 'purposeful' region, themers can add and change it.

3) Add styling to garland and bartik to make them horizontal for other themers to copy, or better yet add an option to blocks/menus so that any menu can be displayed horizontally or vertically.

4) The dynamic linked menus are really clever, but most people aren't going to use it out of the box. Drop it and ask the developer of menu_block to add it to his D7 version if it isn't already there. Since it seems that menu_block will be core in D8 it seems to be a reasonable feature request for that project. People who want that then just need to add the menu_block project to their install.

I don't know how hard it is to do these things but they do seem straightforward to me and #1 should easily be done before the door closes on D7. There have already been changes made that are going to bite some of the more or less dormant D7 themes out there (eg: changing highlight region to highlighted) so why worry about pushing these changes in this late. If you don't a lot of drupal developers are going to do it for you in a myriad of different ways.

ps. I find the distinction between 'main menu' and 'navigation menu' to also be confusing, especially when considering html5 terminology. You may end up discussing it for D8.

dcrocks - what you're really saying is remove the coupling feature from Drupal entirely and use some blocks in regions instead - this doesnt really work for many reasons. Sticking the user menu block in the header region is actually going to be very hard to support from a design perspective - not so much the problem of block placement but how to handle expanded menu items + the WTF that previously *header menus* had unique classes on each item, now they don't, unless we use theme_links, in which case we're right back to using the secondary_links and stick them in the header adjacent to the main_menu aka #18.

You're right, I don't understand the difficulties. But rename 'secondary menu' in blocks/menus anyways and get that issue off the table. Because it appears the rest may drag on.

Hopefully there will be some discussion on this over Drupalcon this week, in mean time I opened an issue for bartik as a proof of concept for moving the secondary links to the header - not saying this is what needs to happen, just a dialog on how it might work should this be the direction taken #889982: Move secondary links to the header in Bartik

Wow, entertaining read. Could someone please assign this to themselves and submit a patch.

I'd love to call this mine, but I'd want to remove things that apparently have reasons for being there that I don't know the history of, even after following these several threads.

For me I think that

      <?php print theme('links__system_secondary_menu', array('links' => $secondary_menu, 'attributes' => array('id' => 'secondary-menu', 'class' => array('links', 'clearfix')), 'heading' => t('Secondary menu'))); ?>

Should be deleted altogether from page.tpl.php as it's adequately replaced by just placing a menu block if you choose to want it and the current behavior is (I think) unwanted in the normal case.

But as we have theme('links__system_secondary_menu',..) at all, someone must think it made sense?

So as a discussion starting point, here's that patch. Just removes that hard-coded thing from the footer.
If this is a blunt and misguided attempt (which I suspect), please let me know what this breaks. Why is that hard-coded there? Then we'll have a direction to move forward from.

It doesn't solve all the 'what does secondary menu really mean' issues, but does at least remove some undesirable behavior

OK, I'll compare what garland is doing, which is apparently correct, and bring system/page.tpl.php into line with that.

@dman - I think we all agree that what you suggest in #68 is ideal, but it will have to wait for Drupal 8. For now we just need to make all the core themes behave as expected in terms of displaying the user links at the top.

Yes, I think thats a good start and what is proposed at this stage for Bartik (patch linked to in #66).

Yes, to be clear. Ultimately, we want to do the following:

1. Remove the one-off $primary_links / $secondary_links variables from page.tpl.php, in favour of using normal blocks like everything else on the page.

2. Remove the one-off menu configuration options for selecting primary/secondary links, in favour of Menu Block (or something like it) in core.

3. Fix themes so that they have a dedicated region for user links, and can handle getting random blocks tossed in there and format them properly.

All of these changes, however, are too much to do at this phase in the D7 release cycle. Drupal 8 material.

So what I'm taking out of this is that :
By decree, the page element called 'secondary nav' - whether or not it defaults to 'user menu' - which it currently does on a clean install (but may be changed)
- must appear in the expected position on the page which is, as per Garland - at the top, under the page element called 'primary nav'.

So - to make this happen.
The bad code has been pulled back into 'theme_preprocess_page' - as ALL themes in core use the same primary, secondary nave routine, and that code did not belong in several page.tpl.php files.

in Stark = system/page.tpl.php, the code has been removed, abstracted back to a page variable, then re-inserted in the top of the page where it's normal.
Special-case garland_preprocess_page code was removed

In Bartik = the code has been abstracted back into a page variable - which is good code.
- it has been MOVED TO THE TOP ... where right now it does NOT look good, compared to the expected theme behavious when Bartik was designed.

Patch rolling in a second ...

I agree, this shouldn't be added by default. However, if we now agree to delete in from core page.tpl.php, we also delete it from Bartik (as it is using core tpl.php.

We have three options:

1. Agree that it will be deleted from Bartik as well
2. Copy page.tpl.php from core to Bartik to leave it as is, in Bartik.
3. Add the block 'Secondary menu' to the Footer region by default.

It is now displaying the Account link and the Logoff link.
Pretty useful links, so I think we should keep them there in Bartik.

I'd suggest to go for the third option. Can we use a hook_block_info_alter() to add it to the footer region?

Didn't mean to remove the tag.

So this is the code change.
it spoils some appearance in Bartik.
But makes all the themes consistent in their treatment of this feature.
And reduces code and removes several WTF.

BUT needs some re-theming in Bartik...

I agree that we should be able to make that default user menu get placed and rendered somewhere nicely using available UI. I think it looked great where it was in Bartik, and it's a shame to mess with that.
BUT it achieved that result by behaving in a non-normal way - which is why we've had to spoil it.

Given that letting themes change block behavior is not going to happen - agreed it's an extension - we are compelled to pull Bartik back into line and be less wonderful.

I wish I could fix this a little more gracefully, but right now, this patch removes the worst of the WTF.

To clarify, your patch moves the secondary links menu below the primary links menu.
I've added a screenshot to clarify.

RE 78.
So I've done #2 and #5 in this patch. (consistent placement and usage, more sensible code)
The terminology renaming thing ( #3, #4 ) I'm not game to fix in the same go.

#1 - removing the concept altogether - is out of scope.

Right - you are 100% correct that even NOW, there is confusion between "Secondary LINKS" (which, I apologise- I thought you meant) and "Secondary MENU" - which currently is a naming WTF.
Even now, this caught me out.

Right now, it says:

Secondary menu
The Secondary menu is used on many sites for legal notices and contact pages.

... NO.
If it is this, call it the Footer Menu. Or ??
So yeah, it IS a really small label change.
REMOVE the similar label altogether.
Or ... remove this thing altogether unless someone wants to make it themselves.
I found that right now I could NOT rename, or delete, this currently empty and unused dummy menu. For me it's not just a waste of space, it's impossible to remove (as far as I can see). At the very least, it should not be a locked-down ummutable 'feature' we can't modify.

Yeah, so hopefully I can massage this layout badness out in CSS.

+1

Quick fix, great win:
- Change the name Secondary menu to Footer menu
- Change the default menu for Secondary links from User menu to Secondary menu
- Change Bartik theme to use the User menu instead of Secondary menu in the Footer

And I agree that Bartik still should display the Secondary Menu below the Main Menu by default.
But let's make another issue for that, as it touches the graphic design.

+++ themes/bartik/templates/page.tpl.php	22 Aug 2010 16:02:24 -0000
@@ -222,18 +212,6 @@
     <?php if ($page['footer'] || $secondary_menu): ?>

Need to remove || $secondary_menu

Otherwise all good. BTW - beyond this issue but when you're massaging Bartik note that we need to massage the header region in there proper somewhere also (not done and is already a known issue), probably going to be adjacent the logo/site name/slogan rather than below them - eg floating right with width maybe), just worth considering.

Powered by Dreditor.

Actually most discussions I've had regarding Bartik are to place the secondary links in the top right corner (LTR), and not below the main menu since that is really a different design and not at all in the original vision of the theme (two rows of navigation), but we really need to leave that to Jen. In fact the way the header region is designed at the moment is meant to facilitate that if you want to use the block (although the actual implimentation of it is borked).

FTR, the last thing I wanted was to get involved with changing anything about Bartik - which was fine. I don't really mind that it chooses to mis-use 'secondary links' as the footer menu, and If I could avoid requiring re-jigging and re-testing I would not have touched it.
But I was told it had to happen to bring things into line with Stark/Core/Expectations.
Jen is on it now. I will leave that for a separate issue . #889982: Move secondary links to the header in Bartik or therabouts

... trying to trace the testbot errors. I can't see what they have to do with my change - unless my patch touched something I didn't expect - which is possible - but LOOKS unrelated. Hm. Looking now.

Cleaner patch. Chew on that testbot.
Does NOT yet incorporate suggestions from #86.
Though I agree in principle.

Does NOT yet incorporate suggestions from #86.

And it never should, all those things have been hashed and re-hased then discounted in this already long discussion.

- the whole point of moving the secondary links to the top is to get the User Menu to the top
- Secondary menu should just be removed from core
- If a user wants a footer menu they can create one and stick it in the footer region, its up to the theme to style that how-they-may.

testbot, your problems with tabs2 are not my fault. Srsly. Try again.

OK if

Secondary menu should just be removed from core
- If a user wants a footer menu they can create one and

So can that be a separate issue? It's related, but not contingent on this. Lets split it off.

Lets have another crack at this.

Fixed some probs: dont remove tabs2 from garland, remove conditional for secondary_menu in bartiks footer.

Yay. 'Secondary Menu' system menu can just go away! Yes.

True, setting up $secondary_menu before just pre-rendering $secondary_nav might be avoidable now. Could be more concise to just do it in one go.
I didn't remove it when I looked at it because of the possibility that theme ports may expect that traditional behavior and want to render from there themselves. Most of the core themes did assume that half-way array would be present and available (until right now).

PS: Hugely Wonderful round-up of current theme behaviors David! It really helps give some background to this.

I'd like to help with troubleshooting and development if someone would explain how it's done.

Endre

Hi reviewing patch now - first thing that happened as soon as I applied on vanilla installation is this that the secondary menu popped up under the primary and disappeared from where it was supposed to be.

Looking into why but thought I'd post the screenshot already.

Ok now read the actual issue discussion and realised it got much more complicated than were I left it yesterday morning! So Bartik is broken but that is somehow expected...

My suggestion would be to get to a consistent minimum solution that does not break stuff. So if you touch Bartik simply remove the menu from the bottom and then let Jen sort it some other way.

If the discussion proves one thing is that secondary menu as a concept is broken because it opens the door to all these interpretations so +1 to remove it completely.

However, in removing one wrong assumption (secondary menu) let us try not to introduce another one (secondary links should be at the top). Let us allow each theme to interpret that - it is a design issue.

However, in removing one wrong assumption (secondary menu) let us try not to introduce another one (secondary links should be at the top). Let us allow each theme to interpret that - it is a design issue.

Not just a design issue.
There actually is (was) a direct semantic link between primary_nav (menu_main_menu, 'menu_main_links_source') and secondary_nav (menu_secondary_menu, 'menu_secondary_links_source')

Secondary links is (can be) the second level of main links.

As observed in #100, some themers were unaware of or ignored (or "interpreted") this special relationship between the two special link lists. It's not just yet another menu ... though it can be made to behave like one. And plenty of developers have never done anything else.
In D6, this relationship was tight and the default behavior. In D7, this relationship is NOT the default behavior, so the special semantic link - though still there, is more tenuous.

... What I'm trying to say is that "(secondary links should be at the top)" is not a new or incorrect assumption. It's a semantic fact.
Removing or generalizing this extra-special behavior is an OK D8 change request, but not going to happen right now.
But while there is a 'secondary links' concept, it's best that it remains somewhat linked to the main menu on the page.

And what if we added another variable (we now have $primary_nav and $secondary_nav) called $user_nav? Which we can put in place of the $secondary_menu in the footer?

Then we only have to change that the Secondary Menu settings in admin/structure/menu/settings doesn't point to User menu, but to No secondary links.

If we do: we don't have to make visual design changes to Bartik and Bartik still keeps working. Right?

#107
Fine idea (at the risk of multiplying special cases)
At issue is the conflation of "secondary links" and "user menu" which is what was introduced and led to the mis-use/misplacement of user-secondary-links.

Somehow I don't think introducing a new feature (page variable) for this is going to happen right now though.

A big move in the right direction - and one that would restore a lot of expected and desired behavior -
Just place the user_menu block in the footer header automatically!
I'm trying it, looks just like it should, though can be subtly themed better.

The difference between our special case 'links' lists and a menu block is just CSS.

I'll look at what that will take. Just as an EG

Same patch as #101, but now added the variable $user_nav and placed it into the footer of Bartik.
I still have to set the default Secondary menu to 'No secondary menu'.

Waaaaaaaaaiiiitt? I thought we had a plan, which was to simply fix Bartik (and Stark) so that the secondary nav showed underneath the primary nav like the majority of core/contrib themes.

Why are we adding additional $user_nav variables and whatnot? I thought that was ruled out several hundred posts ago?

Not convinced that

$variables['user_menu']         = menu_user_menu();

Is a step forward.

The only reason I even got bugged by this issue is because I couldn't go and manage that user menu in the normal, block-admin way.
The fact that a special-case was explicitly being inserted into core page-tpl.php when block/region admin is perfectly capable of doing so was my WTF.

If I can find a way to replicate the element-hidden behavior for the menu title, I'd put this menu, as a block, into the install profile.

... Apologize if that approach is just not going to happen for code lockdown reasons, I'm just exploring ways to solve the actual problem. Other fixes can still be discussed. Obviously, the less original, the better!

I think that's still needed. But Bartik uses the User links in the footer. If we place the Secondary links below the Primary links, we still have to have away to keep the User links in the footer. But placing the Secondary links below the Primary links involves GUI changes. We'd need another issue for that, right?

Another patch, #110 with Secondary menu set to 'No secondary menu' by default. If we agree to this, we might need to have an hook_update too for existing sites.

#110 with Secondary menu set to 'No secondary menu' by default.

I agree it should not be 'user menu', that dual-purposing is what put us wrong everywhere.

What's wrong with 'main menu' inheritance al-la D6? ... Yeah, I'm sure that decision was made with much discussion long ago when user-menu first went in there.... I'll go look for that thread I guess.

Probably no hook_update needed, as it's only a variable_set default. yah? Any site that has ever visited the menu management page will already have that preference set. Any pre-existing site doesn't want us to put through a change that would override their choice so far.

@David_Rothstein - awesome theme roundup and review.

@102 - clean install and the secondary menu is nuke, good stuff. Note to reviewers, you have to run a clean install otherwise the secondary menu will still be there even after running the patch.

@105 - Its a UX issue also, not just a design issue - and for core we must achieve high standards for both. The current patch breaks Bartik - correct - the task now is to test and improve on the patch until its RTBC.

OK, so building on the discussion here this patch implements:

1) the suggestion by David and dman to simply define the $main_menu and $secondary_menu variables in one hit and depreates the use of primary_nav and secondary_nav

2) moves the $secondary_menu to the top of the source order in Bartik and adds some css for it so we don't have a reviewer WTF. Can't say I'm fussed on the new source order here, but afaict the only alternative would be a somewhat major redesign or, gasp, position: absolute;, attempting to float this in there could get real tricky when we start factoring in the header-region.

Some words on scope might not go astray here - are we looking to fix this in one giant patch - meaning the CSS for Bartik etc, this patch assume so, at least to the extent its not broken.

Should we be thinking about upgrade path - there will be many sites out there that are using the secondary menu in D6, what might happen here?

Waaaaaaaaaiiiitt? I thought we had a plan, which was to simply fix Bartik (and Stark) so that the secondary nav showed underneath the primary nav like the majority of core/contrib themes.

Why are we adding additional $user_nav variables and whatnot? I thought that was ruled out several hundred posts ago?

Yes, exactly - washed and done, its the route forward - except that it "below the primary nav" is to narrow whereas "top corner" is better, or even "somewhere up the top", meaning in the header - putting these below the primary nav in Bartik will look bad and need a major redesign. The top right corner for LTR makes a lot of sense imo.

But placing the Secondary links below the Primary links involves GUI changes.

Yes, exactly, see the above patch. The user menu in footer is a UX WTF.

- No new variables such as user_menu - we're trying to go the other way and get everything into blocks in D8, this is not the way forward.
- gui changes are part of the scope (they just have to be).

Spun off #890708: add user_menu block to the header region in default install profile as just discussed with webchick.

Latest word.

** We will NOT touch/fix bartik in this issue, break it out into it's own issue. Slightly inconsistent, but non-critical.

** "Secondary menu" - DIE.

** Fix for system page.tpl.php and code clean-up - DO IT IN THIS NOW- me

** $user_menu NO WAY, no more new page variables and special case.

** Follow-on issue (joachim) see about the placement of user_menu

Re-roll of this, without Baris' initiatives, forthcoming from me in a second

@117 - throwing a major spanner in the works here, need clarification from the code sprinters, this really changes things up, especially for Garland which no can say looks good with a block in the header region.

@118 - OK, good, fix Bartik UI later - just update the variable names and placement ala your first patch.

So, LESS AMBITIOUS patch.
JUST doing the critical blockers as discussed with webchick.

Remove dud secondary menu
Move secondary links to be paired with main links in the top, not the footer in core page.tpl.php
Pre-cook the links lists in theme.inc, not with cruddy code in tpl.php

THAT IS ALL.

Text changes that were in an earlier patch are NOT CRITICAL, but whoever put them in can improve docs is a non-critical issue.
Bartik is untouched, so leave that for another issue.
'user menu 'links' vs block, a spin-off issue. #890708: add user_menu block to the header region in default install profile
- for now existing behavior remains for that

I don't know where the text in menu.admin.inc seen in #115 came from.
It may well be great but I didn't include it in the patch until wording had been batted about and reviewed directly.
ronald_istos is going to do a new issue to review that text (sprinting together)

+++ includes/theme.inc	23 Aug 2010 10:29:31 -0000
@@ -2266,6 +2266,39 @@ function template_preprocess_page(&$vari
+    $variables['primary_nav'] = theme('links__system_main_menu', array(

This adds extra nomenclature with $foo_nav variables... is this less confusing that $primary_links, which at least has history?

Other than that, looks good :)

Powered by Dreditor.

Agree with joachim, Dries has bashed me on several occasions for using variables that are shortages ("nav" vrs "navigation"), return to primary_links could make sense here.

yeah, hm. Unfortunately I based this re-roll off #113/115 - which introduced a huge amount of renaming, so I had to undo a bit. I do not want to re-purpose a variable name that used to return an array into one that now returns a string - that would have hurt.

Existing $primary_links is an array of links.
Thus I would expect anything that copies that naming to provide the same sort of data.

$primary_nav instead is the themed string. If it were the same name as D6, we would expect the same behavior. I think.

*_links should be an array of links.

And what about just $primary_navigation and $secondary_navigation?

ekkkkk, bike-shedding on variable naming - lol. Well I kinda recall JohnAlbin doing some research in this and from what I recall I think he came up with $main_navigation and $secondary_navigation (with regards to a wide number of webmasters understanding what these might be).

$*_links is always going to be an array of links.
To this end, we will NOT use $main_menu and $secondary_menu as var names to store that link array. They are recently-introduced and inaccurate variable names.
Joachim and I looked at issues from catch and David who both say we should do exactly this. (This proposal didn't happen because it got attached to menu_block instead, so this small issue was lost)
Everything that David said here is exactly what Joachim and I have agreed to independently.

These variables will be $primary_links and $secondary_links. Because in NO CASE is there a garanteed relationship between "* menu" (a UI concept) and the "*_menu" A theme variable name. The fact that both existed and didn't always match was a cause for confusion.

$primary_navigation and $secondary_navigation will be the themed strings that theme use and can just print().
Nice long, accurate var names.

(BTW, $primary_nav, $secondary_nav appears to be a left-over - or actually a precedent set by Garland)

- A follow-on issue is the renaming of the class (still currently called 'secondary-menu' !!) because that needs to be fixed with CSS implications. This patch is the same as above with internal-only varnames changed.

(thus I do need to touch bartik - but only by renaming a variable - no actual change.

Oh my, this is so out of hand. There are several issues open and several different people all working on different solutions at the same time. We really need to stop all the chatter and brainstorming and just do one thing.

This is what we are doing:

Moving the secondary links to the header in Bartik. And theming it.

That is all.

API documentation:

• Page template variable $main_menu is not used (if it ever was, seems to be a recent thing). Instead use $primary_links, which is an array of links that need to be run through theme_link() if you want to display them. (But that's for backwards familiarity only, better to print $primary_navigation instead
• Page template variable $secondary_menu is not used. Instead use $secondary_links, which is an array of links that need to be run through theme_link() if you want to display them
• Page template variable $primary_nav (as seen in Garland only) is renamed to $primary_navigation . It is the themed string to be used and printed on the page in page.tpl.php.
• Page template variable $secondary_nav (as seen in Garland only) is renamed to $secondary_navigation . It is the themed string to be used and printed on the page in page.tpl.php. Preferably pretty close to $primary_navigation, as these menus may be expected to be paired.

I wouldn't have tried to rename stuff like this without catch, David, Joachim, and now BarisW, Jeff and JohnAlbin? saying this is a better name

(PS, We are really trying not to touch Bartik here, as that's not the critical part. Bartik inconsistency is just a Bartik inconsistency. This patch repairs correct behavior on system/page.tpl.php which is currently a blocker. The small Bartik fix can be in another issue, now the core page.tpl.php becomes a better example to work on. Bartiks mistake was based on this core mistake.)

> Moving the secondary links to the header in Bartik. And theming it.

Yes, but not in this issue. Needs a new issue opening, AFAIK.

+1 to #127 - lets send this for review.

OK, short roundup to try and clarify what is going on here since this is hard to follow.

1) fixing the critical hard issue of cores page.tpl.php (see the API docs in #129)

2) Gives us option of a easy printing variables for page.tpl.php - $primary_navigation and $secondary_navigation

3) #890708 proposes to enable the user menu block in header region by default (for a "standard install") and set the source for secondary_links as the primary_menu, so it has a source since the secondary menu is nuked by #127.

4) Garland and Bartik will need to clean up any mess themselves.

Is this all correct dman/joachim?

@131

I think that's all correct except for I think #890708: add user_menu block to the header region in default install profile needs to clean up any visual mess it adds. See my comment there.

1. yes.
2. yes, better code.
3. something like that. Joachim is finding the right place in install profiles to slipstream this.
4. Garland and Bartik are currently no worse off, so I'm not introducing any breakage (like my first rough attempt did). Garland has always done it right, it invented the concept. Bartik can improve in its own time, as needed, probably after the block gets positioned in #890708: add user_menu block to the header region in default install profile - if that ends up OK .

So yeah, that understanding is 100% correct.

(testbot, don't think that status change was intentional)

Seems good!

I've changed the documentation in page.tpl.php to match the new variables.

:-(
Patch #134 removed some required changes that were in #127.
Plus mine missed a variable rename, testbot is correct.

I'm running selective tests locally too, but they do take several minutes hours each run-through, it's more fun to let testbot do it. Try this version, only a couple of variable renames - including replacing the ones removed by #134, but keeping the docblock change

(Passed locally, FWIW, testbot, pull finger!)

+++ modules/system/page.tpl.php	23 Aug 2010 13:52:21 -0000
@@ -27,10 +27,14 @@
+ * - $primary_links (array): An array containing the Main menu links that can ¶

Unneeded whitespace.

Otherwise the code looks good. Can't judge the high-level approach, though.

Powered by Dreditor.

Don't know why my patch left your changes out, apologies for that.

So are we not changing the default secondary menu to something else than 'User menu'?
I only see an unneeded space after the description of $primary_links.

Furthermore, it's looking great. Let's get this committed :)

> So are we not changing the default secondary menu to something else than 'User menu'?

Happens in #890708: add user_menu block to the header region in default install profile.

+++ includes/theme.inc	23 Aug 2010 13:52:21 -0000
@@ -2258,14 +2258,47 @@ function template_preprocess_page(&$vari
-  $variables['main_menu']         = theme_get_setting('toggle_main_menu') ? menu_main_menu() : array();
-  $variables['secondary_menu']    = theme_get_setting('toggle_secondary_menu') ? menu_secondary_menu() : array();
+  $variables['primary_links']     = theme_get_setting('toggle_main_menu') ? menu_main_menu() : array();
+  $variables['secondary_links']   = theme_get_setting('toggle_secondary_menu') ? menu_secondary_menu() : array();

Sorry, but we're not doing this minor cosmetic clean-up. It's going to be a year after API freeze next Wednesday, and this would force everyone who's upgraded their theme to D7 at this point to make this change.

Furthermore, going back into CVS annotate it looks like this was a deliberate change, and one that's been in D7 for over 2 years: http://drupal.org/node/270917

+++ modules/system/page.tpl.php	23 Aug 2010 13:52:21 -0000
@@ -102,9 +106,10 @@
-    <?php if ($main_menu): ?>
+    <?php if ($primary_navigation || $secondary_navigation): ?>

Same with this. It's a "nice to have", it breaks APIs, and does not fix this critical bug. Drupal 8.

Let's go back to just removing the "Secondary menu" from the default profile, and just moving the location of the Stark links. Everything else looks like it's out of scope.

Powered by Dreditor.

One thing we did discuss though is the labels for "Source for the Main links" and "Source for the Secondary links" still have the legacy "links" in their labels, and should be updated to menu. SEPARATE ISSUE PLEASE. :)

Cutting it all back to what webchick says is the 'critical' part, that is version #120, before the 'renaming' and without any code fixes,
So it re-inserts this PHP block

 print theme('links__system_secondary_menu', 
array(
  'links' => $secondary_menu, 
  'attributes' => 
    array(
      'id' => 'secondary-menu', 
      'class' => array('links', 'clearfix')
    ), 
  'heading' => t('Secondary menu')
  )
);

back into core page.tpl.php
... but at the top this time.

It reverts the name fix so that garland still calls its variables as $secondary_nav, not $secondary_navigation.

It does not touch the inconsistent names used in theme.inc

So now, in the UI, the thing called "Secondary Menu" can be set to something that is not "secondary menu.
The theme page variable calls that $secondary_menu (though it's actually an arbitrary array of links).
And places a hard-coded text title above it labeling it "Secondary menu" .. even if it is (as is still the default) actually the user menu ... and certainly not really wanting a h2.

But it's much nicer and shorter :-)

bot

Tested the patch in #144 on a fresh default install and everything works as expected.

The theme layer stuff looks good - no problems there.

David: Something like that, yes. ;) I apologize for derailing this issue from your original simplified vision, but I think this discussion has been important to hash out the sub-sub problems and rule specific solutions in/out.

Since Jeff Burnz approves the tpl.php changes, Committed to HEAD. Dear sweet lord, what an EPIC journey. ;)

Epic journey yes. Too much enthusiasm from many parties to make things better overshadowed getting the critical off the table... which was the main point.
Yes, the eventual patch is the minimal version of earlier attempts. :-B

All input was good, and valid as it was. But developed into diversions.

We kept getting pulled back into (David_Rothstein's point, and Joachims, and me to) that calling non-menus 'menu' variables was not nice and was fixable.

As I illustrated, above, #878772: Primary and Secondary links headings are hard coded is the UI WTF that brought this to my attention in the first place, and I wish it were fixable, but without the pre-render code consolidation that we pulled back out of #137, it's not able/going to be fixed in a central place, it's back to individual themes to not do what core system/page.tpl does.

So guys - do not take system/page.tpl.php as a model of good practice! Just accept that it's got some legacy crud in it.

But Phew!

Should it still be critical?

No that should be major. I totally do not get what we done here :P, we changed the usable notion of a user menu to undescriptive secondary menu again? Seems like we traded one trade off, for the other?

Trying to make sense of the major queue currently. Documentation-only follow-ups are normal, as long as they do not affect string freeze.

changing tagging scheme used for updates needed on the module/theme update pages

Spillz - did you post to the wrong thread? This has nothing to do with deleting menu items or content types. That's why it wasn't tagged against the menu system. It's about theming and the idea of 'secondary menu' behavior

If you have a new problem, open a new thread. This one is done.

This is for V6.x but it may work for D7...have no way to test it...

I wanted to display the Secondary menu only to Authorized Users (you can just as easily hide it all together) and posted the solution that worked for me today at: http://drupal.org/comment/reply/1012556/3890160

Good luck

Please don't change the version on an issue. The Version indicates what Drupal version the issue is being worked on. We have to fix it in 7.x before the fix can be ported to 6.x.

This was tagged as needing update documentation in #151.

However, looking at the patch (I didn't read all 150+ comments on this issue!!!) I don't understand what was changed that needs documenting in the Theme Update Guide.

Can someone please post a summary of the issue, or better yet write a section for the theme update guide and post it here?

Since no one has provided any information on what needs to be documented in the update docs, I'm going to try another tactic and ask that the people involved in this issue make a change record node instead. I'm adding a note to the module/theme update pages for d7 to also look at the change list page, because there are already a few d6/7 changes there anyway.

Seems a reasonable tag to add as per DrupalWTFs, since it is advertised in 7.x help text but not working.
Related issue: #950034: Using the same source for main/secondary with a custom menu doesn't work

Issue Summary Standards, edit by minneapolisdan