Context Navigation

← Previous Ticket
Next Ticket →

#18394 closed defect (fixed)

[regression] DateTextBox does not work when using dojox.date.hebrew for the datePackage

Reported by:	bill	Owned by:	bill
Priority:	undecided	Milestone:	1.10.3
Component:	Dijit	Version:	1.10.1
Keywords:		Cc:
Blocked By:		Blocking:

Description

DateTextBox? does not work when using dojox.date.herbew for the datePackage

This can be seen using the following test case: http://download.dojotoolkit.org/release-1.10.1/dojo-release-1.10.1/dojox/date/tests/test_DateTextBoxNewHebrewGreg.html

Based on a colleague's initial investigation apparently this is related to the changes made for Dojo bug report #18201, https://github.com/dojo/dijit/commit/ba88f8e643398cd6b7c086847c346909ff942223#diff-d2d1aff35a9346af3c63c86aca81921a.

Change History (10)

comment:1 Changed 7 years ago by bill

Owner:	changed from bill to Terence Kent
Status:	new → assigned

tkent, can you take a look? dateObject.getDateLocalized(locale) in dojox/date/hebrew.locale.js is failing because getDateLocalized() isn't defined. Not sure of the details beyond that though (or of how or where this should be fixed).

comment:2 Changed 7 years ago by lparsons

I added the following lines before the format and parse calls of the compare function in _DateTimeTextBox.js and it appears to have addressed our issue. Based on the error message from the dojo test case it will probably the address there as well.

            if(val1 instanceof Date && !(this.dateClassObj instanceof Date)){
                val1 = new this.dateClassObj(val1);
            }
            if(val2 instanceof Date && !(this.dateClassObj instanceof Date)){
                val2 = new this.dateClassObj(val2);
            }

comment:3 Changed 7 years ago by Terence Kent

@Bill

I'm happy to take a look at this, but it won't be until this weekend. Working through the final phase of a big project.

@Iparsons

Thanks for figuring out a fix and sharing it! If you'd like to fix it directly and make a pull request, I'm sure the team would appreciate it. If you go that route, I also try to leave behind a link to a jsfiddle with a working aspect that can dropped in by other effected users.

Last edited 7 years ago by Terence Kent (previous) (diff)

comment:4 Changed 7 years ago by bill

I've looked into this some. First of all there seems to be a race condition where it sometimes works and sometimes doesn't. But ignoring that for now...

#5074 implemented support for non-gregorian calendars by letting _DateTimeTextBox use a user specified dateClassObj (actually, specified indirectly via datePackage) rather than the native javascript Date object. The implementation though was sloppy as there are still sometimes native javascript Date objects floating around, specifically this.value is sometimes set to a native Date:

The initial value (specified in the prototype) is a Date rather than dateClassObj.
Likewise, for the initial widget properties (at least in the declarative case), value is a Date rather than a dateClassObj, because that's how the parser works.

Due to those issues, _DateTimeTextBox#_set(attr, value) is sometimes called when this.value is a Date instead of dateClassObj. _DateTimeTextBox#_set() calls compare() with that native javascript Date object which ends up failing.

So @Iparsons, your patch to make compare() handle native Date objects is good, but it seems like a workaround rather than a solution. I submitted an alternate patch in https://github.com/dojo/dijit/pull/74. @tkent and @Iparsons, take a look when you have time.

BTW, I think the patch to compare() is good for monkey patching the existing 1.10 releases. It can be done by advice around _DateTimeTextBox.prototype.compare.