Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#18201 closed defect (fixed)

[regression] display value get reset to null in _TextBoxMixin, which fail to show time for the TimeTextBox

Reported by: kenlau Owned by: kenlau
Priority: undecided Milestone: 1.10.1
Component: Dijit Version: 1.10.0
Keywords: Cc: Terence Kent
Blocked By: Blocking:

Description

Our code extends TimeTextBox?, which fail to show the display value for the time. I dive into the dijit code a bit, it seems like there is new checking of the following function in _TextBoxMixin in 1.10.0 which resets the formattedValue to null.

_TextBoxMixin.js:

_setValueAttr: function(value, /*Boolean?*/ priorityChange, /*String?*/ formattedValue){ ....

if (this.compare(filteredValue, this.filter(this.parse(formattedValue, this.constraints))) != 0){

formattedValue = null;

}

Attachments (2)

testcase18201.html (821 bytes) - added by kenlau 5 years ago.
testcase18201.2.html (821 bytes) - added by kenlau 5 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 5 years ago by bill

Owner: set to kenlau
Status: newpending

Please attach a test case using the "attach file" button. It should be as small as possible to still reproduce the problem, almost always a single HTML file that we can load in the browser (i.e. not PHP, JSP, etc.).

Then, give exact instructions on how to reproduce the problem using your attached test file, including the browser and version to use.

The test case is necessary both to confirm that there's a bug, and for us to be able to debug the problem.

Alternately, you can give instructions on how to reproduce the problem with an existing test case (in the tests/ directory).

Thanks!

Changed 5 years ago by kenlau

Attachment: testcase18201.html added

comment:2 Changed 5 years ago by kenlau

Status: pendingnew

Attachment (testcase18201.html) added by ticket reporter.

Changed 5 years ago by kenlau

Attachment: testcase18201.2.html added

comment:3 Changed 5 years ago by kenlau

Please try the standalone test case. You can notice that the TimeTextBox? does not display the time at all. If you change to use earlier version of dojo, i.e. 1.8.6, you can see that the time is displayed properly in the TimeTextBox?.

At I have pointed out earlier in my comment, the code in problem might be in _TextBoxMixin.js

Thanks.

comment:4 Changed 5 years ago by bill

Cc: Terence Kent added
Milestone: tbd1.10.1
Summary: display value get reset to null in _TextBoxMixin, which fail to show time for the TimeTextBox[regression] display value get reset to null in _TextBoxMixin, which fail to show time for the TimeTextBox

OK, thanks for the test case.

@tkent, the code mentioned is from your f018c601b407592956dd56cd41a5edfe332d55d2 check in, can you take a look?

comment:5 Changed 5 years ago by Terence Kent

@bill

Sure, thing. I have meetings this morning, but I'll take a look tonight.

comment:6 Changed 5 years ago by Terence Kent

@bill @kenlau

I see the problem, the TimeTextBox? breaks the "round trip" from a value standpoint. If you set the value using a date object (e.g. a date of "2014-08-10 10:00AM"), and then use the compare() method with the same object confirm that the correct value was set, you'll see the problem.

I added an explicit guard against scenarios like this in ticket #17955, which is causing the issue. The solution is pretty simple, it's just implementing the compare() method in the TimeTextBox? so only the time portion of the value is considered.

I'll get that checked in tomorrow. If you need an immediate fix, stripping the non-used time components off of date before setting it will work around the issue (or just throwing an aspect around the compare method for the widget).

comment:7 Changed 5 years ago by kenlau

Have discussed with our architect, we need to get it fixed in 1.10.0 instead of 1.10.1. Thanks.

comment:8 Changed 5 years ago by dylan

You mean 1.10.1 instead of 1.11.0 presumably.

1.10.0 is already released. Bug fixes are provides in master, and then as part of the next release. We cannot go back in time and fix a bug in something that's already released, without a new release.

comment:9 Changed 5 years ago by kenlau

Not 1.11.0. You have set the milestone to 1.10.1, we would like it to be fixed in 1.10.0 if possible.

comment:10 Changed 5 years ago by dylan

1.10.1 is the bug fix release for 1.10.0. We don't do multiple releases under the same version number.

Please refer to semantic versioning for further clarification: http://semver.org/

comment:11 Changed 5 years ago by Terence Kent

@kenlau

Here's a jsfiddle showing how you can get around the issue by including about 20 lines of code before the first TimeTextBox? is used. If your custom widget includes the attached code before it loads, that should take care of the issue for you.

I'll have a patch in later today, and providing it's accepted, the next version of dojo will contain the fix.

comment:12 Changed 5 years ago by Terence Kent

@bill

I've submitted a pull request for the patch and it includes a regression test.

@kenlau

Can you please confirm that the jsfiddle patch works for you?

comment:13 Changed 5 years ago by Bill Keese <bill@…>

Resolution: fixed
Status: newclosed

In ba88f8e643398cd6b7c086847c346909ff942223/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:14 Changed 5 years ago by Bill Keese <bill@…>

In d38b0bd2e94af706b27aff49779e706b1fb903bf/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:15 Changed 5 years ago by bill

#18258 is a duplicate of this ticket.

comment:16 Changed 5 years ago by bill

#18407 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.