dijit/form/Select doesn't remove observe listeners when new queries are given

[haddow777]

There is a serious bug with dijit/form/Select's interactions with dojo/store/api/Store style stores that are wrapped with the dojo/store/Observable object. The setStore function of the Select tries to intelligently handle both the old style stores and these newer ones. What is supposed to happen is, I set a store using the setStore function. Through this function I will also pass a query for the select to use to filter out values to get only the ones I want in the select. While doing this, it registers with the observe function to get notifications of changes to those values represented in the query so the list of options are always up to date.

The problem I am having is, I need to be able to change the query dynamically depending on the user's interaction with my application. So I need to call setStore a number of times with different queries. This part seems to work until I update the store somewhere else. The problem is that the Select never deregisters the notifications being sent to it by the Observable wrapper for old queries. This results highly erroneous data being entered into the Select box's options list.

First, it will get results added which are outside of its current query, plus, depending on how many times I have called setStore, and how many times those calls utilize queries that match the data being added to the store, I will get multiple duplicate entries in the Select box's options list, one for each notification.

I was going to write up a test case to show you the problem, but after looking at the code in dijit/form/_FormSelectWidget.js file where the setStore function is defined, I figured I didn't need to. It is pretty obvious that the problem stems from the functionality not being fully converted over from handling the old style stores. First off, on line 351 (of dojo version 1.8.3) that starts with "this._queryRes.observe", the call to observe is done without capturing the returned handle. Without this handle, you can't remove the Observable's listener anyways. Secondly, an attempt to stop the listener does take place on line 307, but it is for the old style stores using the dojo/data/api/Notification close function.

I've written a diff that corrects this issue as well. It might not fit in with your naming conventions, so feel free to change the name of the internal variable where I stored the observe listener's handle.

Thanks.

[Jonathan Stolte]

Also note that if an observable store is used to back a Select widget used in a dgrid instance with virtual scrolling/paging... Select instances may be created and destroyed many times. The observer also needs to be removed when the Select widget is destroyed.

[dylan]

I hope to get to this ticket in time for 1.11 (setting a deadline of end of January). If not, this will get moved to 1.12.

[dylan]

Ok, after massive triage, ended up with about 80 tickets for 1.11 and 400 or so for 1.12. That's a bit unrealistic, so first I changed all 1.12 to 1.13 (with the plan to move some forward to the new 1.12. Now, I'm moving some of the 1.11 tickets that are less likely to get done this month without help to 1.11. Feel free to help out in January if you want to see this ticket land in 1.11.

[dylan]

Ticket planning... move current 1.12 tickets out to 1.13 that likely won't get fixed in 1.12.