Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#8480 closed defect (fixed)

dojo.data.util.sorter.BasicComparator does not handle nulls properly

Reported by: criecke Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.3
Component: Data Version: 1.2.3
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

According to JavaScript spec, the expression a > b is always true if either a or b is null. BasicComparator is spec'ed to return 1 if a===null && b!==null and -1 if a!==null && b===null. However, that's not second case does not hold because a>b is true, thus it returns 1. That messes up sorts with null values because it effectively says null>999 and 999>null.

Attachments (2)

test_compare.html (571 bytes) - added by Jared Jurkiewicz 11 years ago.
sorter.patch (1.3 KB) - added by Jared Jurkiewicz 11 years ago.
Patch for this. Code reduction (I believe) and should nor properly push null/undefined down in a sort.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by bill

Component: GeneralData
Description: modified (diff)
Owner: changed from anonymous to Jared Jurkiewicz

comment:2 Changed 11 years ago by Jared Jurkiewicz

Milestone: tbdfuture

Agreed this is an issue. Not sure of a small fix for it (which is critical for its position in core.) So ... I need to look at this for a bit and determine the best reduction.

comment:3 Changed 11 years ago by Jared Jurkiewicz

Are you sure your claim is correct? I've tested this now on three browsers:

Firefox 2

IE 7

Safari 3.1

All of them return:

null>999: false 999>null: true 999>undefined: false undefined>999: false

Which does not support the statement that a > b if either a or b is null.

I'llattached my exact testcase showing this.

comment:4 Changed 11 years ago by Jared Jurkiewicz

Are you sure your claim is correct? I've tested this now on three browsers:

Firefox 2

IE 7

Safari 3.1

All of them return:

null>999: false 999>null: true 999>undefined: false undefined>999: false

Which does not support the statement that a > b is true if either a or b is null.

I'll attach my exact testcase showing this.

Changed 11 years ago by Jared Jurkiewicz

Attachment: test_compare.html added

comment:5 Changed 11 years ago by Jared Jurkiewicz

Can you provide code illustrating where the current compare fails?

comment:6 Changed 11 years ago by Jared Jurkiewicz

Resolution: invalid
Status: newclosed

Marking invalid till testcase provided showing where the compare currently fails.

comment:7 Changed 11 years ago by Jared Jurkiewicz

Resolution: invalid
Status: closedreopened

Reopening. Doing a bit more investigation, but I think I understand your concern/issue.

Changed 11 years ago by Jared Jurkiewicz

Attachment: sorter.patch added

Patch for this. Code reduction (I believe) and should nor properly push null/undefined down in a sort.

comment:8 Changed 11 years ago by Jared Jurkiewicz

Tested on:

IE 6

IE 7

Firefox 2

Safari 3.1

Opera 9.6

SeaMonkey? 1.1.2

Google Chrome

comment:9 Changed 11 years ago by Jared Jurkiewicz

Resolution: fixed
Status: reopenedclosed

(In [16490]) Tweak to fix case in null comparision. fixes #8480

comment:10 Changed 11 years ago by Adam Peller

Milestone: future1.3

batch move of tickets marked 'future' in the 1.3 timeframe

Note: See TracTickets for help on using tickets.