WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 2 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:
PR Number:

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 8 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
3 years ago

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

@ianbelanger
8 months ago

Explicitly target all instances of site or content classes

#8 @ianbelanger
8 months ago

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

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

Version 0, edited 8 months ago by ianbelanger (next)

#9 @ianbelanger
8 months ago

  • Milestone set to Future Release

#10 @ianbelanger
7 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
2 months ago

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

#12 @ianbelanger
2 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.


2 months ago

#14 @afercia
2 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
2 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
2 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.