Follow-up from #1961946: Simpletest module's #overlay ID causes CSS conflicts.

Updated: Comment #0

Problem/Motivation

The #overlay ID from overlay.module has caused a number of conflicts with other modules: so far we've seen conflicts on the module listing page and on the test list page, see #1961946: Simpletest module's #overlay ID causes CSS conflicts for more details and screenshots.

Proposed resolution

Change the #overlay ID to something more specific like #overlay-wrapper, or maybe a class like .overlay__wrapper.

Remaining tasks

Decide if we want to change the #overlay ID and what we want to change it to.

User interface changes

n/a

API changes

n/a

#1961946: Simpletest module's #overlay ID causes CSS conflicts

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

annikaC’s picture

Getting this rolling. Is the 'overlay' class causing conflicts anywhere?

sidharthap’s picture

Status: Active » Needs review
star-szr’s picture

Status: Needs review » Needs work

Thanks for getting this rolling @annikaC! I think there is definitely some CSS in core that would need to be updated to match this new ID.

sandipmkhairnar’s picture

Thanks Cottser. Updated patch as per comment.

sandipmkhairnar’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, change_overlay_id-2091665-4.patch, failed testing.

sandipmkhairnar’s picture

Status: Needs work » Needs review
FileSize
416 bytes
416 bytes
416 bytes
star-szr’s picture

#4: change_overlay_id-2091665-4.patch queued for re-testing.

star-szr’s picture

#4 looks more along the lines of what we want to do - I think we just had a random test failure.

nod_’s picture

Version: 8.x-dev » 7.x-dev
Issue summary: View changes
Status: Needs review » Needs work

Overlay is dead to D8 #2088121: Remove Overlay.

dsdeiz’s picture

Status: Needs work » Needs review
FileSize
1.21 KB

I wonder if this is the only change.

royal121’s picture

FileSize
2.94 KB

Patch in #11 works great! But I did find some more usages of '#overlay'. Here is a new patch which includes the new ones too. Please someone with experience review this patch as I am new to Drupal. Thanks :)

nod_’s picture

Not sure we can change markup on D7 though.

parthipanramesh’s picture

Patch #12 works fine for me!

parthipanramesh’s picture

Status: Needs review » Reviewed & tested by the community
David_Rothstein’s picture

Status: Reviewed & tested by the community » Closed (won't fix)

Like nod_ said, we can't make a change like this in Drupal 7 without a really good reason, since it's likely to break people's code.

And there isn't anything really wrong with the current ID; it's very much within the Overlay module's own "namespace", so conflicts with other code indicate a bug in that other code instead. For example, if something like #1961946: Simpletest module's #overlay ID causes CSS conflicts ever affects Drupal 7 (guess it would only if Overlay tests ever get added in Drupal 7?) then it would make more sense to backport the fix for that issue instead.