Make WordPress Core

Opened 13 years ago

Closed 13 years ago

#17107 closed defect (bug) (fixed)

Theme/plugin CSS can override admin bar parent menu fonts

Reported by: johnjamesjacoby's profile 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 13 years ago.
Menu Bar with overridden fonts
admin-bar.patch (598 bytes) - added by johnjamesjacoby 13 years ago.
Add font styling to #wpadminbar *
Screenshot.png (2.0 KB) - added by greuben 13 years ago.
17107.diff (605 bytes) - added by greuben 13 years ago.

Download all attachments as: .zip

Change History (19)

#1 @nacin
13 years ago

  • Component changed from Menus to Administration

@johnjamesjacoby
13 years ago

Menu Bar with overridden fonts

#2 follow-up: @andrewryno
13 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.

#3 in reply to: ↑ 2 @nacin
13 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.

#4 follow-up: @azaozz
13 years ago

Wouldn't something like this work there?

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

#5 in reply to: ↑ 4 @johnjamesjacoby
13 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.

@johnjamesjacoby
13 years ago

Add font styling to #wpadminbar *

#6 @johnjamesjacoby
13 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.

#7 @nacin
13 years ago

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

#8 @andrewryno
13 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.

#9 @azaozz
13 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

#10 @azaozz
13 years ago

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

#11 @johnjamesjacoby
13 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.

#12 @azaozz
13 years ago

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

@greuben
13 years ago

#13 @greuben
13 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

@greuben
13 years ago

#14 @greuben
13 years ago

  • Keywords has-patch added

Attached patch moves the font declaration.

#15 @ocean90
13 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.