#343542: Enable optional auto-refreshing views using AJAX adds some new complexity to ajax_view.js. In preparation for D7, there's a move to start using CTools.

To ease the upgrade, we should look at rewriting ajax_view.js to duplicate what CTools is doing.

CommentFileSizeAuthor
#23 views-Make_AJAX_paging_use_GET-591302-ish-23.patch752 bytessokrplare
#22 591302.4.with_.956186.1.against_Views_7.x-3.3.c.patch1.65 KBsokrplare
#18 591302.5.patch11.85 KBkatbailey
#15 591302.4.patch11.75 KByhahn
#15 591302.4.with_.956186.1.patch10.81 KByhahn
#14 591302.3.patch11.16 KByhahn
#13 591302.2.patch16.07 KByhahn
#12 591302.1.patch11.19 KByhahn

Comments

merlinofchaos’s picture

merlinofchaos commented

For 2.x, I think we should duplicate CTools ajax.inc and ajax-responder.js, and in 3.x actually rely on CTools for this functionality.

drewish’s picture

drewish commented

subscribe

dixon_’s picture

dixon_
he/him
Primary language English
Location France
commented

+1 for creating a dependency on CTools for 3.x and share the code base for stuff like this.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

I've taken a first look at requirements.

Currently we have a separate menu callback for AJAX requests and special handling in the JS to parse out view attributes, which are passed along with the request as post variables.

The basic changes needed look to be:

1. Use a single callback for nojs and ajax requests. Rewrite to test for 'ajax' and conditionally use ctools_ajax_render(). The most obvious place to put this would be views_page() (which we would then have to add access tests to and expose to the menu system). Something like:


  ctools_include('ajax');
  // Load the view
  if ($view = views_get_view($name)) {
    $output = $view->execute_display($display_id, $args);
    if ($type == 'ajax') {
      $commands = array();
      $commands[] = ctools_ajax_command_replace('#some-div', $return);
      ctools_ajax_render($commands);
    }
    else {
      ctools_add_js('ajax-responder');
      return $output;
    }
  }

2. Append /nojs to all views links and to the target of exposed filter forms.

3. Also append to these links and target the minimum data to identify the request. This is what I'm not so sure of. Currently we're posting a lot of data to views_ajax: view_name, view_display_id, view_args, view_path, view_dom_id, pager_element. Append all this to URLs? Ugly. Do we need to register the current view path as an alias of this ugly real path?

4. Add ctools-use-ajax class to relevant links (views pager, table header, and summary attachment links) and add ajax properties to exposed filter forms.

Does this sound vaguely right? What do we do about passing the view identification information to a callback?

dawehner’s picture

dawehner
Primary language German
commented

I would wait until d7views is ready, because the js there should have been converted to the new ajax framework, which was created out of ctools ajax.

merlinofchaos’s picture

merlinofchaos commented

Most of that work has already been done (except for preview which should not be difficult) and could potentially just be backported to D6.

Remon’s picture

Remon commented

any updates here?

nedjo’s picture

nedjo
he/him/his
Primary language English
commented
Status: Active » Postponed

Likely it would make sense to do #343542: Enable optional auto-refreshing views using AJAX first for D7 to provide the incentive for this considerable refactoring.

AdrianB’s picture

AdrianB commented

Subscribing.

YK85’s picture

YK85 commented

subscribing

dawehner’s picture

dawehner
Primary language German
commented
Version: 6.x-3.x-dev » 7.x-3.x-dev

This will not happen in d6, because it will not require ctools, but it should be done in d7.

yhahn’s picture

yhahn commented
Status: Postponed » Needs work
StatusFileSize
new 591302.1.patch11.19 KB

Here is a first proof of concept patch that uses D7 core AJAX API. There are still plenty of rough spots. One key problem: all AJAX requests are using HTTP POST (it is hardcoded into misc/ajax.js and there is a rather unsightly hack to accommodate this in views_ajax(). Besides hacking values from $_POST into $_GET, the requests will also require more custom configuration to work with proxy caches like Varnish. One option would be to extend Drupal.ajax to use GET or clean up the hack in place.

Some testing and additional direction would be useful here before the next iteration.

yhahn’s picture

yhahn commented
Status: Needs work » Needs review
StatusFileSize
new 591302.2.patch16.07 KB

Second version. We can avoid $_POST request for Views AJAX by extending Drupal.ajax. Unfortunately the way Drupal.ajax works requires a bit more copy and paste than the couple of lines we need to change, but it is effective and works without needing changes to core.

New patch uses HTTP GET, removes POST to GET vars hack, and extends Drupal.ajax in Drupal.Views.Ajax to allow for GET requests.

yhahn’s picture

yhahn commented
StatusFileSize
new 591302.3.patch11.16 KB

Updated v2 patch to have more selective overrides (overriding Drupal.ajax instances directly rather than doing prototype inheritance). Still faces shortcomings described by earl on IRC of not being able to update JS/CSS dynamically and take advantage of D7 AJAX API ajax_page_state.

However making use of this portion of the API is problematic with GET because the information needed to be passed back to Drupal (ajax_html_ids and ajax_page_state) totally blows through the GET url length limit on most HTTP servers. The request payload can easily reach 5k-15k+ on even simple pages (as to whether the size itself is an issue with a POST request is another topic).

For now here is the cleaned up patch for the GET approach, more to come...

yhahn’s picture

yhahn commented
StatusFileSize
new 591302.4.with_.956186.1.patch10.81 KB
new 591302.4.patch11.75 KB

I've updated a small issue in the patch (the required misc/ajax.js was not getting included on all pages where it was needed). The second version of the patch can be used in conjunction with #956186: Allow AJAX to use GET requests which fixes the AJAX framework request size and also enables use of GET with page state asset updating etc.

BenK’s picture

BenK commented

Subscribing

dasjo’s picture

dasjo
he/him
Primary language German
Location Zurich
commented

applied #15 and got

An error occurred while attempting to process /myprofile/views/ajax: Drupal.progressBar is not a constructor

when using the pager of my view.

katbailey’s picture

katbailey commented
StatusFileSize
new 591302.5.patch11.85 KB

Just made a minor change to the patch to fix #17 - changed views_add_js so that misc/progress.js and misc/ajax.js both get added when either ajax.js or ajax_view.js is being added.

bojanz’s picture

bojanz commented
Assigned: Unassigned » merlinofchaos

Assigning to merlinofchaos, as agreed on.

merlinofchaos’s picture

merlinofchaos commented
Status: Needs review » Fixed

Cleaned up a little and committed. This is no longer forcing $_GET, but instead copying $_POST to $_GET on the other side during AJAX requests.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

sokrplare’s picture

sokrplare commented
StatusFileSize
new 591302.4.with_.956186.1.against_Views_7.x-3.3.c.patch1.65 KB

I've ported the patch 591302.4.with_.956186.1.patch in #15 above to work against Views 7.x-3.3 (and Ctools 7.x-1.0). I also ported #956186-49: Allow AJAX to use GET requests to work against D7.16 in conjunction with this. See the other issue for notes.

This is not intended to go into any dev releases, it is just a workaround for those who absolutely must have AJAX GET cacheing - like me! :) The patch is a whole lot simpler since this issue was committed in comment #20 - now it is basically just the "forcing GET" part.

sokrplare’s picture

sokrplare commented
StatusFileSize
new views-Make_AJAX_paging_use_GET-591302-ish-23.patch752 bytes

Feeling a little silly as I realized while working on this a bit more, that really all this takes is a two-line code change to js/ajax_view.js. I have the patch attached against Views 7.x-3.3 and it makes paging work (in limited use cases - ie pages that don't need CSS/JS to be loaded) with AJAX using GET. A similar thing could probably be done with the same two lines for forms, but I haven't tested that at all as we just needed it for paging.

So in summary, no need for the core stuff, no need for my patch in comment #22 above, just this patch. Unless of course you need the ability to load CSS and JS etc. etc.

alanburke’s picture

alanburke commented
Status: Closed (fixed) » Needs review
sokrplare’s picture

sokrplare commented
Status: Needs review » Closed (fixed)

Leaving this as closed as my "patch" really is just a workaround for those needing caching - not a true patch. For there to be a full-blown patch a lot of Drupal core changes would need to happen. Thanks for keeping an eye on this though and if you have more thoughts on it do share!