WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 6 months ago

#35779 closed defect (bug) (fixed)

Twenty Fourteen: float-clearing rules can break plugin output

Reported by: iandunn Owned by: ianbelanger
Milestone: 5.3 Priority: normal
Severity: normal Version: 3.8
Component: Bundled Theme Keywords: bulk-reopened has-patch commit
Focuses: Cc:

Description

style.css:750 (from r25021 / #24858) uses overly broad selectors, which can unintentionally target elements generated by plugins.

For example, an element like <div class="{pluginslug}-content"> or <p class="{pluginslug}-select-site"> will have empty content added before/after it, and its display changed to fixed, which could break the layout.

For an observable example, visit WordCamp Dayton 2016's schedule, inspect td.wcb-session-site-maintenance, and disable the following rules (which were added to work around this problem):

.wcpt-schedule [class*="content"]:before,
.wcpt-schedule [class*="content"]:after,
.wcpt-schedule [class*="site"]:before,
.wcpt-schedule [class*="site"]:after {
    content: none;
    display: inherit;
}

Other Core themes use broad selectors like these, but they only target align, wp-image, and attachment, which are probably less common in plugin class names than site and content (although I would guess that they're still common enough to avoid using without a wp-specific prefix).

Maybe explicitly targeting TwentyFourteen's classes (site-content, content-area, etc) would be a way to avoid this problem, while still maintaining backwards-compatibility?

Attachments (1)

35779.diff (1.7 KB) - added by ianbelanger 13 months ago.
Explicitly target all instances of site or content classes

Download all attachments as: .zip

Change History (15)

#1 @karmatosed
4 years ago

  • Keywords needs-patch added

I think in this case explicitly targeting makes sense. Want to rustle up a patch?

#2 @karmatosed
4 years ago

  • Milestone changed from Awaiting Review to Future Release

#3 @karmatosed
4 years ago

@iandunn can you work on a patch for this?

#4 @iandunn
4 years ago

Hey, sorry, it's been on my list for awhile, but I haven't found time yet. It doesn't look like I'll have time soon, but I'll keep it on the list just in case.

#5 @SergeyBiryukov
4 years ago

  • Summary changed from TwentyFourteen float-clearing rules can break plugin output to Twenty Fourteen: float-clearing rules can break plugin output

@ianbelanger
13 months ago

Explicitly target all instances of site or content classes

#8 @ianbelanger
13 months ago

  • Keywords bulk-reopened has-patch added; needs-patch removed

35779.diff should explicitly target all instances of either site or content in any of the classes contained in the theme markup.

Last edited 13 months ago by ianbelanger (previous) (diff)

#9 @ianbelanger
13 months ago

  • Milestone set to Future Release

#10 @ianbelanger
11 months ago

  • Milestone changed from Future Release to 5.3
  • Owner set to ianbelanger
  • Status changed from new to accepted

Patch still applies cleanly, setting to 5.3 milestone so we can get this fix in.

#11 @davidbaumwald
6 months ago

@ianbelanger The patch appears to still be good. Is this still on your radar for 5.3?

#12 @ianbelanger
6 months ago

  • Keywords commit added

This also looks good to go @davidbaumwald. Tagging for commit

This ticket was mentioned in Slack in #core by ianbelanger. View the logs.


6 months ago

#14 @afercia
6 months ago

Worth noting core uses a .wp-clearfix CSS class for this purpose. Not sure the huge pile of selectors used by Twenty Twenty is the best option :) when it would be as simple as using an utility CSS class and add it where necessary.

#15 @SergeyBiryukov
6 months ago

Unless I'm missing something, .wp-clearfix is only available in the admin and is not used in any of the default themes. It might be a good idea to explore its usage in a new ticket.

I'm not a fan of the huge pile of selectors either, but it seems to be the most straightforward fix here for now.

#16 @SergeyBiryukov
6 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 46430:

Twenty Fourteen: Replace overly broad float-clearing selectors with more specific ones, to avoid unintentionally targeting elements generated by plugins.

Props ianbelanger, iandunn, karmatosed.
Fixes #35779.

Note: See TracTickets for help on using tickets.