I've created a private site where content is not available to anonymous users and registration is restricted. I've managed to get all the content to not show to the anonymous user (a couple of modules don't have the required settings, so I've had to disable those blocks - but that's for another day).

Anyway, 'edit primary links' is showing to the general public since they can't access the menu. It also appears that this is hardwired into an include file to show this link if no primary menu items are available to view. Granted the link goes to an Access Denied page, but the link itself is not appropriate content to show at all to the general public under any circumstances. This link should only be visible and/or available to the administrator.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

macgirvin’s picture

Small addition - shouldn't it be permissable for a site to not have any primary links? It seems impossible to do this. The 'edit primary links' hack may be useful for a brand new site administrator, but perhaps I'm questioning it's right to exist at all.

greggles’s picture

I tried to remove that message by deleting the "Primary Links" Menu item and that had no effect.

As macgirvin states, this is hard coded on lines 783-786 of includes/menu.inc Removing those lines seems to work fine. I tested removing them without any content in the "Primary Links" menu item and it went away. Then when you add content to the "Primary Links" menu that content will show up as expected.

I'm not sure whether those lines should be removed and the functionality of "add an edit primary links if no links exist" should be added somewhere else.

macgirvin’s picture

Status: Active » Needs review

I ended up disabling those lines (includes/menu.inc, 783-786) as well, but I don't really like to run with hacked core code unless there aren't any other options. Upgrading becomes a royal pain, and we're not even at beta3 yet.

So anyway, I'd like to suggest changing those lines to the following:

  // Special case - provide link to admin/menu if primary links is empty.
  if (is_null($links) && $start_level == 1 && $pid == variable_get('menu_primary_menu', 0) && user_access('administer menu')) {
    $links['1-1'] = l(t('edit primary links'),'admin/menu');
  }

By adding the permission check for 'administer menu' I believe this achieves all of the desired results. Anonymous users don't ever see the offending link. Only those who actually have the ability to make changes to the menu system will ever see it. Does that sound like a workable compromise?

macgirvin’s picture

Priority: Critical » Normal
killes@www.drop.org’s picture

Status: Needs review » Active

Please attach a proper patch.

macgirvin’s picture

Status: Active » Needs review
FileSize
517 bytes
macgirvin’s picture

> Please attach a proper patch

Sigh... locate original drupal package. Backup the entire live site so I can overwrite the working code with non-working code. Grab the menu file from the archive. Do a diff. Restore the live website. Open Filezilla so I can transfer it through the firewall to the client machine, oops, first move it to a location that's accessible to Filezilla. Transfer to the machine that has a web browser. Yay. Now find the message thread again, and upload it to drupal.org. ...For a one liner.

Ya' happy?

jvandyk’s picture

Status: Needs review » Reviewed & tested by the community

macgirvin, next time please sign your patch with an appropriate digital cryptographic signature. j/k :)

tangent’s picture

Perhaps I'm being nitpicky or perhaps I'm not but I would have put user_access('administer menu') as the first test.

tangent’s picture

Strike that. user_access() is probably more expensive than the previous tests.

Dries’s picture

Status: Reviewed & tested by the community » Closed (works as designed)

I don't think this is a problem per se. These links are meant to be temporary and are easy to remove/disable.

tangent’s picture

Status: Closed (works as designed) » Active

It is a problem if access control is used.

1. Create primary links for "forums" and "image galleries".
2. Remove "access content" from the anonymous role.
3. Visit the site while logged out and in the primary links area you will see "edit primary links" which links to an admin page which the anonymous user does not have access to.

greggles’s picture

I followed the steps that tangent laid out and agree with his point. For a site that is primarily a private site, this could lead to confusing results.

The patch applied and worked fine for me.

mfb’s picture

Version: 4.7.0-beta2 » 4.7.0-beta3
Component: base system » menu system
Status: Active » Reviewed & tested by the community

This is a common scenario for e.g. a corporate intranet drupal site. Anonymous users don't have access to anything in the navigation menu, which is therefore empty, Primary links are set to use navigation menu. So the "edit primary" link appears, which really should not be available to users who don't have access to it. This simple patch does the trick.

Dries’s picture

I don't see how a check for 'administer menus' avoids the problem tangent describes. Please elaborate, or mark it 'by design' again.

macgirvin’s picture

The bug is that 'edit primary links' is shown to the wrong audience. In this case, it is an anonymous browser for whom all content (including existing menu items) has been disallowed. This makes it a critical bug for a site with protected content. The fix is to only show the link to the proper audience, that is anybody who has the ability to traverse the link and see a menu edit page.

Dries’s picture

If you don't use the primary menus, there must be a way to get rid of them so that the link isn't visible to administrator's either.

tangent’s picture

In the case I described, the primary menu is used. It is populated with menu items so the admin will not see a "edit primary links" message. The problem conditions are these.

1. Primary links menu is populated with links to modules like "image", "blog", and "forum"
2. At least one role has no "access content" permission

Links to those modules above disappear (rightly so in my opinion) from the menu if the the user does not have "access content" permission. This leaves the menu empty and the menu shows a "edit primary links" message if empty.

There are workarounds for this problem but the fact is that message should not be displayed to any user without the permission to modify the menu so the patch solution is quite appropriate. Forcing admins to do workarounds for things like this doesn't seem like a user-friendly approach.

macgirvin’s picture

Dries:
If you don't use the primary menus, there must be a way to get rid of them so that the link isn't visible to administrator's either.

I agree (as I noted in followup #1). There doesn't appear to be any way of doing this currently. The only realistic solution would be to remove this block of code, since there is no easy way to ascertain that the administrator has no desire for primary menus vs. he/she just hasn't gotten around to creating them yet...

I don't claim to have the authority to make that sort of executive decision (abolish the feature).

Adding yet another option to allow the site to not have any primary links and also not have a reminder link seems like a waste of effort for marginal/questionable gain.

The patch for permission checks seemed like the cleanest way to move forward. But I would have no problem with abolishing this section of logic entirely if such a decision was deemed appropriate.

greggles’s picture

As a workaround I tried to delete the "Primary Links" menu item in admin > menus. That change had almost the exact opposite effect to what I thought: the 'edit primary links' shows on every page for everyone (certainly not the desired/intended behavior).

In trying to find a workaround I also looked in Blocks and Themes. There is no Block associated with this section of text and there is no configuration item in the theme that would remove it. Of course it can be removed in creating your own theme - but the point is that the default themes should have reasonable behavior.

I'm not sure if this means that the whole thing should be reimplemented in a different way or just removed or...some other solution.

Chris Johnson’s picture

This "feature" clearly annoys a lot of people -- including me. If it doesn't work right, why not delete it completely until someone codes it to correctly not display that link to anybody, including an administrator, when it is unwanted.

greggles’s picture

Well, the "why not just remove this" is a big usability issue. Lots of first time users are very happy to see that link, click on it, discover where to change those and then they remember them forever.

If someone has a good idea how to fix this I'd be happy to try to implement it.

Crell’s picture

I've not been following this thread closely, but wouldn't the quick and easy way simply be a checkbox on the menu admin page where one selects the primary and secondary links? Uncheck "Use Primary Links" and the system skips over generating links in the first place, default message or otherwise.

macgirvin’s picture

In fact I just discovered quite by accident... instead of going to administer|menus, I clicked the wrong menus link and ended up had at administer|settings|menus, and there is indeed a selector for what menu to use for primary/(secondary) links or none at all. I selected none and the menu went away. No edit primary links, no nothing.

Don't know how long this settings page has existed, can't recall ever seeing it before. Maybe it's been there all along. There's still a permission bug if you disallow content access and do have primary links established (for some group of users who can access content). So the patch is still valid in that case. But I will drop the issue of being able to turn off primary links completely and not get a primary menu or the 'edit primary links' link. You can.

greggles’s picture

Category: bug » support
Status: Reviewed & tested by the community » Closed (works as designed)

Well...now I agree that this is a documentation problem and is a "works as designed" situation. Thanks macgirvin for finding that workaround and pointing it out - it is helpful to me for sure.

The default "edit primary links" item is good to keep as a way to help with the "discoverability" of the primary/secondary links and where to edit them.

I believe the best way to handle this is to leave the code as is and just remember the problem and point people to the "admin/settings/menus and no primary links" workaround when they encounter this problem.

mgcarley’s picture

Version: 4.7.0-beta3 » 4.7.0-beta6

Having gone through the posts here, I am having difficulty on a fresh install of Drupal 4.7.0-b6, where the primary links menu shows "edit primary links" even if there is a menu item with "Primary Links" as the parent... am I missing something?

greggles’s picture

mgcarley - I'd say that you're missing a full bug report because I can't understand whether your comment is a a continuation about this same bug, or a report of a different related bug.

This bug is currently marked "by design" meaning that it's effectively closed and nobody is going to do anything about it. If you disagree with that decision you need to present a convincing case of why you feel that way.

If your comment is about a new bug, I recommend you open a new issue.

sambeckett’s picture

ok im a really new user. I just learned about drupal, downloaded and installed it all in 20 minutes.

then it took another 20 minutes to get rid of this stupid link in the header.

this 'by design' is bad design.

I almost spent another 2 seconds to rm -rf drupal because of it.

sime’s picture

Once you figure out how to work with Drupal themes, you shouldn't have any trouble. Here are two hypothetical ways to remove it:

Add this to style.css
div#primary {display:none;}

Remove this from page.tpl.php:
<?php if (isset($primary_links)) { ?><div id="primary"><?php print theme('links', $primary_links) ?>

jonty_q’s picture

Version: 4.7.0-beta6 » 4.7.0

macgirvin and greggles .... Aaaaagh!
Come on guys, your 'discovered' solution is not the solution at all!

You mentioned the use case yourselves:
Private site.
Anonymous users see _no content_ !! <--- and don't forget I am an 'anonymous' user everytime I return after logging out.
Logged in users see _Primary Links_ menu!

But, anonymous users also see _primary Links_ - but because no content available, it show the irritating "edit primary links" .
Therefore, you are saying the solution is to not use a primary links menu for your authenticated users???
Come on! I don't think so! Please guys - we need that patch - do a quick scan of the forums to see how many people are having that same annoyance!

jonty_q’s picture

HOw do I edit my previous 'follow up' ?? I think it interpreted <-- as a comment ... or maybe just a bad tag ...

jonty_q’s picture

So as I was saying ...

Site:

  • Totally private
  • Anonymous users hitting front page have no access to content
  • Logged in users see primary links menu with my beautifully thought out menu structure ;-)

Problem is, anonymous users hitting front page still see "edit primary links".
Don't forget, every time I return to the site after logging out, I also am initially an 'anonymous' user!

SO you're saying, the solution is to not use a Primary menu ???

Come on guys! This is not a solution. Just a have a trawl through the forum to see how many other people are being afflicted by this problem - we need that patch! Please re-open the issue!

No wonder sambeckett deleted drupal if these issues get assigned "By Design".

greggles’s picture

Category: support » bug
Status: Closed (works as designed) » Active

jonty_q - After only 3 comments and a fair amount of insults you finally made a set of steps and a use case that shows a bug. That is a nice first step. Do you have a patch, perhaps? Or at least a plan of how it should be fixed?

jonty_q’s picture

Sorry if you took them as insults - take them as frustration ... hopefully I am conveying not just mine, but that of many people ... ...Ok in future I'll try not to get my late night stressed out emotions get into the posts :-P

You agree I have a use-case that shows the need for a patch - that is great, thanks for acknowledging! Though you had already discussed that use case early in your bug discussion I thought ...

I was under the impression of reading this issue that you had come up with a patch, but then decided not to go ahead and put it in. Maybe I need to re-read it ... won't that patch do the trick?

I'm sorry, I'm not a PHP programmer at all ... C++ yes, but haven't started to learn PHP at this stage ... so as I already said, I thought you guys had come up with a patch, and just needed to put it in ... my apologies if this is not the case. My only contribution is to point out the definite need for a patch ... sorry I can't help out in fixing it.

ANyone else? I can help out afterwards by testing it, I hope ... don't even know if I need a PHP dev. environment set up or what ... I'm not running a *nix at home and my hoster is a wholsaler on-seller, ie you get a control panel and FTP that's it thing.

rootwork’s picture

I agree with jointy_q and have exactly the situation described. A site that has primary links for logged in users but has no content access for anonymous users, thus they see "edit primary links" tab. Removing primary links entirely obviously isn't the solution.

It's amazing to me that after six months of comments no one has fixed this. I am not a competent PHP programmer and I think it's pretty frustrating that when jointy_q and others have brought up this frustration expressed by many (many many many) people they are met with "stop insulting us, if you care so much write a patch."

I don't know enough to write a patch but I know enough to see that this is a problem. I imagine it was simply overlooked when it was designed this way. But now it has been brought up and I hope someone with the skills to write the patch will be kind enough to do it.

rootwork’s picture

In case anyone else is following this thread or discovered it (as I did) through searching, I ended up using the conditional statement suggested by karldied here: http://drupal.org/node/57367#comment-139072

I still think this is an issue that should be patched, however.

greggles’s picture

ivan.boothe - that only solves the problem in one of the themes. You'd have to patch every theme with that solution in order for it to work.

For getting a bug fixed sooner, there are more efficient ways: http://drupal.org/node/73178

Note that complaining about it not being fixed is noticeably absent from the list.

macgirvin’s picture

I just got an email from the new(?) bug reminder system that this bug is still open. If anybody still cares about it, you'll have to take it on yourself. I fixed this problem by dumping Drupal and writing my own CMS that does exactly what I want. It's been deployed on all my web properties for months now. Why did I do such a crazy thing? I've got a community system that works. All these issues were fixed long ago. Looks like all the Drupal critical issues I cared about deeply and couldn't live with are still being discussed and debated. None of them have been fixed.

Case closed.

butler’s picture

As per (/node/42826) comment #26, I am also having the problem that, though I have created primary links, unauthenticated users are still shown the "edit primary links" link. The site in question is divided into a public section for unauthenticated users and a private section for authenticated users. in the public area "edit primary links" shows up on every page. So in this case, not all content access is denied.

Possibly relevant details:

Drupal 4.7 bluemarine theme

under admin/settings/menu-

-Menu containing primary links: Navigation
-Menu containing secondary links: Navigation
-Restrict parent items to: Primary Links

... under admin/block I have navigation and primary links disabled and am using the nice_menus module to display primary links.

I think the problems detailed above and here in my comment are probably too similar to be unrelated (it seems clear that the point here is that the system is able to react as though there are no primary links when there are, which is true in both cases), so am posting here. If someone thinks I should start a new report I will be happy to do so but in that case I would still think this could be relevant here-it is true that this issue is in discussion in one way or another all over the Drupal site, and it may also be true that that is why it is still an issue. As a newbie I have spent most of my time trying to figure out how navigation should/could work on a Drupal site.

Though I am not the person to do this, perhaps the title of this report should be changed to something more descriptive/inclusive?

mfb’s picture

Version: 4.7.0 » x.y.z
Status: Active » Needs review
FileSize
816 bytes

updated patch for HEAD

hickory’s picture

mfb's patch above is perfect.

jvandyk’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
731 bytes

This is a very simple bug.

Currently we show "edit primary links" link to users that do not have the proper access to edit primary links. That's silly.

With this patch, we only show the link to users who can actually edit primary links.

macgirvin’s picture

Yep - that's the same patch I submitted back in December last year. I didn't think it was a big deal either. The issue seems to be that creating restricted websites isn't a supported feature of Drupal. The focus is purely on public content. I've got a 'members only' adult site or three where the content is completely protected from anonymous access - plus a kids only site for my 9-year old daughter's friends where I approve all the members, and likewise has no content that is anonymously accessible. I am protecting each site from age innapropriate audiences and the default state of the website is protected or closed to public access. (This is in addition to twenty or thirty public websites.)

The patch in question solves the problem now just as it did last December.

If you are travelling this path, be aware that there are many other content bugs which will bite you and the website descriptions I provided above can not be implemented with Drupal 'off the shelf'. Many 'blocks' do not honor the content access settings. Until the Drupal community at large accepts restricted websites as a supported configuration, you will have to apply hacks to several modules and core files (and maintain those changes on your own). It is a lonely road.

Morbus Iff’s picture

+1 to #42. Let's get this in.

Darren Oh’s picture

I'm surprised the patch in comment #6 wasn't committed months ago. This patch solves a real problem without creating more problems. Anyone who doesn't want to read the entire thread can find a clear summary of why this patch is necessary in comment #32. The patches in comments #40 and #42 are identical; one of them should be committed immediately.

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

mfb’s picture

Thanks!

killes@www.drop.org’s picture

backported

Anonymous’s picture

Status: Fixed » Closed (fixed)