Currently the theme and theme token are not sent as part of the request, due to the removal of ajax_page_state. However, due to the checks carried out in menu_get_custom_theme() we require these variables for the correct theme to be used for rendering.
I saw this problem manifest when using an administrative AJAX view. The HTML returned was being rendered using a template from the front-end theme rather than the admin theme, because theme detection couldn't be performed from the request.
The fix here is to at least pass the theme and theme_token along with the request, and alter the theme detection to allow GET to be used instead of POST (see ajax_base_page_theme()). This will make the caching less effective given multiple sessions and distinct theme tokens, but will make the behaviour correct (which I would argue beats better caching!).
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | views_ajax_get-theme_detection-2938055-D8-12.patch | 597 bytes | id.aleks |
| #10 | views_ajax_get-theme_detection-2938055-D8-10.patch | 444 bytes | leon kessler |
| #5 | views_ajax_get-theme_detection-2938055-D8-2.patch | 409 bytes | leon kessler |
| #2 | views_ajax_get-theme_detection-2938055-1.patch | 2.19 KB | ben.kyriakou |
Comments
Comment #2
ben.kyriakou commentedPatch attached
Comment #3
ben.kyriakou commentedUpon further investigation it looks like this isn't a particularly useful approach because the theme token changes between requests, invalidating caching completely. This is still a useful change, but it may have to use a different mechanism to verify theme access.
Comment #4
leon kessler commentedInteresting, this is the first time I've seen an issue caused by the request stripping.
It's also an issue on the Drupal 8 version, so we should look at a solution there first (and then possibly back-porting to Drupal 7).
Comment #5
leon kessler commentedJust looked into this, and looks like it might be an easy change...
This is not what I found to be the case. The theme token is generated using
\Drupal::csrfToken()->get($active_theme_key);(in system_js_settings_alter). It's always based off the machine name of the theme, so does not change between requests.This is for Drupal 8, so perhaps it works differently for Drupal 7.
Here is a patch for Drupal 8.
Comment #6
geek-merlinThe theme token is based on the user session so it should not change between requests of the same anonymous browser session, but changes between different anonymous users.
Which on nondefault theme makes us run into #2177975-8: Page cache breaks nondefault ajax theme on GET, see my comments there.Correction. Fromthe IS:
> The fix here is to at least pass the theme and theme_token along with the request, and alter the theme detection to allow GET to be used instead of POST (see ajax_base_page_theme()).
> This will make the caching less effective given multiple sessions and distinct theme tokens, but will make the behaviour correct (which I would argue beats better caching!).
Right. And wrong caching is not an option and can even disclose information.
>... and alter the theme detection to allow GET to be used instead of POST (see ajax_base_page_theme()).
This should not be necessary for D8 as \Drupal\Core\Theme\AjaxBasePageNegotiator::determineActiveTheme uses request->get().
Comment #7
geek-merlinComment #8
leon kessler commentedDoes this mean the patch I previously posted needs work? (Has been a while since I looked at).
Comment #9
geek-merlin#8: The patch code looks straightforward and good. Just needs someone to test it manually (not me currently).
Raising prio as this can cause information disclosure.
Comment #10
leon kessler commentedRe-rolling patch.
Comment #11
leon kessler commentedIf this breaks the cache between requests, then I agree it makes using the module effectively useless.
Perhaps another solution is to detect whether a non-default theme is in use, and conditionally disable the module functionality? (With a notification message to the admin user, and to watchdog).
Comment #12
id.aleks commentedHello Collegues. I believe that
options.data['ajax_page_state[libraries]'] = drupalSettings.ajaxPageState.libraries;needs to be added as well in order to prevent returning of already loaded files. See docroot/core/misc/ajax.es6.js. Adding this line to the existing patch.Comment #13
akalam commentedThe patch on #12 is fixing another patch for me: The ajax js libraries are getting added to the project again and again until the site throws a javascript error "Maximum call stack" when trying to do an ajax request.
Comment #14
rcodinaBe aware of custom implementations of ThemeSwitchNegotiator because they may be the source of errors when Views Ajax Get is enabled. I had a custom implementation and I had to add and "if" in case of AJAX requests.