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
Related Issues
#1961946: Simpletest module's #overlay ID causes CSS conflicts
Comment | File | Size | Author |
---|---|---|---|
#12 | overlay-id-2091665-12.patch | 2.94 KB | royal121 |
#11 | drupal-2091665-11-overlay-id.patch | 1.21 KB | dsdeiz |
#7 | overlay_interdiff-1-4.txt | 416 bytes | sandipmkhairnar |
#7 | change_overlay_id-2091665-7.patch | 416 bytes | sandipmkhairnar |
#4 | change_overlay_id-2091665-4.patch | 1.03 KB | sandipmkhairnar |
Comments
Comment #1
annikaC CreditAttribution: annikaC commentedGetting this rolling. Is the 'overlay' class causing conflicts anywhere?
Comment #2
sidharthapComment #3
star-szrThanks 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.
Comment #4
sandipmkhairnar CreditAttribution: sandipmkhairnar commentedThanks Cottser. Updated patch as per comment.
Comment #5
sandipmkhairnar CreditAttribution: sandipmkhairnar commentedComment #7
sandipmkhairnar CreditAttribution: sandipmkhairnar commentedComment #8
star-szr#4: change_overlay_id-2091665-4.patch queued for re-testing.
Comment #9
star-szr#4 looks more along the lines of what we want to do - I think we just had a random test failure.
Comment #10
nod_Overlay is dead to D8 #2088121: Remove Overlay.
Comment #11
dsdeiz CreditAttribution: dsdeiz commentedI wonder if this is the only change.
Comment #12
royal121 CreditAttribution: royal121 commentedPatch 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 :)
Comment #13
nod_Not sure we can change markup on D7 though.
Comment #14
parthipanramesh CreditAttribution: parthipanramesh commentedPatch #12 works fine for me!
Comment #15
parthipanramesh CreditAttribution: parthipanramesh commentedComment #16
David_Rothstein CreditAttribution: David_Rothstein commentedLike 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.