Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#8931 closed defect (fixed)

FilteringSelect: doing redundant dojo.store.fetch() calls

Reported by: bill Owned by: Douglas Hays
Priority: high Milestone: 1.4
Component: Dijit - Form Version: 1.3.0b3
Keywords: Cc:
Blocked By: Blocking:

Description

See tests/form/robot/_autoComplete.html?testWidget=dijit.form.FilteringSelect? [17050] on FF/mac or FF/win.

Getting strange results as per what queries ComboBox is sending to the data store. For example, on mac for the "enter key handling" test, where the only query issued should be for "co*", I get three queries:

START query on {} (0 chars), delay = 2000
START query on Co* (3 chars), delay = 500
START query on Co (2 chars), delay = 1000
END query on Co* (3 chars), delay = 500
END query on Co (2 chars), delay = 1000
END query on {} (0 chars), delay = 2000

FF on the PC is also strange, but different:

START query on Co* (3 chars), delay = 500
START query on Co* (3 chars), delay = 500
END query on Co* (3 chars), delay = 500
END query on Co* (3 chars), delay = 500

For some reason it's executing the same query twice.

Attachments (1)

8931.patch (9.2 KB) - added by Douglas Hays 10 years ago.
Make FilteringSelect? and ComboBox? smarter about canceling pending queries and tweak the automated test to have fewer random failures

Download all attachments as: .zip

Change History (11)

comment:1 Changed 10 years ago by bill

Commented out these tests in [17084] and [17085]. Reinstate once this problem is fixed.

comment:2 Changed 10 years ago by bill

Owner: set to Douglas Hays

Maybe you can work on this one too, Doug? It's similar to #8950.

Not sure if it can be classified as a bug but seems like a performance problem anyway.

comment:3 Changed 10 years ago by bill

(In [17696]) Update test cases for #8950 and #8931 (ComboBox?/data store interaction). Refs #8931 and #8950 !strict.

comment:4 Changed 10 years ago by Douglas Hays

Milestone: tbd1.4

Changed 10 years ago by Douglas Hays

Attachment: 8931.patch added

Make FilteringSelect? and ComboBox? smarter about canceling pending queries and tweak the automated test to have fewer random failures

comment:5 Changed 10 years ago by Douglas Hays

After patch:

GROUP "race conditions" has 2 tests to run
START query on {} (0 chars), delay = 2000
CANCEL query on {} (0 chars), delay = 2000
START query on C* (2 chars), delay = 1000
CANCEL query on C* (2 chars), delay = 1000
START query on Co* (3 chars), delay = 500
END query on Co* (3 chars), delay = 500
PASSED test: query canceling on new input (#8950) 0 ms
START query on {} (0 chars), delay = 2000
CANCEL query on {} (0 chars), delay = 2000
START query on Co* (3 chars), delay = 500
CANCEL query on Co* (3 chars), delay = 500
START query on Co (2 chars), delay = 1000
END query on Co (2 chars), delay = 1000
Total time for GROUP " race conditions " is 0 ms
PASSED test: pressing enter before search returns 0 ms

Patch probably needs some review and/or testing.

comment:6 Changed 10 years ago by bill

I didn't expect the initial query on {} when focusing an empty ComboBox/FilteringSelect. I thought we didn't do a query at that stage unless the user clicked the down arrow (or pressed the down arrow key)? In any case the robot test is strange in that it has 100ms delay between focusing the control and typing the first character, which is exactly equal to the searchDelay. We should bump up the delay in the robot test to make things clearer.

Also, in "query canceling on new input (#8950)" the last query on "Co" (after pressing the ENTER key) seems like it should be running on FilteringSelect, but not on ComboBox, right?

comment:7 Changed 10 years ago by Douglas Hays

how to handle blank values discussed in #7630

comment:8 Changed 10 years ago by bill

Yup, chatted w/Doug about this. The initial query on {} is caused by the attr('value', ) in the test, which according to #7630 is OK.

And as for the last query on "Co", FilteringSelect runs that query but ComboBox currently doesn't. Looks like we want it to run on ComboBox too, to make item available (if a matching item exists), see the discussion in #6022.

So, looks like this patch is good, after dealing with the ComboBox issue one way or another, so that the test doesn't get a test failure.

comment:9 Changed 10 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [19887]) Fixes #8931. Cancel curent fetch if a new query arrives too quickly. Fix robot testcase to expect cancels from slowdatastore.

comment:10 Changed 8 years ago by bill

Component: DijitDijit - Form
Note: See TracTickets for help on using tickets.