WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 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:

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

Download all attachments as: .zip

Change History (33)

comment:1 @ocean903 years 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 3 years ago by ocean90 (next)

comment:2 @azaozz3 years ago

Yes, we need an onclick attrib on the link.

comment:3 @ocean903 years ago

  • Keywords has-patch added; needs-patch removed

Patch attached. Not tested in IE.

@ocean903 years ago

comment:4 @azaozz3 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 @SergeyBiryukov3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:6 @ocean903 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 @toscho3 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 @ryan3 years ago

  • Component changed from UI to Admin Bar

@kurtpayne3 years ago

IE placeholder fix for twentyeleven theme

comment:9 @kurtpayne3 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 @ocean903 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 @kurtpayne3 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?

@kurtpayne3 years ago

IE placeholder fix for admin bar

comment:13 @nacin3 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 @ryan3 years ago

See also #19149

comment:15 @azaozz3 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: @SergeyBiryukov3 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: @azaozz3 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.

@SergeyBiryukov3 years ago

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

@SergeyBiryukov3 years ago

comment:19 @SergeyBiryukov3 years ago

  • Keywords has-patch added; close removed

@SergeyBiryukov3 years ago

With 19151.3.patch

@toscho3 years ago

Accessibility problems without images.

comment:20 @toscho3 years ago

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

comment:21 @azaozz3 years ago

In [19407]:

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

@chexee3 years ago

@ocean903 years ago

Magic

@chexee3 years ago

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