Changes between Initial Version and Version 3 of Ticket #1802


Ignore:
Timestamp:
03/01/11 02:03:13 (6 years ago)
Author:
fbennett
Comment:

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #1802

    • Property Cc ajlyon added
    • Property Component changed from uncategorized to interface
    • Property Summary changed from localeCompare() returns incorrect values when invoke in a notifier to localeCompare() returns incorrect values when invoked in a notifier
    • Property Version changed from 1.0 to 2.1
    • Property Type changed from enhancement to defect

  • Ticket #1802 – Description

    initial v3  
    1 I think there is a problem in itemTreeView.js, in the madeChanges block at lines 500 to 584. The function invokes this.sort(), which ultimately invokes localeCompare() on strings for comparison. When invoked from the main thread (?), localeCompare() returns correct values, so "An Apple" comes before "A Pear" in a sort. But when invoked via the notifier, the sort comes out the other way. 
     1I think there is a problem in itemTreeView.js, in the madeChanges block at lines 500 to 584 of the current revision (rr8425). The function invokes this.sort(), which ultimately invokes localeCompare() on strings for comparison. When invoked from the main thread (?), localeCompare() returns correct values, so "An Apple" comes before "A Pear" in a sort. But when invoked via the notifier, the sort comes out the other way. 
    22 
    3 The attached screenshot traces the relevant functions. Everything after the == SORT AFTER CHANGES === line comes from the notifier invocation. The string comparisons outlined in yellow are for the same two strings, first in one sequence ordering, then in the other. One of the comparisons should be 1, but both are -1. In the notifier invocation, localeCompare() is apparently (judging from what I see on the screen) performing a straight binary string sort, so "A Pear" would come out before "An Apple" (i.e. the space is treated as significant). 
     3The attached screenshot traces the relevant functions. Everything after the == SORT AFTER CHANGES === line comes from the notifier invocation. The string comparisons outlined in yellow are for the same two strings, first in one sequence ordering, then in the other. One of the comparisons should be 1, but both are -1. 
    44 
    5 The different bubbles up into the UI when an item is dropped onto a collection. The center pane then gets resorted in binary string order, so the user sees a change in the listing even though the operation hasn't touched the content of entries. 
     5In the notifier invocation, localeCompare() is apparently (judging from what I see on the screen) performing a straight binary string sort, so "A Pear" would come out before "An Apple" (i.e. the space is treated as significant). 
     6 
     7The difference bubbles up into the UI when an item is dropped onto a collection. The center pane then gets resorted in binary string order, so the user sees a change in the listing even though the operation hasn't touched the content of entries. 
    68 
    79As a workaround (possibly a fix, but I don't know the code well enough to be sure), I've replaced all this.sort() invocations inside the notifier with this_needsSort = true. This seems to trigger a sort operation (there is a pause after dropping the item from a largeish library), and the ordering returned is the same as we get from a fresh visit to the target collection or library (so the user sees no change). 
    810 
    911I'm not sure if this has any bad side effects, but it seems promising as a starting point. 
     12 
     13(I've tested this only in the multilingual branch, but I can't see anything in the code that would otherwise account for this; and the comparison values shown in the screenshot come directly from localeCompare(), so there doesn't seem to be a mistake there.)