Make WordPress Core

Opened 15 years ago

Closed 15 years ago

#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: Focuses:

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

Download all attachments as: .zip

Change History (33)

#1 @ocean90
15 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 15 years ago by ocean90 (previous) (diff)

#2 @azaozz
15 years ago

Yes, we need an onclick attrib on the link.

#3 @ocean90
15 years ago

  • Keywords has-patch added; needs-patch removed

Patch attached. Not tested in IE.

@ocean90
15 years ago

#4 @azaozz
15 years ago

  • Owner set to azaozz
  • Resolutionfixed
  • Status newclosed

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

#5 @SergeyBiryukov
15 years ago

  • Resolution fixed
  • Status closedreopened

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

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

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

#8 @ryan
15 years ago

  • Component UIAdmin Bar

@kurtpayne
15 years ago

IE placeholder fix for twentyeleven theme

#9 @kurtpayne
15 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.

#10 @ocean90
15 years ago

kurtpayne, please open a new ticket for Twenty Eleven.

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

#12 @kurtpayne
15 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?

@kurtpayne
15 years ago

IE placeholder fix for admin bar

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

#14 @ryan
15 years ago

See also #19149

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

#16 follow-up: @SergeyBiryukov
15 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.

#17 in reply to: ↑ 16 ; follow-up: @azaozz
15 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.

#18 in reply to: ↑ 17 @SergeyBiryukov
15 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?

#19 @SergeyBiryukov
15 years ago

  • Keywords has-patch added; close removed

@SergeyBiryukov
15 years ago

With 19151.3.patch

@toscho
15 years ago

Accessibility problems without images.

#20 @toscho
15 years ago

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

#21 @azaozz
15 years ago

In [19407]:

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

@ocean90
15 years ago

Magic

#22 @azaozz
15 years ago

  • Resolutionfixed
  • Status reopenedclosed

In [19413]:

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

Note: See TracTickets for help on using tickets.