WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#17107 closed defect (bug) (fixed)

Theme/plugin CSS can override admin bar parent menu fonts

Reported by: johnjamesjacoby Owned by:
Milestone: 3.2 Priority: normal
Severity: normal Version: 3.1
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

Attaching a screenshot of the menu-bar when using the Mystique theme as an example of what happens.

Spent a few minutes playing with the admin-bar CSS trying to come up with a patch that didn't conflict with existing CSS but couldn't come up with a solution that didn't replace a bunch of it. Will revisit a patch this week.

The admin bar font is originally applied here to the entire #wpadminbar: http://core.trac.wordpress.org/browser/trunk/wp-includes/css/admin-bar.dev.css#L27

Single link root-level menu items aren't affected in my screen shot because they have specific CSS that prevents it here: http://core.trac.wordpress.org/browser/trunk/wp-includes/css/admin-bar.dev.css#L74

(Putting this in the "Menus" component even though that's meant for Appearance -> Menus. Will probably want to add a dedicated "Admin Bar" component.)

Attachments (4)

menu-bar.png (23.4 KB) - added by johnjamesjacoby 3 years ago.
Menu Bar with overridden fonts
admin-bar.patch (598 bytes) - added by johnjamesjacoby 3 years ago.
Add font styling to #wpadminbar *
Screenshot.png (2.0 KB) - added by greuben 3 years ago.
17107.diff (605 bytes) - added by greuben 3 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 nacin3 years ago

  • Component changed from Menus to Administration

johnjamesjacoby3 years ago

Menu Bar with overridden fonts

comment:2 follow-up: andrewryno3 years ago

Dupe of #16667? It was meant for 3.1.1 but it should probably just be reopened if there is more that can be done. I'll leave it up for someone else to decide.

comment:3 in reply to: ↑ 2 nacin3 years ago

Replying to andrewryno:

Dupe of #16667? It was meant for 3.1.1 but it should probably just be reopened if there is more that can be done. I'll leave it up for someone else to decide.

Nope, once a ticket is closed on a completed milestone, it's time for a new ticket.

comment:4 follow-up: azaozz3 years ago

Wouldn't something like this work there?

#wpadminbar * {
    font: normal 12px/28px Arial, Helvetica, sans-serif;
}

comment:5 in reply to: ↑ 4 johnjamesjacoby3 years ago

Replying to azaozz:

Wouldn't something like this work there?

#wpadminbar * {
    font: normal 12px/28px Arial, Helvetica, sans-serif;
}

Depends how much prevention we're baking in. I initially started including color and text-shadow in addition to the font, and realized it changes for submenus.

I also noticed that the CSS at http://core.trac.wordpress.org/browser/trunk/wp-includes/css/admin-bar.dev.css#L74 bumps the font-size up to 13px for some reason. Wasn't sure if that was intentional or not, so rather than start recoding all the CSS for the conditions I was finding, I opened this ticket.

johnjamesjacoby3 years ago

Add font styling to #wpadminbar *

comment:6 johnjamesjacoby3 years ago

Attached patch moves the main bar font styling up into #wpadminbar *

This works because all of the other core admin bar elements have dedicated styling already to change their respective font sizes, colors, and shadows, including submenus.

As a result, this patch also fixes a small font discrepancy where top level links had a 13px font size, and elements with submenus has a 12px font size.

comment:7 nacin3 years ago

Cool, but I don't necessarily see that as a discrepancy. On my screen 12px looks quite visibly different.

comment:8 andrewryno3 years ago

I'd be curious as to see what the performance is like of that universal selector, especially with the added font declarations. There's a limited number of elements that would appear in the admin bar, so replacing * with each element (similar to Eric Meyer's reset, HTML5 boilerplate reset, etc.) that is used, and then also only add the font styles to the elements which have a font in them. Also, no specificity should be lost by switching.

Granted, this was probably brought up before when it was initially made, but thought I'd like to bring up the point.

comment:9 azaozz3 years ago

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

(In [17626]) Stop themes and plugins from overriding the admin bar CSS, props johnjamesjacoby, fixes #17107

comment:10 azaozz3 years ago

  • Keywords needs-patch removed
  • Milestone changed from Awaiting Review to 3.2

comment:11 johnjamesjacoby3 years ago

Replying to nacin:

Cool, but I don't necessarily see that as a discrepancy. On my screen 12px looks quite visibly different.

To clarify, by "top level links" I meant items that did not have submenus. Items like "Edit Post" and "Comments" were always 13px, but any item with a submenu was 12px.

Replying to andrewryno:

I'd be curious as to see what the performance is like of that universal selector, especially with the added font declarations. There's a limited number of elements that would appear in the admin bar, so replacing * with each element (similar to Eric Meyer's reset, HTML5 boilerplate reset, etc.) that is used, and then also only add the font styles to the elements which have a font in them. Also, no specificity should be lost by switching.

Also makes sense. One of the goals of this patch was to modify as few lines as possible, and this was the simplest solution. Can always revisit more exacting CSS later.

comment:12 azaozz3 years ago

(In [17627]) Not 12, 13px, see #17107

greuben3 years ago

comment:13 greuben3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

[17626] changed the "My sites" dropdown color.. see attached screenshot

Reporduced on Chrome 10.0.648 on Ubuntu

greuben3 years ago

comment:14 greuben3 years ago

  • Keywords has-patch added

Attached patch moves the font declaration.

comment:15 ocean903 years ago

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

Ticket and a patch for "my sites" issue: #17178

Note: See TracTickets for help on using tickets.