WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#18966 closed defect (bug) (fixed)

Notice when HTTP_USER_AGENT not set

Reported by: scribu Owned by: azaozz
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

In [18975], yet another global was introduced: $wp_htmltag_class.

Unlike the other globals ($is_winIE, $is_opera etc.), it doesn't check if $_SERVER['HTTP_USER_AGENT'] exists, causing a notice.

Attachments (3)

18966.diff (4.0 KB) - added by scribu 3 years ago.
18966.2.diff (6.6 KB) - added by scribu 3 years ago.
18966-3.patch (4.1 KB) - added by azaozz 3 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 scribu3 years ago

  • Version set to 3.3

scribu3 years ago

comment:2 scribu3 years ago

  • Keywords has-patch added

While attempting to fix this, I noticed two things:

  • $wp_htmltag_class is only used once per page load, so having it as a global doesn't help performance at all
  • the beginning HTML for all the places where it's used is the same.

Hence, the _wp_admin_html_begin() function in 18966.diff.

comment:3 azaozz3 years ago

Yes, the patch makes sense. We could probably use it for other HTML headers in the admin too. The $wp_htmltag_class was set as global to give access to plugins to modify it just like $is_IE, $is_Gecko, etc. but we can add a hook later if that's necessary.

scribu3 years ago

comment:4 scribu3 years ago

Most of the pages are handled in admin-header.php.

In 18966.2.diff, network/sites.php and upgrade.php are covered.

All the rest (install, setup-config and repair) don't seem to have the function available.

comment:5 nacin3 years ago

Per IRC, let's go with the initial patch here, 18966.diff, but modify it so it uses boilerplate-like conditionals.

Other pages covered in 18966.2.diff are fine as well, but I don't really like the network admin conditional. Why not always use the root blog?

comment:6 scribu3 years ago

I don't like the conditional either, but was trying to stick to what was there.

azaozz3 years ago

comment:7 azaozz3 years ago

When using IE conditional tags we have to call do_action('admin_xml_ns'); and language_attributes(); twice, see 18966-3.patch. It's probably a tiny bit faster to check $_SERVER['HTTP_USER_AGENT'] although not as precise.

comment:8 azaozz3 years ago

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

In [18982]:

Use IE conditionals when adding the ie8 class, introduce _wp_admin_html_begin(), props scribu, fixes #18966

Note: See TracTickets for help on using tickets.