X-HoverXRef-Version header doesn't get set consistently on readthedocs.io

[shiftinv]

First of all, thanks for this extension, it's a really neat feature!

There's one issue I've experienced specifically on readthedocs.io, where the custom X-HoverXRef-Version header doesn't get set on API requests consistently between page reloads, i.e. it only gets added to requests in some instances.

This first appeared since we noticed infrequent/inconsistent CORS issues on a custom domain used with readthedocs.io, which after some debugging turned out to be what I suppose you'd call a race condition between hoverxref.js and readthedocs' own readthedocs-doc-embed.js, as both use ajaxSetup to define the beforeSend callback:

sphinx-hoverxref/hoverxref/_static/js/hoverxref.js_t

Lines 97 to 101 in 3b47d75

	$.ajaxSetup({
	beforeSend: function(request) {
	request.setRequestHeader('X-HoverXRef-Version', '{{ http_hoverxref_version }}');
	}
	});

https://github.com/readthedocs/readthedocs.org/blob/b958bb8d04c454324d612345890b13af54a19eb6/readthedocs/core/static-src/core/js/doc-embed/footer.js#L31-L37

If the former ajaxSetup runs last, the X-HoverXRef-Version header gets set on every request, which results in CORS issues when not using the proxied API endpoint, as Access-Control-Allow-Headers doesn't contain that header.
If the latter runs last, hoverxref's beforeSend gets overwritten, the header never gets set, and CORS isn't an issue.

This primarily happens in Firefox, I haven't been able to reproduce it in Chrome, so perhaps the order in Chrome is deterministic?

After some tinkering and finding #134 I figured out that the trivial solution is of course to just use the proxied API endpoint :) However I still felt like filing this issue since it doesn't change the fact that the header doesn't seem to get set consistently.

(example page with a few xrefs: https://rtd-test.shiftinv.cc/en/test-rtd-cors/whats_new.html#new-features)

[humitos]

Thanks a lot for such a detailed description!

@agjohnson @nienn do you know a good way to solve this problem? is it possible to "append" the function to be called? or at least to check if there are already some defined ones to make the append manually and then re-set it?

[agjohnson]

ajaxSetup configures all calls to ajax at a global level, is there a reason this configuration can't happen on the individual ajax call?

I've never used this method of global configuration, but jQuery doesn't seem to recommend it:

Note: The settings specified here will affect all calls to $.ajax or Ajax-based derivatives such as $.get(). This can cause undesirable behavior since other callers (for example, plugins) may be expecting the normal default settings. For that reason we strongly recommend against using this API. Instead, set the options explicitly in the call or define a simple plugin to do so.

So perhaps configuring the individual $.ajax({}) call with the same attribute is the best option.

[humitos]

@davidfischer @agjohnson does it make sense to review the ajaxSetup call done in the footer.js as well and add it only where it's needed?

[humitos]

We just released 1.0.1. @shiftinv can you confirm that it works as you expected? 🙏🏼

[shiftinv]

Yup, looks like it's consistent now. Thanks!