WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#19669 closed enhancement (wontfix)

body_class - add browser class to the body

Reported by: nofearinc Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.3
Component: Themes Keywords: close
Focuses: Cc:

Description

I didn't find this request with the search so apologies if it has already been discussed here.

My request is for adding browser class to the body_class function. I think it would be useful core functionality and I have used this one via external scripts. No extra ie8.css and ie9.css files would be needed and several CSS declarations could be listed separated with commas in the main css files. One great idea as listed in this blog is what I recall.

Browsed through the body_class function and down there but haven't found any resolution in the repository.

Change History (12)

comment:1 follow-up: @scribu3 years ago

  • Keywords close added

Such a browser class would depend on the user agent string, which is not always accurate.

If you want to target IE versions, a more reliable method is employed in the Twentyeleven theme, using conditional comments:

http://core.trac.wordpress.org/browser/tags/3.3/wp-content/themes/twentyeleven/header.php#L12

comment:2 @toscho3 years ago

  • Cc info@… added

This would interfere with caching plugins. Check for features, not for UA strings. Conditional comments are the way to go, as @scribu suggested.

comment:3 follow-up: @nofearinc3 years ago

I don't really mind checking for the user agent string. There are different tools and plugins checking for compatibility etc that represent themselves for other browsers - so why should it be wrong this way?

@toscho - haven't thought about this - conditional are standard for including scripts, but including files with 3-10 lines of CSS styling might be an overhead in my opinion (using combine scripts could be a solution here).

I don't see any difference between setting the ID in html tag or placing it in the body. Probably I lack seniority in caching.

comment:4 in reply to: ↑ 3 @toscho3 years ago

Replying to nofearinc:

conditional are standard for including scripts, but including files with 3-10 lines of CSS styling might be an overhead in my opinion (using combine scripts could be a solution here).

Don’t include separate CSS files, add class names to the HTML element and use separate selectors in your stylesheet. Content-Negotiation is tricky. Getting it right with local caches and remote proxy servers is more work than most people imagine. Avoid it whenever possible.

comment:5 @scribu3 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Yes, I haven't thought about caching. Plugins like WP Super Cache simply couldn't work, since the page would have to be generated dynamically for each user.

So, this is not something we should include in Core, especially since it's possible to do with a filter.

comment:6 in reply to: ↑ 1 ; follow-ups: @azaozz3 years ago

Replying to scribu:

Currently we do this for IE8 http://core.trac.wordpress.org/browser/trunk/wp-admin/includes/template.php#L1652. Can add IE9 and perhaps 10 when it comes out. IE7 is still loading ie.css which seems the sensible thing to do until we drop support for it.

comment:7 in reply to: ↑ 6 ; follow-ups: @nofearinc3 years ago

Replying to azaozz:

From what I see this is used for thickbox iframes and is already part of the core. Still doesn't make any sense to add it to the html or body tag by default?

This could save several conditional checks in the header calls and external files as well.

comment:8 in reply to: ↑ 7 @scribu3 years ago

Replying to nofearinc:

From what I see this is used for thickbox iframes and is already part of the core. Still doesn't make any sense to add it to the html or body tag by default?

The thickbox is part of the admin, which is never cached.

This could save several conditional checks in the header calls and external files as well.

You didn't read toscho's comment, did you?

comment:9 @nofearinc3 years ago

I did indeed. Just brainstorming on latest comments by azaozz and getting updated.

comment:10 in reply to: ↑ 7 @azaozz3 years ago

Replying to nofearinc:

From what I see this is used for thickbox iframes and is already part of the core. Still doesn't make any sense to add it to the html or body tag by default?

This is used everywhere in the admin except when showing specific error pages like errors while installing, etc. where we don't have access to all code (and don't queue styles). I'm not sure what you mean by "Still doesn't make any sense to add it to the html or body tag by default?" :)

Last edited 3 years ago by azaozz (previous) (diff)

comment:11 @nofearinc3 years ago

@azaozz - I was referring to the very beginning of the ticket as body_class() is required for themes submitted in WPORG. Therefore, for easier and standardized compatibility, my idea was extending body_class() (or now, creating html_class() for example) that could be a requirement that helps browser compatibility settings.

However, as I see I'm still going to use filters for this, thanks for the heads up guys.

comment:12 in reply to: ↑ 6 @dd323 years ago

Replying to azaozz:

Replying to scribu:

Currently we do this for IE8 http://core.trac.wordpress.org/browser/trunk/wp-admin/includes/template.php#L1652. Can add IE9 and perhaps 10 when it comes out. IE7 is still loading ie.css which seems the sensible thing to do until we drop support for it.

Yeah this ticket is talking about for the front-end, not backend.

Here's what TwentyEleven does to get the IE class using conditional templates, it's fully caching-compliant too and doesn't require extra stylesheets: http://core.trac.wordpress.org/browser/trunk/wp-content/themes/twentyeleven/header.php#L11

Note: See TracTickets for help on using tickets.