Problem/Motivation
The current creation of the autocomplete url_value in js_process_autocomplete()
doesn't work with enabled language prefixes and most likely also breaks in other scenarios (language subdomain?).
Reason for this is that we can't add a q
parameter on our own if url()
will generate it's own q
parameter.
Excerpt from url()
:
if (!empty($path)) {
$query['q'] = $path;
}
if ($options['query']) {
// We do not use array_merge() here to prevent overriding $path via query
// parameters.
$query += $options['query'];
}
So if $path
isn't empty $query['q']
is set and will be kept! This is exactly what happens if there are path prefixes.
Proposed resolution
Build the required URL on our own by using $base_path
and parsing the url for the frontpage provided by url()
.
Attached patch tries to do this and respect possible rewrites happening in url()
- it seems to work fine with language path prefixes.
Remaining tasks
reviews needed
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#8 | interdiff-2873484-6-8.txt | 2.45 KB | markhalliwell |
#8 | 2873484-8.patch | 3.51 KB | markhalliwell |
Comments
Comment #2
markhalliwellI really don't understand how this is any different than the current implementation, except maybe more verbose/manual.
This is, for the most part, a direct replication of form_process_autocomplete(), which a few modifications to handle the necessary
js_*
parameters.I think what you're saying is that the autocomplete path needs the proper prefix. In which case, wouldn't just the following line change suffice?
Comment #3
das-peter CreditAttribution: das-peter at Cando commented@Mark thanks for the feedback :) I try to clarify:
Unfortunately that doesn't work because we're setting
q
butq
is overwritten to<front>
inurl()
.Simply, we can not manually set the parameter
q
if we don't use clean urls.Full excerpt of the relevant handling in
url()
:If you wonder why
$path
isn't empty with<front>
- it's exactly because of the language prefix.<front>
becomes e.g./fr
- triggering the overwrite of theq
parameter.We might be able to generate the url as now, parse it and replace
q
again. Could by a bit less verbose.Is that preferred?
Comment #4
markhalliwellAh! I see what you're saying now. Hm, that's unfortunate.
I just didn't like the idea of using
parse_url
if it could have been avoided :-/The other concern I had was that it was removing the
clean_url
temporary override, which gave me pause because this is the way it is done in core.I suppose since this is manually constructing the URL (as external), it will always construct it as a non-clean url, yes?
Yes, if possible. The documentation around this needs to be a little clearer as well.
Comment #5
markhalliwellComment #6
das-peter CreditAttribution: das-peter at Cando commentedOkay here we go. I re-wrote the patch - can't say it looks any nicer :| All the parsing / glueing back together is quite nasty.
I've re-used the "glue" function from advagg: http://cgit.drupalcode.org/advagg/tree/advagg.module#n4931
And I tried to add comments to explain why we need to do all the nasty.
@Mark Please have a look and tell me what you think :) Thanks!
Comment #7
markhalliwellThis is even more verbose than the last patch and it introduces the new
js_http_build_url
function, that's only used once.I don't think that is necessary here.
Comment #8
markhalliwellConsidering that this hasn't gotten much attention and I really don't have a better idea (or time to come up with something better), I'm just going to commit this. I made a few minor spacing and typo changes as well as making
_js_http_build_url
a private function.