#5842 closed bug (duplicate)

Opened July 20, 2010 10:58PM UTC

Closed October 18, 2012 12:27PM UTC

Does not update field on date select

Reported by: igor.nadj@gmail.com Owned by:
Priority: minor Milestone: 1.11.0
Component: ui.datepicker Version: 1.8.2
Keywords: Cc:
Blocked by: Blocking:
Description

(Little bit of a bad title, rather it reselects the current date instead of select the hovered date)

Replication:

1. select a date (any method)

2. reopen the datepicker, use ctrl-arrow keys to select a different date. Note: the new date must be before the previously selected date. Press enter.

Expected output: field changes to new date.

Actual output: field does not change to new date.

Problem code:

...
_doKeyDown: function(event) {
...
case 13: var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv).add($('td.' + $.datepicker._currentClass, inst.dpDiv));

Bug info:

The intended behaviour here when enter is pressed is to update the state and field with the newly selected date.

It builds an array, the first element is the dayOverClass, the newly selected date. The second element I believe is the fallback, the current date.

However, the method used to build this collection, the add() function, does not guarantee that new elements will be appended to the end of the collection. In fact, it uses DOM ordering. Therefore, if your newly selected date is after the old date, you're fine, but if it's before, the order of the collection is backwards and the code uses the wrong date.

Code fix:

Replace:

case 13: var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv).add($('td.' + $.datepicker._currentClass, inst.dpDiv));

With:

case 13: var sel = [$('td.' + $.datepicker._dayOverClass, inst.dpDiv), $('td.' + $.datepicker._currentClass, inst.dpDiv)];
Attachments (0)
Change History (7)

Changed July 30, 2010 11:48AM UTC by jzaefferer comment:1

component: ui.core → ui.datepicker

Changed August 16, 2010 01:40PM UTC by jamescryer comment:2

I have found the same problem. The existing code doesn't not work because the DOM order is not as the code expects i.e., sel[0] is not always the selected date td.

My fix is:

var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv);

sel = sel.length > 0 ? sel : $('td.' + $.datepicker._currentClass, inst.dpDiv);

Changed August 16, 2010 01:54PM UTC by jamescryer comment:3

The bug can be reproduced on http://jqueryui.com/demos/datepicker/.

1) Select a date

2) Gain focus and open the datepicker by pressing enter

3) Use the ctrl+right key to move to a future day on the current month

4) Press enter.

The date field is populated with the current date not the selected date.

Please increase the priority of this bug, this is obviously a serious issue with regard to accessibility.

Changed August 16, 2010 03:36PM UTC by jamescryer comment:4

Have forked code for review.

http://github.com/jamescryer/jquery-ui/commit/75a98538e7022fef4ac3754cddf7ce9ec55ca1ad

Changed September 06, 2010 05:36PM UTC by goobsoft comment:5

I'd like to see this fixed. I love the keyboard :)

I don't like using "add" to create a set only to use the first element. What element are you going to get out of the set first? It seems to not always be the first one you put in which makes sense for a set.

Here is the solution I came up with before I found this bug report:

case 13:						
	var sel = $('td.' + $.datepicker._dayOverClass, inst.dpDiv)
	if (!sel)
		sel = $('td.' + $.datepicker._currentClass, inst.dpDiv)
	if (sel)
		$.datepicker._selectDay(event.target, inst.selectedMonth, inst.selectedYear, sel);
	else 
		$.datepicker._hideDatepicker();

Changed October 11, 2012 09:04PM UTC by scottgonzalez comment:6

milestone: TBD → 1.11.0

Changed October 18, 2012 12:27PM UTC by mikesherov comment:7

resolution: → duplicate
status: new → closed

Duplicate of #5527.