Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#8192 closed bug (notabug)

IE Focus error when element is hidden

Reported by: dkonopacki Owned by:
Priority: minor Milestone: 1.9.0
Component: ui.datepicker Version: 1.8.18
Keywords: Cc:
Blocked by: Blocking:

Description

When a UI element or its parent is set to visibility:hidden, IE is throwing:

Can't move focus to the control because it is invisible, not enabled, or of a type that does not accept the focus.

Specifically, if attempting to create a Datepicker instance on an input[type="text] field whose parent is set to hidden, JQuery UI core does a .is(':visible') check on the element itself, which is returning the hidden object and causing IE to error. Please see this JSFiddle test case:

http://jsfiddle.net/R7UzC/1/

The Datepicker target element is hidden because its parent DIV element is hidden, but .datepicker('show') still triggers .focus().

The offending line, though, occurs in UI.Core, Line 8512. Try/catch should be used, as it is the most backwards compatible option for IE.

Please see the following for further details:

http://bugs.jqueryui.com/ticket/4878

http://bugs.jqueryui.com/ticket/5052

Change History (7)

comment:1 Changed 8 years ago by Scott González

Component: ui.coreui.datepicker
Resolution: invalid
Status: newclosed

You shouldn't be calling show on a datepicker that's attached to an element which cannot receive focus.

comment:2 Changed 8 years ago by dkonopacki

While I agree that datepicker should not receive an execution for "show" on an element that is hidden, I disagree that the defect is invalid. There should still be a check for visibility on the element and ancestor elements. UI.Core already does this, so there's little reason why Datepicker should not do it for consistency.

comment:3 Changed 8 years ago by Scott González

What specifically are you referring to when you say UI.Core already does this?

comment:4 Changed 8 years ago by dkonopacki

In the definition for the focusable function (jquery.ui.core), part of the check to test whether or not an element can receive focus is to check its visibility. Line 184 contains the comment, "the element and all of its ancestors must be visible". It would seem inconsistent that in the core there would be a test for visibility on the called element and all ancestors, but in datepicker the test for an element about to receive focus is simply "is this element visible". There doesn't seem to be a plausible reason why one function should have an ancestor check, but another does not. Especially since datepicker does check for some sort visibility before trying to call focus.

Additionally, if the code is not modified I would argue that the defect still stands as "valid" because the documentation is incorrect. Nowhere does the datepicker docs state that a form field using datepicker is required to be visible. In our use case, a different action other than focusing on the element to which datepicker was attached caused the datepicker to display. Additional confusion was caused by the fact that only IE manifested the problem. If calling "show" requires the element to be visible, though I'm doubtful since the code partially supports checking for visibility before setting focus, then it might be best to clearly state this.

comment:5 Changed 8 years ago by Scott González

The focusable selector is not related to actually setting focus (other than you might use it to determine which element to automatically focus in some scenarios).

See http://bugs.jquery.com/ticket/11054#comment:6

comment:6 in reply to:  5 Changed 8 years ago by dkonopacki

And why is "show" on datepicker actually setting focus on the element? Shouldn't that be at least configurable? I feel that saying "focusable selector is not related to actually setting focus" is an irrelevant argument, as there isn't any specific reason for calling focus on the attached element. When "show" executes, a div is appended to the body that is independent of the element associated with the plugin. I'm missing the justification for even executing focus on the bound element.

Further, the comments you reference only apply insofar as dismissing the use of try/catch. Based upon the argument presented, I can agree with that. But, reply #4 of that same thread should be the one referenced. If the point of calling "show" was specifically execute a focus() command, I could agree with your last statement. But the intended purpose is to "[c]all up a previously attached date picker". The problem still resides with the plugin - either the code is wrong and should check for visibility on all ancestors before calling focus, calling focus shouldn't happen at all or be configurable, or the documentation needs to change to reflect the fact that "show" can only be called from a datepicker bound element that is visible.

comment:7 Changed 8 years ago by Scott González

I added a note to the documentation.

The input gets focus because users are allowed to type a date.

Note: See TracTickets for help on using tickets.