WordPress.org

Make WordPress Core

#19151 closed defect (bug) (fixed)

Admin Bar Search issues in IE

Reported by: SergeyBiryukov Owned by: azaozz
Priority: normal Milestone: 3.3
Component: Toolbar Version: 3.3
Severity: normal Keywords: has-patch
Cc: info@…, kpayne@…

Description

In IE 8, search throws a JS error:

Message: 'event.target.value' - is null or not an object
Line: 445
Char: 1
Code: 0

In IE 9, "Search" label is not visible.

Attachments (11)

19151.patch (395 bytes) - added by ocean90 20 months ago.
19151.no-search-label.png (8.1 KB) - added by SergeyBiryukov 20 months ago.
19151.2.patch (2.4 KB) - added by kurtpayne 20 months ago.
IE placeholder fix for twentyeleven theme
19151.2.2.patch (1.5 KB) - added by kurtpayne 19 months ago.
IE placeholder fix for admin bar
19151.ie8.png (9.2 KB) - added by SergeyBiryukov 19 months ago.
19151.3.patch (1001 bytes) - added by SergeyBiryukov 19 months ago.
19151.after.png (8.2 KB) - added by SergeyBiryukov 19 months ago.
With 19151.3.patch
admin-bar-without-images.png (3.6 KB) - added by toscho 19 months ago.
Accessibility problems without images.
adminbarsprite.png (1.4 KB) - added by chexee 19 months ago.
adminbarsprite.2.png (1.7 KB) - added by ocean90 19 months ago.
Magic
admin-bar-sprite.png (2.0 KB) - added by chexee 19 months ago.

Download all attachments as: .zip

Change History (33)

comment:1 ocean9020 months ago

  • Keywords needs-patch added

The JS is removed in #19159. But I think we need something similar again because it still links to # which ends in scrolling to top.

Version 0, edited 20 months ago by ocean90 (next)

comment:2 azaozz20 months ago

Yes, we need an onclick attrib on the link.

comment:3 ocean9020 months ago

  • Keywords has-patch added; needs-patch removed

Patch attached. Not tested in IE.

ocean9020 months ago

comment:4 azaozz20 months ago

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

In [19160]:

Admin-bar.css refresh, add back IE compat. "return false" so clicking on the search box doesn't scropp the page to the top, props ocean90, fixes #19153, fixes #19151

comment:5 SergeyBiryukov20 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

In IE 9, "Search" label is still invisible (see the screenshot).

comment:6 ocean9020 months ago

IE 7: http://cl.ly/BZxI
IE 8: http://cl.ly/BZpO
IE 9: http://cl.ly/BZe4

The placeholder attribute is supported in all major browsers, except Internet Explorer.

It's the same as in Twenty Eleven. We could make the background color for IE 9 white too.

comment:7 toscho20 months ago

  • Cc info@… added

The main problem here is the usage of the 'placeholder' attribute: It should not be used as a replacement for the 'label' element. Nobody will type 'Search' into the box. :)

Screenreader users with IE don’t have access to the 'placeholder' attribute, no matter how it is styled.

comment:8 ryan20 months ago

  • Component changed from UI to Admin Bar

kurtpayne20 months ago

IE placeholder fix for twentyeleven theme

comment:9 kurtpayne20 months ago

  • Cc kpayne@… added

The missing search label is a problem for twentyeleven, too. screenshot. 19151.2.patch is an extremely simple fix for twentyeleven that doesn't rely on jQuery.

If a javascript fix for IE is okay, I can write a patch for the admin side, too. Just wanted to get feedback on this first.

comment:10 ocean9019 months ago

kurtpayne, please open a new ticket for Twenty Eleven.

But -1 for such a big file for a simple placeholder attribute.

comment:12 kurtpayne19 months ago

@ocean90, #19230 created per your request. Yes, patch file is large because jQuery isn't available in twentyeleven. Patch is also readable and commented. Minified, it's ~825 bytes. I looked at other approaches for solving the placeholder problem in IE, and this was the simplest approach. Is there a better way to solve this?

kurtpayne19 months ago

IE placeholder fix for admin bar

comment:13 nacin19 months ago

This placeholder stuff is pretty complicated. Why not just wrap it in a <label> for screen readers, and convert it to a simple icon, no text? (The icon will need to be lighter to contrast better.) WP.com's admin bar implements this and it works well (though obviously our search button will not be flush right).

Even better, the search box should probably appear below the admin bar, rather than grow within the admin bar. (Think how Shortlink works -- same deal.) This prevents buttons from being temporarily pushed off. This is likely for 3.4 unless something else comes up that requires a bigger re-work.

comment:14 ryan19 months ago

See also #19149

comment:15 azaozz19 months ago

  • Keywords close added; has-patch removed

Currently the search box is displayed in IE directly, no "Search" text overlay/placeholder. Thinking that's good enough for now. It degrades well.

comment:16 follow-up: SergeyBiryukov19 months ago

The problem isn't the missing placeholder. Unlike the one in Twenty Eleven, the search icon in Admin Bar is barely visible in IE 9: 19151.no-search-label.png.

comment:17 in reply to: ↑ 16 ; follow-up: azaozz19 months ago

Replying to SergeyBiryukov:

It is barely visible in all browsers, perhaps we need a lighter version of it that will be visible both on the dark background and on white.

SergeyBiryukov19 months ago

comment:18 in reply to: ↑ 17 SergeyBiryukov19 months ago

Replying to azaozz:

It is barely visible in all browsers

In IE 8 it's better (see the screenshot). Could we add a browser-specific class for IE 9 (along the lines of [19368]) to make it white there too?

SergeyBiryukov19 months ago

comment:19 SergeyBiryukov19 months ago

  • Keywords has-patch added; close removed

SergeyBiryukov19 months ago

With 19151.3.patch

toscho19 months ago

Accessibility problems without images.

comment:20 toscho19 months ago

If there is no placeholder and images are disabled it will be really hard just to find the search field.

comment:21 azaozz19 months ago

In [19407]:

Admin bar: add class for IE9 and fix background color in search, props SergeyBiryukov, see #19151

chexee19 months ago

ocean9019 months ago

Magic

chexee19 months ago

comment:22 azaozz19 months ago

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

In [19413]:

Admin bar: lighter search icon, props chexee, fixes #19151

Note: See TracTickets for help on using tickets.