Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#19151 closed defect (bug) (fixed)

Admin Bar Search issues in IE

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


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 4 years ago.
19151.no-search-label.png (8.1 KB) - added by SergeyBiryukov 4 years ago.
19151.2.patch (2.4 KB) - added by kurtpayne 4 years ago.
IE placeholder fix for twentyeleven theme
19151.2.2.patch (1.5 KB) - added by kurtpayne 4 years ago.
IE placeholder fix for admin bar
19151.ie8.png (9.2 KB) - added by SergeyBiryukov 4 years ago.
19151.3.patch (1001 bytes) - added by SergeyBiryukov 4 years ago.
19151.after.png (8.2 KB) - added by SergeyBiryukov 4 years ago.
With 19151.3.patch
admin-bar-without-images.png (3.6 KB) - added by toscho 4 years ago.
Accessibility problems without images.
adminbarsprite.png (1.4 KB) - added by chexee 4 years ago.
adminbarsprite.2.png (1.7 KB) - added by ocean90 4 years ago.
admin-bar-sprite.png (2.0 KB) - added by chexee 4 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 @ocean904 years ago

  • Keywords needs-patch added

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

Last edited 4 years ago by ocean90 (previous) (diff)

comment:2 @azaozz4 years ago

Yes, we need an onclick attrib on the link.

comment:3 @ocean904 years ago

  • Keywords has-patch added; needs-patch removed

Patch attached. Not tested in IE.

@ocean904 years ago

comment:4 @azaozz4 years 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 @SergeyBiryukov4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:6 @ocean904 years 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 @toscho4 years 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 @ryan4 years ago

  • Component changed from UI to Admin Bar

@kurtpayne4 years ago

IE placeholder fix for twentyeleven theme

comment:9 @kurtpayne4 years 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 @ocean904 years ago

kurtpayne, please open a new ticket for Twenty Eleven.

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

comment:12 @kurtpayne4 years 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?

@kurtpayne4 years ago

IE placeholder fix for admin bar

comment:13 @nacin4 years 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 @ryan4 years ago

See also #19149

comment:15 @azaozz4 years 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: @SergeyBiryukov4 years 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: @azaozz4 years 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.

@SergeyBiryukov4 years ago

comment:18 in reply to: ↑ 17 @SergeyBiryukov4 years 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?

@SergeyBiryukov4 years ago

comment:19 @SergeyBiryukov4 years ago

  • Keywords has-patch added; close removed

@SergeyBiryukov4 years ago

With 19151.3.patch

@toscho4 years ago

Accessibility problems without images.

comment:20 @toscho4 years ago

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

comment:21 @azaozz4 years ago

In [19407]:

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

@chexee4 years ago

@ocean904 years ago


@chexee4 years ago

comment:22 @azaozz4 years 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.