Replace suggest.js with jQuery autocomplete

[Ryan J Ollos]

suggest.js was deprecated in [11081] / #7355, to be replaced with jQuery autocomplete. The only use of suggest.js is:

• tags/trac-1.1.6/trac/versioncontrol/web_ui/changeset.py@:1228#L1228​
• tags/trac-1.1.6/trac/versioncontrol/templates/diff_form.html@:23#L20​.

Concurrently we can look at resolving #8471 and #8429.

[Peter Suter]

Just in case this safes anyone some time:

• You can find that use of suggest.js on diff. (Available via Browse Source → View changes… button)
• ​jQuery UI Autocomplete Examples
• ​jQuery UI Autocomplete API Docs
• Sketch of a patch​

[Jun Omae]

Replying to psuter:

• Sketch of a patch​

At least, three things:

• We should keep parameter naming for compatibilities (q → term)
• Use to_json() instead of JSONEncoder.encode
• It would be good to rename variable elem since the variable is a list of path, e.g. paths

[walty <walty8@…>]

Basically the same patch as psuter, just modified the 3 places mentioned by jomae.

For the query string problem ( term → q), there is actually no easy way to do that except providing a source function. The reference is here: ​http://stackoverflow.com/questions/10791968/can-the-default-term-name-passed-in-the-jquery-ui-autocomplete-feature-be-ch

[Ryan J Ollos]

The changes look good to me. I added some minor modification in log:rjollos.git:t12169_suggest_js_replacement. I'll test in additional browsers tomorrow.

[Ryan J Ollos]

What are the use cases for keeping compatibility in parameter naming? [5097dc7b/rjollos.git] will keep the server-side API backward compatible. It wouldn't cover the use-case of implementing an IRequestFilter for modifying the xhr request. I'm always in favor of backwards compatibility, but it doesn't seem worthwhile to add a more complex solution unless we have a concrete use case. This change is being made in a major release, so API changes are acceptable even though we want to minimize them.

[Jun Omae]

No use-cases for me. However, no way to make sure of that all plugins and users except Trac core don't use q parameter in the AnyDiffModule.

The server-side API backward compatibilities in [5097dc7b/rjollos.git] looks good to me.

[Jun Omae]

Another suggestions:

1. It would be good to use window.location.pathname rather than hard-coded diff url.

   -        $("#anydiff input[name$='_path']").autocomplete({source: "diff"});
   +        $("#anydiff input[name$='_path']").autocomplete({source: window.location.pathname});
   

2. .encode('utf-8') is redundant since to_json() always returns str instance. In Trac 1.0.x with Python 2.5, to_json() returns str or unicode instance.

   -            content = to_json(paths).encode('utf-8')
   +            content = to_json(paths)
   

[Ryan J Ollos]

suggest.js displays directory entries in bold. I've added similar behavior in rjollos.git:t12169_suggest_js_replacement.1. Overriding _renderItem required that jQuery UI be upgraded (#11019). The changes will require much more extensive testing than I've done so far.

[Jun Omae]

What are the use cases for keeping compatibility in parameter naming? [5097dc7b/rjollos.git] will keep the server-side API backward compatible. It wouldn't cover the use-case of implementing an IRequestFilter for modifying the xhr request.

Sorry. I didn't think straight. I understand no need to keep parameter naming for compatibilities since response for the xhr request is modified (api change) in the proposed changes. It is enough by handling only term parameter.

-            term = req.args.get('q') or req.args.get('term')
+            term = req.args.get('term')

[Ryan J Ollos]

After additional revisions the changes have been committed to the trunk in [14316].

[walty <walty8@…>]

Just found a minor issue, might be it's better add a return statement after the ajax response?

trunk/trac/versioncontrol/web_ui/changeset.py@14316:1202#L1200​

Otherwise for each ajax request, all the logic below that would still be executed (though no effect).

[Jun Omae]

Just found a minor issue, might be it's better add a return statement after the ajax response?

Not needed. The req.send raises RequestDone exception that indicates whether request processing has completed and a response was sent.

[walty <walty8@…>]

actually I tried to add some debug messages on line 1204 and line 1230 last night, and found them did get executed. however when I tried again this morning, the debug message seems to be correct now.

I guess I mixed up the message of http request and xhr request by yesterday.

sorry about that.