I just upgraded from 7.x-3.0-rc2 to 7.x-3.0-rc3 and now the admin menu is showing at the bottom on an Omega subtheme. It's showing fine (at the top) with Seven.

Comments

okokokok’s picture

Same with with 7.x-3.x-dev (and many cache clears)

okokokok’s picture

Priority: Normal » Minor

Things went back to normal after I checked "Keep menu at top of page", reloaded a page, unchecked "Keep menu at top of page" and reloaded a page.

Not sure if this needs follow up but I guess this could happen to more people.

okokokok’s picture

Version: 7.x-3.0-rc3 » 7.x-3.x-dev

Now suddenly it started showing the admin menu slightly below the top.

I turned Keep menu at top of page on and off, and it's still doing this.

So I'll just leave Keep menu at top of page on for now.

sun’s picture

Did you run update.php?

markhalliwell’s picture

Yes, update.php is clean. I think it has to do with the page_bottom region's classes somehow including the 'contextual-links-region' class?

Also would like to reference the following issues:
#1309660: Ensure $title_suffix is always rendered (i.e. fix contextual links)
#1493210: Contextual links added to page element if 'page' view is displayed
#1170428: Contextual page links behavior causes gap above admin_menu on Seven theme

dcyuri7’s picture

just wanted to confirm Mark's post above, that the [contextual-links-region] class assigned to the document 'body' is indeed the problem. Remove it using the Firefox/Chrome html inspection tool, and problem instantly is resolved. So tracing this back....

Modules:: Context ->Skinr (..etc..) -> Skinr UI module
I resolved by disabling Skinr UI (+ the aforementioned) and flushing all cache.
To verify I re-enabled Skinr UI module, flushed, and tested. I found I actually didn't need to flush(noteworthy --- not cache, not css loaded files -- depending on your config ^.^).
Disabling Skinr UI resolves problem entirely and instantly.

So Skinr UI module is producing the [contextual-links-region] class inside body classes tag; causing the massive random deformation to admin menu ({moving to bottom of page}, {disappearing}, {generating blank space above it}... etc? ). Curiously there is no reference to it at all within the Skinr module folder (via GREP search).
The .module file has some generation code for links; but no apparent CSS injection. I don't know where the string literal [contextual-links-region] is coming from.

Assumption #1:

So it seems to be something dynamically generated and injected into the page.

Assumption #2:

I disabled the module because I have NO desire to fool with this crap right now. Skinr is great, but this need not occur,especially during fresh Drupal 7.x startups ;)
I hereof assume that Skinr UI is the issue (injected-css issue : assumption #1 above), rather not the administration menu (some sort of conflict with said assumption #1?).

Hopefully this post will assist someone, in some form. :)

markhalliwell’s picture

Project: Administration menu » Skinr
Version: 7.x-3.x-dev » 7.x-2.0-beta1
Component: Code » User interface
Priority: Minor » Major

I can verify that it is indeed Skinr UI. I also traced this issue to this module. Moving this issue to the correct project.

Curiously there is no reference to it [the contextual-links-region class] at all within the Skinr module folder (via GREP search).

This is because contextual (core) is the one that actually adds the class if there are contextual links in the render array. See: http://api.drupal.org/api/drupal/modules!contextual!contextual.module/function/contextual_preprocess/7. If it finds contextual links in the render array, then it adds the class to the element.

That being said, Skinr is the one that actually goes through all the module hooks and inserts the "Edit skin" to the contextual links menu. So I thought maybe it might be because I have the theme hook 'region' in my theme's hook_skinr_skin_info() function. I did... but taking it out didn't actually fix the problem. Somehow Skinr is still making the page_top and page_top regions to become contextual-link-regions. My guess is that is has to do something with either sites/all/modules/skinr/modules/system.skinr.inc or.... possibly the strongest culprit: skinr_ui_preprocess() in sites/all/modules/skinr/skinr_ui.module.

sun’s picture

markhalliwell’s picture

Also just to clarify from the original user's post: I also thought it might have been an Omega issue at first. I was, however, able to install a fresh copy of Omega and Admin Menu works just fine with it. This issue isn't apparent until Skinr UI is enabled.

markhalliwell’s picture

@sun: That's a good point. Any module can technically be the culprit if they start injecting contextual-links-region classes via JavaScript (oy... what a nightmare). I think that issue is similarly related, but a separate issue nonetheless. Admin Menu is at least still at the top, albeit down few pixels. This issue causes Admin Menu to stay completely at the bottom of the page because the div#page_bottom region (not body) has the contextual-links-region class applied to it.

jurriaanroelofs’s picture

I'm also experiencing problems with contextual links class on body after updating skinr.

jurriaanroelofs’s picture

Priority: Major » Critical

I have 13 Drupal distributions and they all break when upgraded to skinr beta1 so I'm marking this critical and I asked moonray to make alpha1 the recommended release so that the number of disadvantaged users will be minimized.
btw downgrading to skinr alpha1 doesn't seem to fix it for me.

nicholas.alipaz’s picture

Status: Active » Needs review

The results of the issue are slightly different for me. I only see the admin_menu is pushed 20px further south from the top of the screen leaving a blank 20px at the top. I also notice that all the contextual link's gear icons become z-indexed higher than the modal window.

From what I can tell, skinr_ui_preprocess() is where the problem is. It is foreach'ing through each $hook and then foreach'ing through all the $elements. This means it will add contextual links for the html element and that is why the body class gets the additional class. I changed skinr_ui_preprocess() as follows and it fixed the issue for me:

/**
 * Implements hook_preprocess().
 */
function skinr_ui_preprocess(&$variables, $hook) {
  $original_hook = $hook;
  $theme_registry = theme_get_registry();
  if (isset($theme_registry[$hook]['original hook'])) {
    $original_hook = $theme_registry[$hook]['original hook'];
  }

  $contextual_links = array();
  $counter = 0;
  $array_elements = skinr_invoke_all('skinr_elements', $variables, $original_hook, 'contextual_links');
  foreach ($array_elements as $module => $elements) {
    foreach ($elements as $element) {
      if ($element !== 'html') {
        $contextual_links['skinr-' .  $module . '-' . $counter++] = array(
          'admin/structure/skinr/edit', array($module, $element),
        );
      }
    }
  }
  if (!empty($contextual_links)) {
    skinr_ui_contextual_links($variables, $original_hook, $contextual_links);
  }
  return;
}

Please review and let me know if this is the proper way to go about this change.

jurriaanroelofs’s picture

It works for me thanks.

moonray’s picture

Status: Needs review » Needs work

The downside of this approach is that there's no way for a user to set Skinr settings for the body element through contextual links, even for themes that would want that option.

We need a better solution here.

Perhaps adding a setting to toggle whether or not it's allowed? Or perhaps we need to add the contextual links to a different element (if so, which one)?

p.s.
Apologies for not responding to any tickets sooner. I'm a little swamped with work and life at the moment.

jurriaanroelofs’s picture

I can imagine it could be useful to have contextual links on the body element to add body classes but the body should never get position:relative by any module because it can mess up a layout or screw up toolbar positioning as we've now experienced.

The best solution would be to patch contextual links:

.contextual-links-region {
outline: none;
position: relative;
}
body.contextual-links-region {
position:static;
}

Second best would be to add this code to Skinr, since skinr is probably the only module that adds contextual links to the body element.

OnkelTem’s picture

The issue #1719440: Don't add "contextual links" to body element has been resolved as duplicate of this by moonray.
OK, I've read this thread and I see how that one is dup of this.

In my patch I'm disabling adding contextual links (CL) via skipping 'html' hook:

 function skinr_ui_preprocess(&$variables, $hook) {
+  // Adding exclusion for html hook since it breaks contextual links appearing,
+  // setting "contextual-links-region" class to body element.
+  if ($hook == 'html') {
+    return;
+  }

and it was my understanding that this solution is both necessary and sufficient. But after reading #15 I see now that the solution is not sufficient since it drops links (if there are any) to configure body element . A direction to a workaround was also given:

Or perhaps we need to add the contextual links to a different element (if so, which one)?

and it looks like the only option.

I have been playing with contextual links in my recent sandbox'ed project TabsNoMore where I remove all tabs (LOCAL_TASKS) and place them in global contextual links which I attach to page's content - most sensible place to my mind. I believe I can use some code from there to fix this issue.

OnkelTem’s picture

Title: admin menu showing at bottom » Correct injection of top-level contextual links

I think this suits more

jurriaanroelofs’s picture

Title: admin menu showing at bottom » Correct injection of top-level contextual links
Version: 7.x-2.x-dev » 7.x-2.0-beta1

Fixed and commited to dev as per #16.
There's nothing wrong with adding contextual links to the body tag because that's the tag you want if you have a theme that uses fullsize backgrounds and you want to offer alternative skins for that.

However, I don't get why elements get the contextual link class while it doesn't have contextual links inside, is that skinr fault too?

jurriaanroelofs’s picture

Title: Correct injection of top-level contextual links » admin menu showing at bottom
Version: 7.x-2.0-beta1 » 7.x-2.x-dev
Status: Needs work » Fixed

I think the old title will bring people to this issue who experience problems with the bug because so far I think it's the only symptom of the problem.

Jeff if you're reading this you can now remove these lines that I found in adaptivetheme lol:

  // Strip stupid contextual-links-region class, wtf?
  $vars['classes_array'] = array_values(array_diff($vars['classes_array'], array('contextual-links-region')));
OnkelTem’s picture

Title: Correct injection of top-level contextual links » admin menu showing at bottom
Version: 7.x-2.0-beta1 » 7.x-2.x-dev
Status: Fixed » Active

@JurriaanRoelofs

There's nothing wrong with adding contextual links to the body tag because that's the tag you want if you have a theme that uses fullsize backgrounds and you want to offer alternative skins for that.

Have you seen the screenshot of the problem when adding contextual links to the body tag because that's the tag you definitely don't want since it:

1) makes contextual links to be visible always
2) and breaks Overlay interface.

Please take a look: http://drupal.org/files/Screenshot%20from%202012-08-08%2020:44:46.png

UPDATE I haven't tried the fix from #16 yet. Let me test it tomorrow. If this will fix both problems I'll close this issue back.

jurriaanroelofs’s picture

update: Sometimes after completing an operation in Seven theme (e.g. disabling a theme) the admin_menu and other contents of $page_bottom are rendered into a <div class="region region-page-bottom contextual-links-region"> wrapper and the problem recurs.

Not sure what is causing this but should be fixed too before this issue is completely resolved.

OnkelTem’s picture

@JurriaanRoelofs

I've tried you solution.

Well, I can't neither confirm nor disconfirm whether it fixes original issue or not (since I don't use admin menu) but it has nothing to do with what I've been claiming in #21 as well as with my original issue #17, which had been resolved as duplicate of this by @moonray. Following that, I had edited the issue's title to reflect the root of problem, but was corrected by you back. So, I've got into a situation when one side tells me 'its dupe, dude', and the other tells opposite ("this is reloved", while it is not).

If you feel satisfied with this solution and not-showing admin menu at the bottom is the only issue you are fixing, then I feel duty to reopen my issue back and continue fixing the real problem there.

Look forward to hear from moonray about his vision.

jurriaanroelofs’s picture

Anyone having this problem is welcome to test the patch in this meta issue: http://drupal.org/node/1753486#comment-6419808

arrays’s picture

Hi,

I have not installed Skinr module and I am still facing this issue. Any solution to the issue??

nedjo’s picture

Status: Active » Closed (duplicate)
marcelodornelas’s picture

Hi guys, the quickest and cleaner way to fix this is to edit the views page and set HIDE CONTEXTUAL LINKS to YES.