#11624 closed defect (fixed)
ItemFileReadStore fetchItemByIdentity Object.prototype identifier bug
Reported by: | patrickzope | Owned by: | Jared Jurkiewicz |
---|---|---|---|
Priority: | high | Milestone: | 1.6 |
Component: | Data | Version: | 1.5 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
If any identifier of the items provided to ItemFileReadStore? is also an Object.prototype property name, ItemFileReadStore? raises an erroneous collision error.
var data = {identifier: 'id', items: [{id: 'toString', value: 'blah'}]}; var store = new dojo.data.ItemFileReadStore({data: data}); store.fetchItemByIdentity({identity: 'toString'});
result:
Error: dojo.data.ItemFileReadStore?: The json data provided by the creation arguments is malformed. Items within the list have identifier: [id]. Value collided: [toString]
Obviously, there is only one item with the identifier 'toString'. This example is contrived, but on Firefox the same can happen with the identifier 'watch', since 'watch' happens to be a nonstandard method in Object's prototype.
Another issue is as follows: fetching an item with fetchItemByIdentity when providing an identifier that is also the name of an Object.prototype property name will return that property instead of null.
var data = {identifier: 'id', items: [{id: 'key', value: 'blah'}]}; var store = new dojo.data.ItemFileReadStore({data: data}); store.fetchItemByIdentity({identity: 'toString', onItem: function (item) {console.log(item);}});
will result in "toString()" being written to console.
Attached is a patch that (I believe) fixes this issue.
This affects Dojo 1.4.x and 1.5.
Attachments (2)
Change History (9)
Changed 12 years ago by
Attachment: | ItemFileReadStore.patch added |
---|
comment:1 Changed 12 years ago by
I suspect a better fix would be to write a dedicated dictionary type or dictionary functions that avoid this pitfall and use those (everywhere).
comment:2 follow-up: 3 Changed 12 years ago by
Do you have a CLA on file? I cannot look at your patch unless you have signed a CLA.
comment:3 Changed 12 years ago by
Replying to jaredj:
Do you have a CLA on file? I cannot look at your patch unless you have signed a CLA.
I do now.
comment:4 Changed 12 years ago by
Awesome. Thank you for the fix, then. I'll go through it later and get it in this week.
comment:5 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:6 Changed 12 years ago by
Milestone: | tbd → 1.6 |
---|
Thanks for the patch and tests! Looks good. Committed.
A patch for this bug