Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
overlay.module
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
26 Oct 2009 at 19:27 UTC
Updated:
30 Apr 2012 at 14:03 UTC
Jump to comment: Most recent file
Comments
Comment #1
David_Rothstein commentedRetitling.
Comment #2
David_Rothstein commentedThis is very likely the cause of the following bug reported by @webchick as part of #610234: Overlay implementation:
Since node/*/delete is defined in hook_admin_paths(), it should be showing the admin theme in the overlay regardless of other settings in Drupal. However, it is not - other settings in Drupal which cause the main site theme to show on this page even when the admin theme is being used for content creation (actually that sounds like a bug in and of itself?) are taking precedence...
By the way, once we get this fixed, we should also remove this line from default.install:
As far as I know, that was only put there in order to make things look like some mockups... but since after this issue goes in the overlay will show the admin theme on content creation pages anyway, we don't need or want it anymore. The way that setting works, it is not something that would be commonly used by most community-minded Drupal sites, so it's not appropriate for the default install profile. (See also comments by me and @cwgordon7 somewhere in #517688: Initial D7UX admin overlay where at one point this setting was taken out as part of the overlay patch, for this reason.)
Comment #3
ksenzeeThis patch is dependent on #553944: Define hook_menu_get_item_alter() as a reliable hook that runs before the page is doomed, specifically the patch at #88.
Comment #4
David_Rothstein commentedThe patch looks good to me. I'm not going to bother testing it out until the other one is committed though :)
This will probably break the "demonstrate block regions" page but we already have an issue to figure out what to do about that at #655416: "Demonstrate block regions" bugs with regions, navigation, overlay anyway.
Although technically not required to solve this issue, it seems like this might not be a bad time to introduce a function along the lines of overlay_will_be_displayed()?
Comment #5
casey commentedcould be rewritten to:
Comment #6
David_Rothstein commentedIt can't, because the overlay mode isn't set yet when this code is called. Would be hard to know that without studying the other patch in detail, though :)
It did occur to me that we could perhaps just set the overlay mode here rather than waiting until hook_init() where the same check is performed ... but conceptually, it seems like this is the wrong place to set it.
Comment #7
casey commentedoww wait this is about hook_custom_theme() not about hook_overlay_child_initialize(). never mind.
Comment #8
quicksketchI was working on porting Webform to Drupal 7 when I ran into this problem. Basically right now there are two COMPLETELY different mechanisms for defining admin paths. Node module does one thing to set the admin theme (using "theme callback" in hook_menu()), while Overlays uses hook_admin_paths() to determine if an overlay should be used. This results in inconsistencies between when the overlay is used and when the admin theme is used.
Here's an expected user workflow:
- Set the Admin theme at "admin/appearance" to Seven
- Uncheck "Use the administration theme when editing or creating content"
- Now creating a new piece of content should NOT use the overlay right? However what happens is that the overlay is brought up when creating/editing content, but uses Garland inside the overlay.
This patch makes it so that system.module is responsible for turning on the admin theme, not overlay.module. node.module then bases it's list of admin paths based on the "node_admin_theme" variable.
Comment #9
quicksketchLooks like system.module was also using this "theme callback" approach for the "admin" path. It's very strange that we were using completely different mechanisms for setting the "admin theme" and determining "admin paths". In order to keep the block-preview page working, I switched the order of precedence when both a "theme callback" is specified and a module is returning a theme through hook_custom_theme(). Now if a URL requires a particular theme, that is the final say. However the ONLY path that does this now is the blocks preview page.
Comment #10
David_Rothstein commented@quicksketch, I think the issue you want for this is actually #669510: Merge administration theme with hook_admin_paths() ... hopefully still has a shot for D7, since the current behavior is indeed confusing.
Comment #11
quicksketchOh dear, so many admin-theme issues. Yes I think you're right David, mind if I merge this issue with that one? After fixing this problem generally, the only thing necessary for overlay.module is just to remove anything relating to $custom_theme.
Comment #13
quicksketchFixes tests (I hope).
Comment #14
quicksketchOops, I didn't mean to upload that patch here. I'm consolidating with #669510: Merge administration theme with hook_admin_paths().
Comment #15
casey commentedAt least we can remove the usage of $custom_theme as it doesn't so anything anyway.
Comment #16
casey commentedAnyone?
Comment #17
aspilicious commentedLooks fine to me, but I'm not an expert, so won't rtbc.
- I did apply the patch (just to be sure and didn't see any problems with the overlay) ==> OK
- I checked for trailing whitespaces, couldn't find any ==> OK
Comment #18
David_Rothstein commentedThe patch is good, and at the very least directly addresses the issue title :)
It doesn't solve the issue that the existing code had been trying to solve, so we'd still need a follow-up along the lines of "Overlay does not show all its pages in the admin theme". But perhaps that can still be solved via #669510: Merge administration theme with hook_admin_paths() anyway.
Comment #19
dries commentedCommitted to CVS HEAD.
Comment #21
David_Rothstein commentedI was going to file a followup issue for this, but we already have lots of patches above addressing it, so easier just to reopen.
Since #662868: user edit link should open in a overlay was committed, we now have a good visible example of this in core: If you go to a user account edit page, that page now displays in the overlay using Bartik (or whatever your main site theme is). Since the overlay is intended to be an administrative feature, it's supposed to show all its pages in the admin theme, if you are using one.
I'm reopening this issue to track it, and because we could (in theory) go back to trying an overlay-specific solution to this, but hopefully we'll just still be able to do #669510: Merge administration theme with hook_admin_paths() instead. (I keep meaning to find some time to get back to that one.)
Comment #22
tgeller commentedFor one example of the problem, see #930524: Type style is wrong in tabs of View Revisions page, which David closed and redirected here.
Comment #23
Jeff Burnz commentedAgree with 18, is there a follow up issue posted, I have found multiple instances of themes not loading in the Overlay, including Garland and Stark under certain circumstances.
Comment #24
markabur commentedSubscribing; the overlay theme really needs to stay consistent from screen to screen.
Is #669510: Merge administration theme with hook_admin_paths() likely to make it into d7 at this point?
Comment #25
quicksketchFrom #18:
No point in having two issues open, especially since this particular issue was "fixed", the other patch addresses the remaining problems.
Comment #26
David_Rothstein commentedI think this should be "postponed" instead, since the other issue is broader and it's not 100% guaranteed it can make it into Drupal 7.
If it does get bumped to Drupal 8, we should still try for a more limited solution here, since this bug is pretty bad. (With the default Bartik theme it isn't so terrible since Bartik is designed to look good in the overlay, but with other themes, it can be extremely extremely ugly.)
That said, I hope we can still do #669510: Merge administration theme with hook_admin_paths(). I believe I will have some time (hopefully on Wednesday) to work on updating the patch there. Let's see what happens there first, and then reopen this one only if we have to.
Comment #27
David_Rothstein commented#669510: Merge administration theme with hook_admin_paths() was committed, so this is basically fixed.
The behavior now is that the site shows all users a consistent theme within the overlay, but only shows them the admin theme itself if they have the "View the administration theme" permission. There was some discussion in that issue of whether the overlay module should always force the admin theme when one is available, so perhaps this issue (or another one) should be reopened for that. However, it then means the overlay would be overriding/ignoring the "View the administration theme" permission, so we'd have to tread carefully there.
Personally I think it's OK as is, but if someone wants to reopen, feel free. The patch to start from for that is Gábor's at http://drupal.org/node/669510#comment-3309172
Comment #29
fejn commentedHi,
This support request is similar to http://drupal.org/node/615138 (this issue), except that I am getting a region of my custom_theme blocking out a portion of the overlay; I'm currently using Seven as my default theme and my administration theme.
See the attached pictures for what it does to the displays: a) the first picture shows the overlay partially covered by a region of the custom_theme; b) shows the overlay presented totally in the admin theme (the way it should).
I've found somewhat of a workaround for this:
a) uses path http://theme.com/Disclaimer#overlay=admin/structure/block
b) uses path http://theme.com/admin/structure/block (essentially the same as above, w/o the "node#overlay=" in path)
I'm using Drupal 7.12; are the patches discussed above included in this version off core? Are they even applicable to this problem?
Is there any easier/better fix than this(it's kind of hard to tell users to do this, so they can enter data into fields on a data entry form)
Thanks for any comments you might have.
Jeff
Comment #30
David_Rothstein commentedHi Jeff, yes, these patches were committed over one year ago and are in all recent versions of Drupal 7.
It doesn't look to me like your issue is related. This issue was about the entirely incorrect theme being shown in the overlay, not about portions of the underlying page obscuring the overlay. It's possible your issue could be caused by using a very high z-index in the custom theme (higher than the z-index used by the Overlay module's CSS).
Comment #31
fejn commentedThanks, David - I'd have never thought of that one. I found that the z-index for that area was set to 2105; I reset it to -1 and now I can at least dependably get to the admin pages w/o typing in the admin/.... path to force it to come up using seven. This may solve most of my problems. Do you know what z-index the overlays use, or where to look to find out what they want? Now my problem is that the main menu (which had z=2105) is inaccessible, an I can't click on the items, or have anything happen when I hover over them.
If I can get a relatively easy fix to that issue, this issue can probably be closed.
Jeff
Comment #32
David_Rothstein commentedI believe it's around 500; check modules/overlay/overlay-parent.css (or use a tool like Firebug) for the details. If you use a large z-index (but smaller than 500), you'll hopefully be OK.
Comment #33
fejn commentedIt's 500 in modules/overlay/overlay-parent.css .
Thanks for your help.