Updated: Comment #9

Problem/Motivation

When viewing the test listing page (admin/config/development/testing) within the overlay, the test group for overlay module doesn't appear correctly.

Proposed resolution

Adjust the ID in the simpletest module by prefixing "module-".

Before:
1961946-2.png

After:
1961946-9.png

Remaining tasks

None

User interface changes

Overlay module on admin/config/development/testing will appear correctly in overlay.

API changes

n/a

None yet.

Original report by @sayela

There is an ID (overlay) in the overlay.tpl template of the core overlay module that serves as a wrapper for the contents. This is however conflicting with another ID (overlay) that identifies the overlay module in the module listings. This results in the latter inheriting the CSS properties of the former thereby causing undesirable results.

Proposed Solution:

Change the ID wrapping the overlay (line 25 of overlay.tpl) from "overlay" to "overlay-wrapper" and then edit the overlay-child.css (line 17) accordingly.

Thank you.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mandclu’s picture

Status: Active » Closed (cannot reproduce)

Unable to replicate. On a fresh Drupal 8 install it appears that the IDs are now prefixed with "module-" so it seems that this is unlikely to be an issue anymore.

star-szr’s picture

Title: Overlay module display bug » Overlay module's #overlay ID causes CSS conflicts
Status: Closed (cannot reproduce) » Active
Issue tags: -overlay +CSS, +Novice
FileSize
17.11 KB

Found a very similar bug on the testing page (admin/config/development/testing) with overlay enabled. There is a conflict with a td that has an ID of overlay. So I think #overlay-wrapper or another solution might be worth exploring.

1961946-2.png

TR’s picture

Component: overlay.module » simpletest.module
Status: Active » Needs review
FileSize
741 bytes

I can confirm the behavior reported by Cottser.

The SimpleTest module assigns a CSS ID for each of the check boxes on this page, and uses the name of the module as the ID. So the <tr> holding the Overlay module's checkbox has an ID of #overlay. Because this page is an admin page, it's shown within an overlay, so the overlay CSS gets loaded. But the #overlay ID is already targeted by CSS in the Overlay module, namely overlay-child.css. The properties assigned to #overly in overlay-child.css (specifically, the min-height: 100px and padding: 2em 40ex properties) are the ones that add the extra horizontal and vertical spacing.

Probably the simplest fix is to rename all the IDs on this page -- choosing the module name as an ID is highly risky and is something that should only be done from within the module. The attached patch prefixes the IDs on this page with 'module-'.

Note, the original post is exactly this same problem, except on admin/modules instead of admin/configuration/development/testing. As stated in #2, admin/modules was fixed by changing the CSS ID on that page, which is exactly the same fix as I'm proposing here in my patch for admin/configuration/development/testing.

TR’s picture

Title: Overlay module's #overlay ID causes CSS conflicts » Simpletest module's #overlay ID causes CSS conflicts

Better title.

star-szr’s picture

@TR - Thanks, that seems reasonable. Longer term, do you think it would make sense to change the #overlay ID to something else, or should that be owned by the overlay module's "namespace"?

TR’s picture

Yes, I think it probably makes sense to take the additional step for the overlay module to choose a less generic ID in the long run, but that's more of a "nice-to-have" rather than a bug. Re-writing the CSS generated and used by overlay isn't something that's going to happen anytime soon.

It's not an either/or fix - both can be done, and I certainly don't want this current issue to be sidetracked by an overlay rewrite effort. Let's fix the bug here, in the same way that admin/modules was already fixed.

star-szr’s picture

Status: Needs review » Reviewed & tested by the community

I agree. Can you please open another issue for the overlay ID update and link it back here? The new issue could simply be changing #overlay to #overlay-wrapper to start with. That can happen sometime soon :)

Manually tested to confirm the bug is fixed, adding before/after screenshots to the issue summary.

star-szr’s picture

FileSize
13.62 KB

It helps if you attach the screenshot :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Nice catch.

Committed and pushed to 8.x. Thanks!

star-szr’s picture

Here's the follow-up issue to discuss reworking the #overlay ID: #2091665: Change #overlay ID to avoid conflicts

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Update summary to use full template and add before/after screenshots